Patchwork D3870: rebase: add --confirm option

login
register
mail settings
Submitter phabricator
Date July 10, 2018, 3:43 a.m.
Message ID <3105be9ce5c0a2fa365f6e77fccaa50b@localhost.localdomain>
Download mbox | patch
Permalink /patch/32725/
State Not Applicable
Headers show

Comments

phabricator - July 10, 2018, 3:43 a.m.
khanchi97 updated this revision to Diff 9486.
khanchi97 edited the summary of this revision.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D3870?vs=9386&id=9486

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

AFFECTED FILES
  hgext/rebase.py
  mercurial/cmdutil.py
  tests/test-rebase-inmemory.t

CHANGE DETAILS




To: khanchi97, #hg-reviewers
Cc: yuja, mercurial-devel
Yuya Nishihara - July 10, 2018, 1:18 p.m.
This can't be applied, can you rebase?


> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -677,7 +677,7 @@
>      ('a', 'abort', False, _('abort an interrupted rebase')),
>      ('', 'auto-orphans', '', _('automatically rebase orphan revisions '
>                                 'in the specified revset (EXPERIMENTAL)')),
> -     ] + cmdutil.dryrunopts + cmdutil.formatteropts,
> +     ] + cmdutil.dryrunopts + cmdutil.formatteropts + cmdutil.confirmopts,
>      _('[-s REV | -b REV] [-d REV] [OPTION]'))
>  def rebase(ui, repo, **opts):
>      """move changeset (and descendants) to a different branch
> @@ -808,6 +808,10 @@
>              raise error.Abort(_('cannot specify both --dry-run and --abort'))
>          if opts.get('continue'):
>              raise error.Abort(_('cannot specify both --dry-run and --continue'))
> +        if opts.get('confirm'):
> +            raise error.Abort(_('cannot specify both --dry-run and --confirm'))
> +    if opts.get('confirm'):
> +        dryrun = True

Nit: we'll have to reject "--confirm --abort" and "--confirm --continue" as
well.

>      with repo.wlock(), repo.lock():
>          try:
>              overrides = {('rebase', 'singletransaction'): True}
>              with ui.configoverride(overrides, 'rebase'):
>                  _origrebase(ui, repo, opts, rbsrt, inmemory=True,
>                              leaveunfinished=True, supptrwarns=True)
>          except error.InMemoryMergeConflictsError:
> +            conflict = True
>              ui.status(_('hit a merge conflict\n'))
>              return 1
>          else:
> -            ui.status(_('dry-run rebase completed successfully; run without '
> -                        '-n/--dry-run to perform this rebase\n'))
> +            conflict = False
> +            if confirm:
> +                ui.status(_('rebase completed successfully\n'))
> +            else:
> +                ui.status(_('dry-run rebase completed successfully; run without'
> +                            ' -n/--dry-run to perform this rebase\n'))
>              return 0
>          finally:
> -            # no need to store backup in case of dryrun
> -            rbsrt._prepareabortorcontinue(isabort=True, backup=False,
> -                                          suppwarns=True)
> +            if confirm and not ui.promptchoice(_(b'apply changes (yn)?'
> +                                                 b'$$ &Yes $$ &No')):
> +                if conflict:
> +                    rbsrt._prepareabortorcontinue(isabort=True,
> +                                                  backup=False,
> +                                                  suppwarns=True)
> +                    _dorebase(ui, repo, opts, inmemory=False)
> +                else:
> +                    # finish unfinished rebase
> +                    rbsrt._finishrebase()

I'm not a fan of doing much things "finally:" clause. Can you move this to
the "else:" clause? The "conflict" flag can be flipped to be a "needsabort",
and it has to be initialized before "try:".
phabricator - July 10, 2018, 1:18 p.m.
yuja added a comment.


  This can't be applied, can you rebase?
  
  >   - a/hgext/rebase.py +++ b/hgext/rebase.py @@ -677,7 +677,7 @@ ('a', 'abort', False, _('abort an interrupted rebase')), ('', 'auto-orphans', '', _('automatically rebase orphan revisions ' 'in the specified revset (EXPERIMENTAL)')),
  > - ] + cmdutil.dryrunopts + cmdutil.formatteropts, +     ] + cmdutil.dryrunopts + cmdutil.formatteropts + cmdutil.confirmopts, _('[-s REV | -b REV] [-d REV] [OPTION]')) def rebase(ui, repo, **opts): """move changeset (and descendants) to a different branch @@ -808,6 +808,10 @@ raise error.Abort(_('cannot specify both --dry-run and --abort')) if opts.get('continue'): raise error.Abort(_('cannot specify both --dry-run and --continue')) +        if opts.get('confirm'): +            raise error.Abort(_('cannot specify both --dry-run and --confirm')) +    if opts.get('confirm'): +        dryrun = True
  
  Nit: we'll have to reject "--confirm --abort" and "--confirm --continue" as
  well.
  
  >   with repo.wlock(), repo.lock():
  >       try:
  >           overrides = {('rebase', 'singletransaction'): True}
  >           with ui.configoverride(overrides, 'rebase'):
  >               _origrebase(ui, repo, opts, rbsrt, inmemory=True,
  >                           leaveunfinished=True, supptrwarns=True)
  >       except error.InMemoryMergeConflictsError:
  > 
  > +            conflict = True
  > 
  >       ui.status(_('hit a merge conflict\n'))
  >       return 1
  >   else:
  > 
  > - ui.status(_('dry-run rebase completed successfully; run without '
  > - '-n/--dry-run to perform this rebase\n')) +            conflict = False +            if confirm: +                ui.status(_('rebase completed successfully\n')) +            else: +                ui.status(_('dry-run rebase completed successfully; run without' +                            ' -n/--dry-run to perform this rebase\n')) return 0 finally:
  > - # no need to store backup in case of dryrun
  > - rbsrt._prepareabortorcontinue(isabort=True, backup=False,
  > - suppwarns=True) +            if confirm and not ui.promptchoice(_(b'apply changes (yn)?' +                                                 b'$$ &Yes $$ &No')): +                if conflict: +                    rbsrt._prepareabortorcontinue(isabort=True, +                                                  backup=False, +                                                  suppwarns=True) +                    _dorebase(ui, repo, opts, inmemory=False) +                else: +                    # finish unfinished rebase +                    rbsrt._finishrebase()
  
  I'm not a fan of doing much things "finally:" clause. Can you move this to
  the "else:" clause? The "conflict" flag can be flipped to be a "needsabort",
  and it has to be initialized before "try:".

