D6501: state: created new class statecheck to handle unfinishedstates

taapas1128 (Taapas Agrawal) phabricator at mercurial-scm.org
Sun Jun 9 06:29:23 EDT 2019


taapas1128 marked an inline comment as done.
taapas1128 added inline comments.

INLINE COMMENTS

> martinvonz wrote in state.py:103
> Why? (I asked the same thing on a different patch, but I think that one may have been dropped. I'm on mobile and too lazy to check.)

actually I merge is not detected w.r.t statefile but with the help of a predicate but since it needs be part of the same class I named the statefile as None for it.

> martinvonz wrote in state.py:105
> It looks like the code actually just deletes the state file (fname). Should we say that here instead? (I'm not sure we should.)

telling that here would be fine I suppose so I should add it here.

> martinvonz wrote in state.py:118
> It seems unfortunate to have to write each message in this function. That's going to make it a little harder for extensions to add their commands to the list. Perhaps the hint should instead be passed in to the statecheck constructor (much like it was before this patch)? Same thing with msg() below.

I might have a better solution to it what if I leave this as it is and add two more variables to the constructor `specialmsg=None` and `specialhint=None`. In case the extension uses any special message or hint in place of standard ones these two values can be given accordingly and standard messages can be overidden.

> martinvonz wrote in state.py:143
> Should this be left for a later patch? I don't see "merge" getting registered anywhere yet.

yeah it should be part of the next patch but having it here will make `isfinished`complete

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6501

To: taapas1128, durin42, martinvonz, #hg-reviewers
Cc: mjpieters, mercurial-devel


More information about the Mercurial-devel mailing list