Patchwork D3870: rebase: add --confirm option

login
register
mail settings
Submitter phabricator
Date July 11, 2018, 5:17 p.m.
Message ID <e43b4c6668112034add8acb252ffec21@localhost.localdomain>
Download mbox | patch
Permalink /patch/32781/
State Not Applicable
Headers show

Comments

phabricator - July 11, 2018, 5:17 p.m.
khanchi97 updated this revision to Diff 9538.

REPOSITORY
  rHG Mercurial

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

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 12, 2018, 1:14 p.m.
Queued, thanks.

> -    ui.status(_('starting dry-run rebase; repository will not be changed\n'))
> +    confirm = opts.get('confirm')
> +    if confirm:
> +        ui.status(_('starting rebase...\n'))

Nit: this message is misleading since nothing will be committed until
accepting the changes. Can you remove it? Or alternatively, we can say that
you're doing rebase in-memory so you're safe.

>      with repo.wlock(), repo.lock():
>          try:
> +            needsabort = True

Moved this before the "try".

>          except error.InMemoryMergeConflictsError:
>              ui.status(_('hit a merge conflict\n'))
> +            if confirm:
> +                # abort as in-memory merge doesn't support conflict
> +                rbsrt._prepareabortorcontinue(isabort=True, backup=False,
> +                                              suppwarns=True)
> +                needsabort = False
> +                if not ui.promptchoice(_(b'apply changes (yn)?'
> +                                         b'$$ &Yes $$ &No')):

Nit: This isn't actually "apply changes".

> +                    _dorebase(ui, repo, opts, inmemory=False)

`return _dorebase(...)` to not loose the actual result code. Can you
fix by a follow-up patch?
phabricator - July 12, 2018, 1:14 p.m.
yuja added a comment.


  Queued, thanks.
  
  > - ui.status(_('starting dry-run rebase; repository will not be changed\n')) +    confirm = opts.get('confirm') +    if confirm: +        ui.status(_('starting rebase...\n'))
  
  Nit: this message is misleading since nothing will be committed until
  accepting the changes. Can you remove it? Or alternatively, we can say that
  you're doing rebase in-memory so you're safe.
  
  >   with repo.wlock(), repo.lock():
  >       try:
  > 
  > +            needsabort = True
  
  Moved this before the "try".
  
  >   except error.InMemoryMergeConflictsError:
  >       ui.status(_('hit a merge conflict\n'))
  > 
  > +            if confirm:
  >  +                # abort as in-memory merge doesn't support conflict
  >  +                rbsrt._prepareabortorcontinue(isabort=True, backup=False,
  >  +                                              suppwarns=True)
  >  +                needsabort = False
  >  +                if not ui.promptchoice(_(b'apply changes (yn)?'
  >  +                                         b'$$ &Yes $$ &No')):
  
  Nit: This isn't actually "apply changes".
  
  > +                    _dorebase(ui, repo, opts, inmemory=False)
  
  `return _dorebase(...)` to not loose the actual result code. Can you
  fix by a follow-up patch?

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
@@ -323,3 +324,246 @@ 
   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 . --confirm --dry-run
+  abort: cannot specify both --confirm and --dry-run
+  [255]
+  $ hg rebase -s 2 -d . --confirm --abort
+  abort: cannot specify both --confirm and --abort
+  [255]
+  $ hg rebase -s 2 -d . --confirm --continue
+  abort: cannot specify both --confirm and --continue
+  [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,14 @@ 
             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'):
+        dryrun = True
+        if opts.get('dry_run'):
+            raise error.Abort(_('cannot specify both --confirm and --dry-run'))
+        if opts.get('continue'):
+            raise error.Abort(_('cannot specify both --confirm and --continue'))
+        if opts.get('abort'):
+            raise error.Abort(_('cannot specify both --confirm and --abort'))
 
     if (opts.get('continue') or opts.get('abort') or
         repo.currenttransaction() is not None):
@@ -844,24 +852,50 @@ 
 
 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:
+            needsabort = True
             overrides = {('rebase', 'singletransaction'): True}
             with ui.configoverride(overrides, 'rebase'):
                 _origrebase(ui, repo, opts, rbsrt, inmemory=True,
                             leaveunfinished=True)
         except error.InMemoryMergeConflictsError:
             ui.status(_('hit a merge conflict\n'))
+            if confirm:
+                # abort as in-memory merge doesn't support conflict
+                rbsrt._prepareabortorcontinue(isabort=True, backup=False,
+                                              suppwarns=True)
+                needsabort = False
+                if not ui.promptchoice(_(b'apply changes (yn)?'
+                                         b'$$ &Yes $$ &No')):
+                    _dorebase(ui, repo, opts, inmemory=False)
             return 1
         else:
-            ui.status(_('dry-run rebase completed successfully; run without '
-                        '-n/--dry-run to perform this rebase\n'))
+            if confirm:
+                ui.status(_('rebase completed successfully\n'))
+                if not ui.promptchoice(_(b'apply changes (yn)?'
+                                         b'$$ &Yes $$ &No')):
+                    # finish unfinished rebase
+                    rbsrt._finishrebase()
+                else:
+                    rbsrt._prepareabortorcontinue(isabort=True, backup=False,
+                                                  suppwarns=True)
+                needsabort = False
+            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 needsabort:
+                # 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)