REPOSITORY
  rHG Mercurial

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

To: khanchi97, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/tests/test-rebase-inmemory.t b/tests/test-rebase-inmemory.t
--- a/tests/test-rebase-inmemory.t
+++ b/tests/test-rebase-inmemory.t
@@ -4,6 +4,7 @@ 
   > amend=
   > rebase=
   > debugdrawdag=$TESTDIR/drawdag.py
+  > strip=
   > [rebase]
   > experimental.inmemory=1
   > [diff]
@@ -156,8 +157,8 @@ 
   o  0: b173517d0057 'a'
   
 Test dry-run rebasing
-  $ hg init skrepo
-  $ cd skrepo
+  $ hg init repo3
+  $ cd repo3
   $ echo a>a
   $ hg ci -Aqma
   $ echo b>b
@@ -321,3 +322,240 @@ 
   merging e
   hit a merge conflict
   [1]
+
+==========================
+Test for --confirm option|
+==========================
+  $ cd ..
+  $ hg clone repo3 repo4 -q
+  $ cd repo4
+  $ hg strip 7 -q
+  $ hg log -G --template "{rev}:{short(node)} {person(author)}\n{firstline(desc)} {topic}\n\n"
+  @  6:baf10c5166d4 test
+  |  g
+  |
+  o  5:6343ca3eff20 test
+  |  f
+  |
+  | o  4:e860deea161a test
+  | |  e
+  | |
+  | o  3:055a42cdd887 test
+  | |  d
+  | |
+  | o  2:177f92b77385 test
+  |/   c
+  |
+  o  1:d2ae7f538514 test
+  |  b
+  |
+  o  0:cb9a9f314b8b test
+     a
+  
+Check it gives error when both --dryrun and --confirm is used:
+  $ hg rebase -s 2 -d . --keep --config ui.interactive=True --confirm --dry-run
+  abort: cannot specify both --dry-run and --confirm
+  [255]
+
+Test --confirm option when there are no conflicts:
+  $ hg rebase -s 2 -d . --keep --config ui.interactive=True --confirm << EOF
+  > n
+  > EOF
+  starting rebase...
+  rebasing 2:177f92b77385 "c"
+  rebasing 3:055a42cdd887 "d"
+  rebasing 4:e860deea161a "e"
+  rebase completed successfully
+  apply changes (yn)? n
+  $ hg log -G --template "{rev}:{short(node)} {person(author)}\n{firstline(desc)} {topic}\n\n"
+  @  6:baf10c5166d4 test
+  |  g
+  |
+  o  5:6343ca3eff20 test
+  |  f
+  |
+  | o  4:e860deea161a test
+  | |  e
+  | |
+  | o  3:055a42cdd887 test
+  | |  d
+  | |
+  | o  2:177f92b77385 test
+  |/   c
+  |
+  o  1:d2ae7f538514 test
+  |  b
+  |
+  o  0:cb9a9f314b8b test
+     a
+  
+  $ hg rebase -s 2 -d . --keep --config ui.interactive=True --confirm << EOF
+  > y
+  > EOF
+  starting rebase...
+  rebasing 2:177f92b77385 "c"
+  rebasing 3:055a42cdd887 "d"
+  rebasing 4:e860deea161a "e"
+  rebase completed successfully
+  apply changes (yn)? y
+  $ hg log -G --template "{rev}:{short(node)} {person(author)}\n{firstline(desc)} {topic}\n\n"
+  o  9:9fd28f55f6dc test
+  |  e
+  |
+  o  8:12cbf031f469 test
+  |  d
+  |
+  o  7:c83b1da5b1ae test
+  |  c
+  |
+  @  6:baf10c5166d4 test
+  |  g
+  |
+  o  5:6343ca3eff20 test
+  |  f
+  |
+  | o  4:e860deea161a test
+  | |  e
+  | |
+  | o  3:055a42cdd887 test
+  | |  d
+  | |
+  | o  2:177f92b77385 test
+  |/   c
+  |
+  o  1:d2ae7f538514 test
+  |  b
+  |
+  o  0:cb9a9f314b8b test
+     a
+  
+Test --confirm option when there is a conflict
+  $ hg up tip -q
+  $ echo ee>e
+  $ hg ci --amend -m "conflict with e" -q
+  $ hg log -G --template "{rev}:{short(node)} {person(author)}\n{firstline(desc)} {topic}\n\n"
+  @  9:906d72f66a59 test
+  |  conflict with e
+  |
+  o  8:12cbf031f469 test
+  |  d
+  |
+  o  7:c83b1da5b1ae test
+  |  c
+  |
+  o  6:baf10c5166d4 test
+  |  g
+  |
+  o  5:6343ca3eff20 test
+  |  f
+  |
+  | o  4:e860deea161a test
+  | |  e
+  | |
+  | o  3:055a42cdd887 test
+  | |  d
+  | |
+  | o  2:177f92b77385 test
+  |/   c
+  |
+  o  1:d2ae7f538514 test
+  |  b
+  |
+  o  0:cb9a9f314b8b test
+     a
+  
+  $ hg rebase -s 4 -d . --keep --config ui.interactive=True --confirm << EOF
+  > n
+  > EOF
+  starting rebase...
+  rebasing 4:e860deea161a "e"
+  merging e
+  hit a merge conflict
+  apply changes (yn)? n
+  [1]
+  $ hg log -G --template "{rev}:{short(node)} {person(author)}\n{firstline(desc)} {topic}\n\n"
+  @  9:906d72f66a59 test
+  |  conflict with e
+  |
+  o  8:12cbf031f469 test
+  |  d
+  |
+  o  7:c83b1da5b1ae test
+  |  c
+  |
+  o  6:baf10c5166d4 test
+  |  g
+  |
+  o  5:6343ca3eff20 test
+  |  f
+  |
+  | o  4:e860deea161a test
+  | |  e
+  | |
+  | o  3:055a42cdd887 test
+  | |  d
+  | |
+  | o  2:177f92b77385 test
+  |/   c
+  |
+  o  1:d2ae7f538514 test
+  |  b
+  |
+  o  0:cb9a9f314b8b test
+     a
+  
+
+  $ hg rebase -s 4 -d . --keep --config ui.interactive=True --confirm << EOF
+  > y
+  > EOF
+  starting rebase...
+  rebasing 4:e860deea161a "e"
+  merging e
+  hit a merge conflict
+  apply changes (yn)? y
+  rebasing 4:e860deea161a "e"
+  merging e
+  warning: conflicts while merging e! (edit, then use 'hg resolve --mark')
+  unresolved conflicts (see hg resolve, then hg rebase --continue)
+  [1]
+
+  $ echo e>e
+  $ hg resolve --mark --all
+  (no more unresolved files)
+  continue: hg rebase --continue
+  $ hg rebase --continue
+  rebasing 4:e860deea161a "e"
+  $ hg log -G --template "{rev}:{short(node)} {person(author)}\n{firstline(desc)} {topic}\n\n"
+  o  10:9fa3731dd6df test
+  |  e
+  |
+  @  9:906d72f66a59 test
+  |  conflict with e
+  |
+  o  8:12cbf031f469 test
+  |  d
+  |
+  o  7:c83b1da5b1ae test
+  |  c
+  |
+  o  6:baf10c5166d4 test
+  |  g
+  |
+  o  5:6343ca3eff20 test
+  |  f
+  |
+  | o  4:e860deea161a test
+  | |  e
+  | |
+  | o  3:055a42cdd887 test
+  | |  d
+  | |
+  | o  2:177f92b77385 test
+  |/   c
+  |
+  o  1:d2ae7f538514 test
+  |  b
+  |
+  o  0:cb9a9f314b8b test
+     a
+  
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -62,6 +62,11 @@ 
      _('do not perform actions, just print output')),
 ]
 
