| Submitter | phabricator |
|---|---|
| Date | June 23, 2019, 5:43 p.m. |
| Message ID | <differential-rev-PHID-DREV-yfmsboaj4zaxtbv6aoym-req@mercurial-scm.org> |
| Download | mbox | patch |
| Permalink | /patch/40652/ |
| State | Superseded |
| Headers | show |
Comments
martinvonz added a comment. Need to set the parent of this patch REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6566/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6566 To: taapas1128, #hg-reviewers Cc: martinvonz, mercurial-devel
taapas1128 added a comment. @martinvonz I wanted this as a seperate stack for now so that the previous stack can be committed first. Do you want me to update its parents and add it to the previous stack? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6566/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6566 To: taapas1128, #hg-reviewers Cc: martinvonz, mercurial-devel
pulkit added inline comments. INLINE COMMENTS > commands.py:147 > + if state.isunfinished(repo): > + abortstate = state > + break `return state.abortfunc(ui, repo, **opts)` instead of storing and calling that later. > commands.py:152 > + if abortstate._abortfunc: > + return abortstate._abortfunc(ui, repo) > + raise error.Abort((_("%s does not support 'hg abort'") % we should pass opts to abortfunc too. I can think of `--no-backup` as a flag which tells not to store a backup when aborting. A nice addition will be `--dry-run` flag which tells which operation will be aborted. Also, maybe add a message saying `aborting <command>`. > state.py:102 > reportonly=False, continueflag=False, stopflag=False , > - cmdmsg="", cmdhint="", statushint=""): > + cmdmsg="", cmdhint="", statushint="", abortfunc=None): > """opname is the name the command or operation the new argument needs documentation. > state.py:136 > self._continueflag = continueflag > + self._abortfunc = abortfunc > since we will be using this argument outside of this class, let's not prepend `_` to the name. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6566/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6566 To: taapas1128, #hg-reviewers Cc: pulkit, martinvonz, mercurial-devel
pulkit added inline comments. INLINE COMMENTS > commands.py:166 > + > + if nobackup: > + arglist = inspect.getargspec(abortstate.abortfunc)[0] Arguments for abort can increase in future. Rather pass the opts directly to abort functions for now and make related functions look for flag which they need. I agree it's not nice, but seems like a good tradeoff to start and have better code. > commands.py:173 > + if dryrun: > + raise error.Abort(_('aborting %s') % (abortstate._opname)) > + should be `ui.status()` instead of `error.Abort()` REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6566/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6566 To: taapas1128, #hg-reviewers Cc: pulkit, martinvonz, mercurial-devel
pulkit added inline comments. INLINE COMMENTS > commands.py:135 > > +@command('abort', > + [('', 'no-backup', False, _('do not save backup copies of files')), Can you mark this command as experimental for now, we will be close to release soon. Marking this as experimental will help us take more time to smoothen things out. > commands.py:136 > +@command('abort', > + [('', 'no-backup', False, _('do not save backup copies of files')), > + ] + dryrunopts, helpcategory=command.CATEGORY_CHANGE_MANAGEMENT, This help should be `'do not save backup bundle'` as similar to strip command. > commands.py:142 > + > + Aborts an multistep operation like graft, histedit, rebase, merge, > + and unshelve if they are in an unfinished state. s/an/a > commands.py:154 > + abortstate = None > + dryrun = opts.get('dry_run') > + for state in statemod._unfinishedstates: need to prepend `r''` in front of dry_run for py3 compat. > commands.py:165 > + if dryrun: > + ui.status(_('aborting %s\n') % (abortstate._opname)) > + return abortstate.abortfunc(ui, repo, **opts) we should return here so that `abortstate.abortfunc()` is not called below in case of dry-run. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6566/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6566 To: taapas1128, #hg-reviewers Cc: pulkit, martinvonz, mercurial-devel
pulkit added a comment. > hg abort currently supports --dry-run/-n flag only. > It is used to dry run hg abort We should specify what does dry run does i.e. it prints the operation which will be aborted if `hg abort` is run. INLINE COMMENTS > commands.py:139 > +def abort(ui, repo, **opts): > + """abort an unfinished operation(EXPERIMENTAL) > + You need a space between operation and (EXPERIMENTAL) > commands.py:153 > + if not abortstate.abortfunc: > + raise error.Abort((_("%s does not support 'hg abort'") % > + (abortstate._opname)), hint=abortstate.hint()) `%s is in progress but does not support 'hg abort'`. > commands.py:158 > + return > + return abortstate.abortfunc(ui, repo, **opts) > + Now that we don't have backup flag, and --dry-run is tackled before calling abortfunc, we can prevent passing `**opts`. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6566/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6566 To: taapas1128, #hg-reviewers Cc: pulkit, martinvonz, mercurial-devel
Patch
diff --git a/tests/test-help.t b/tests/test-help.t --- a/tests/test-help.t +++ b/tests/test-help.t @@ -5,6 +5,7 @@ basic commands: + abort abort an unfinished operation add add the specified files on the next commit annotate show changeset information by line for each file clone make a copy of an existing repository @@ -26,6 +27,7 @@ (use 'hg help' for the full list of commands or 'hg -v' for details) $ hg -q + abort abort an unfinished operation add add the specified files on the next commit annotate show changeset information by line for each file clone make a copy of an existing repository @@ -73,6 +75,7 @@ Change manipulation: + abort abort an unfinished operation backout reverse effect of earlier changeset graft copy changes from other branches onto the current branch merge merge another revision into working directory @@ -199,6 +202,7 @@ Change manipulation: + abort abort an unfinished operation backout reverse effect of earlier changeset graft copy changes from other branches onto the current branch merge merge another revision into working directory @@ -399,6 +403,7 @@ basic commands: + abort abort an unfinished operation add add the specified files on the next commit annotate, blame show changeset information by line for each file @@ -2350,6 +2355,13 @@ <tr><td colspan="2"><h2><a name="main" href="#main">Main Commands</a></h2></td></tr> <tr><td> + <a href="/help/abort"> + abort + </a> + </td><td> + abort an unfinished operation + </td></tr> + <tr><td> <a href="/help/add"> add </a> diff --git a/mercurial/state.py b/mercurial/state.py --- a/mercurial/state.py +++ b/mercurial/state.py @@ -99,7 +99,7 @@ def __init__(self, opname, fname, clearable=False, allowcommit=False, reportonly=False, continueflag=False, stopflag=False , - cmdmsg="", cmdhint="", statushint=""): + cmdmsg="", cmdhint="", statushint="", abortfunc=None): """opname is the name the command or operation fname is the file name in which data should be stored in .hg directory. It is None for merge command. @@ -135,6 +135,7 @@ self._stopflag = stopflag self._reportonly = reportonly self._continueflag = continueflag + self._abortfunc = abortfunc def statusmsg(self): """returns the hint message corresponding to the command for diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -131,6 +131,28 @@ # Commands start here, listed alphabetically +@command('abort', [], helpcategory=command.CATEGORY_CHANGE_MANAGEMENT, + helpbasic=True) +def abort(ui, repo, **opts): + """abort an unfinished operation + + This function is the first prototype for hg abort command. + It first checks the state and checks which command is active + then calls then returns the respective logic for aborting the + command. + """ + abortstate = None + for state in statemod._unfinishedstates: + if state.isunfinished(repo): + abortstate = state + break + if not abortstate: + raise error.Abort(_('no operation in progress')) + if abortstate._abortfunc: + return abortstate._abortfunc(ui, repo) + raise error.Abort((_("%s does not support 'hg abort'") % + (abortstate._opname)), hint=abortstate.hint()) + @command('add', walkopts + subrepoopts + dryrunopts, _('[OPTION]... [FILE]...'),