+confirmopts = [
+    ('', 'confirm', None,
+     _('ask before applying actions')),
+]
+
 remoteopts = [
     ('e', 'ssh', '',
      _('specify ssh command to use'), _('CMD')),
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -677,7 +677,7 @@ 
     ('a', 'abort', False, _('abort an interrupted rebase')),
     ('', 'auto-orphans', '', _('automatically rebase orphan revisions '
                                'in the specified revset (EXPERIMENTAL)')),
-     ] + cmdutil.dryrunopts + cmdutil.formatteropts,
+     ] + cmdutil.dryrunopts + cmdutil.formatteropts + cmdutil.confirmopts,
     _('[-s REV | -b REV] [-d REV] [OPTION]'))
 def rebase(ui, repo, **opts):
     """move changeset (and descendants) to a different branch
@@ -808,6 +808,10 @@ 
             raise error.Abort(_('cannot specify both --dry-run and --abort'))
         if opts.get('continue'):
             raise error.Abort(_('cannot specify both --dry-run and --continue'))
+        if opts.get('confirm'):
+            raise error.Abort(_('cannot specify both --dry-run and --confirm'))
+    if opts.get('confirm'):
+        dryrun = True
 
     if (opts.get('continue') or opts.get('abort') or
         repo.currenttransaction() is not None):
@@ -844,24 +848,45 @@ 
 
 def _dryrunrebase(ui, repo, opts):
     rbsrt = rebaseruntime(repo, ui, inmemory=True, opts=opts)
-    ui.status(_('starting dry-run rebase; repository will not be changed\n'))
+    confirm = opts.get('confirm')
+    if confirm:
+        ui.status(_('starting rebase...\n'))
+    else:
+        ui.status(_('starting dry-run rebase; repository will not be '
+                    'changed\n'))
     with repo.wlock(), repo.lock():
         try:
             overrides = {('rebase', 'singletransaction'): True}
             with ui.configoverride(overrides, 'rebase'):
                 _origrebase(ui, repo, opts, rbsrt, inmemory=True,
                             leaveunfinished=True, supptrwarns=True)
         except error.InMemoryMergeConflictsError:
+            conflict = True
             ui.status(_('hit a merge conflict\n'))
             return 1
         else:
-            ui.status(_('dry-run rebase completed successfully; run without '
-                        '-n/--dry-run to perform this rebase\n'))
+            conflict = False
+            if confirm:
+                ui.status(_('rebase completed successfully\n'))
+            else:
+                ui.status(_('dry-run rebase completed successfully; run without'
+                            ' -n/--dry-run to perform this rebase\n'))
             return 0
         finally:
-            # no need to store backup in case of dryrun
-            rbsrt._prepareabortorcontinue(isabort=True, backup=False,
-                                          suppwarns=True)
+            if confirm and not ui.promptchoice(_(b'apply changes (yn)?'
+                                                 b'$$ &Yes $$ &No')):
+                if conflict:
+                    rbsrt._prepareabortorcontinue(isabort=True,
+                                                  backup=False,
+                                                  suppwarns=True)
+                    _dorebase(ui, repo, opts, inmemory=False)
+                else:
+                    # finish unfinished rebase
+                    rbsrt._finishrebase()
+            else:
+                # no need to store backup in case of dryrun
+                rbsrt._prepareabortorcontinue(isabort=True, backup=False,
+                                              suppwarns=True)
 
 def _dorebase(ui, repo, opts, inmemory=False):
     rbsrt = rebaseruntime(repo, ui, inmemory, opts)