Submitter | Pierre-Yves David |
---|---|
Date | Jan. 9, 2014, 1:40 a.m. |
Message ID | <01daa33a1bd975621e9e.1389231617@marginatus.fb.com> |
Download | mbox | patch |
Permalink | /patch/3281/ |
State | Superseded |
Commit | 6545770bd37991b4ff0400479455a6e3ffa5976b |
Headers | show |
Comments
On Wed, Jan 08, 2014 at 05:40:17PM -0800, pierre-yves.david@ens-lyon.org wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1389230606 28800 > # Wed Jan 08 17:23:26 2014 -0800 > # Node ID 01daa33a1bd975621e9e54ef359f306746c69e23 > # Parent 693ba13bf57b3654617cc77e485df7f68235227b > backout: add a message after backout that need manual commit I think the message makes sense, but I think I'd rather it just said "changeset %s backed out - do not forget to commit" rather than including the potentially-confusing "in-place" statement. Thoughts? I've queued patches 1 and 2. > > In some case Backout silently succeeded to back out but left all the change > uncommitted. This may be confusing for user so this changeset add a note > reminding to commit. Other backout case already actively informs the user about > created commit. > > diff --git a/mercurial/commands.py b/mercurial/commands.py > --- a/mercurial/commands.py > +++ b/mercurial/commands.py > @@ -463,15 +463,21 @@ def backout(ui, repo, node=None, rev=Non > bheads = repo.branchheads(branch) > rctx = scmutil.revsingle(repo, hex(parent)) > if not opts.get('merge') and op1 != node: > try: > ui.setconfig('ui', 'forcemerge', opts.get('tool', '')) > - stats = mergemod.update(repo, parent, True, True, False, node, False) > + stats = mergemod.update(repo, parent, True, True, False, > + node, False) > repo.setparents(op1, op2) > hg._showstats(repo, stats) > if stats[3]: > - repo.ui.status(_("use 'hg resolve' to retry unresolved file merges\n")) > + repo.ui.status(_("use 'hg resolve' to retry unresolved " > + "file merges\n")) > + else: > + msg = _("changeset %s backed out in place, " > + "do not forget to commit.\n") > + ui.status(msg % short(node)) > return stats[3] > 0 > finally: > ui.setconfig('ui', 'forcemerge', '') > else: > hg.clean(repo, node, show_stats=False) > diff --git a/tests/test-backout.t b/tests/test-backout.t > --- a/tests/test-backout.t > +++ b/tests/test-backout.t > @@ -187,10 +187,11 @@ backout should not back out subsequent c > update: (current) > > without --merge > $ hg backout -d '3 0' 1 --tool=true > 1 files updated, 0 files merged, 0 files removed, 0 files unresolved > + changeset 22bca4c721e5 backed out in place, do not forget to commit. > $ hg locate b > b > $ hg update -C tip > 1 files updated, 0 files merged, 0 files removed, 0 files unresolved > $ hg locate b > @@ -322,10 +323,11 @@ named branches > adding file2 > > without --merge > $ hg backout -r 1 --tool=true > 0 files updated, 0 files merged, 1 files removed, 0 files unresolved > + changeset bf1602f437f3 backed out in place, do not forget to commit. > $ hg branch > branch2 > $ hg status -A > R file1 > C default > diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t > --- a/tests/test-subrepo.t > +++ b/tests/test-subrepo.t > @@ -467,10 +467,11 @@ update > backout calls revert internally with minimal opts, which should not raise > KeyError > > $ hg backout ".^" > 0 files updated, 0 files merged, 0 files removed, 0 files unresolved > + changeset c373c8102e68 backed out in place, do not forget to commit. > > $ hg up -C # discard changes > 1 files updated, 0 files merged, 0 files removed, 0 files unresolved > > pull > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On Wed, Jan 08, 2014 at 05:40:17PM -0800, pierre-yves.david@ens-lyon.org wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1389230606 28800 > # Wed Jan 08 17:23:26 2014 -0800 > # Node ID 01daa33a1bd975621e9e54ef359f306746c69e23 > # Parent 693ba13bf57b3654617cc77e485df7f68235227b > backout: add a message after backout that need manual commit (Actually, on reflection I'm going to just queue patch 1, since I wonder if maybe the lack of messaging would be confusing with patch 2 and not patch 3.) > > In some case Backout silently succeeded to back out but left all the change > uncommitted. This may be confusing for user so this changeset add a note > reminding to commit. Other backout case already actively informs the user about > created commit. > > diff --git a/mercurial/commands.py b/mercurial/commands.py > --- a/mercurial/commands.py > +++ b/mercurial/commands.py > @@ -463,15 +463,21 @@ def backout(ui, repo, node=None, rev=Non > bheads = repo.branchheads(branch) > rctx = scmutil.revsingle(repo, hex(parent)) > if not opts.get('merge') and op1 != node: > try: > ui.setconfig('ui', 'forcemerge', opts.get('tool', '')) > - stats = mergemod.update(repo, parent, True, True, False, node, False) > + stats = mergemod.update(repo, parent, True, True, False, > + node, False) > repo.setparents(op1, op2) > hg._showstats(repo, stats) > if stats[3]: > - repo.ui.status(_("use 'hg resolve' to retry unresolved file merges\n")) > + repo.ui.status(_("use 'hg resolve' to retry unresolved " > + "file merges\n")) > + else: > + msg = _("changeset %s backed out in place, " > + "do not forget to commit.\n") > + ui.status(msg % short(node)) > return stats[3] > 0 > finally: > ui.setconfig('ui', 'forcemerge', '') > else: > hg.clean(repo, node, show_stats=False) > diff --git a/tests/test-backout.t b/tests/test-backout.t > --- a/tests/test-backout.t > +++ b/tests/test-backout.t > @@ -187,10 +187,11 @@ backout should not back out subsequent c > update: (current) > > without --merge > $ hg backout -d '3 0' 1 --tool=true > 1 files updated, 0 files merged, 0 files removed, 0 files unresolved > + changeset 22bca4c721e5 backed out in place, do not forget to commit. > $ hg locate b > b > $ hg update -C tip > 1 files updated, 0 files merged, 0 files removed, 0 files unresolved > $ hg locate b > @@ -322,10 +323,11 @@ named branches > adding file2 > > without --merge > $ hg backout -r 1 --tool=true > 0 files updated, 0 files merged, 1 files removed, 0 files unresolved > + changeset bf1602f437f3 backed out in place, do not forget to commit. > $ hg branch > branch2 > $ hg status -A > R file1 > C default > diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t > --- a/tests/test-subrepo.t > +++ b/tests/test-subrepo.t > @@ -467,10 +467,11 @@ update > backout calls revert internally with minimal opts, which should not raise > KeyError > > $ hg backout ".^" > 0 files updated, 0 files merged, 0 files removed, 0 files unresolved > + changeset c373c8102e68 backed out in place, do not forget to commit. > > $ hg up -C # discard changes > 1 files updated, 0 files merged, 0 files removed, 0 files unresolved > > pull > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On 1/16/14, 6:48 AM, Augie Fackler wrote: > On Wed, Jan 08, 2014 at 05:40:17PM -0800, pierre-yves.david@ens-lyon.org wrote: >> # HG changeset patch >> # User Pierre-Yves David <pierre-yves.david@fb.com> >> # Date 1389230606 28800 >> # Wed Jan 08 17:23:26 2014 -0800 >> # Node ID 01daa33a1bd975621e9e54ef359f306746c69e23 >> # Parent 693ba13bf57b3654617cc77e485df7f68235227b >> backout: add a message after backout that need manual commit > I think the message makes sense, but I think I'd rather it just said > "changeset %s backed out - do not forget to commit" rather than > including the potentially-confusing "in-place" statement. Thoughts? > > I've queued patches 1 and 2. > Should backout write an "unfinished state" marker (similar to how rebase, graft, etc work)? While I haven't written a patch yet, I'm pretty sure the "annotated backouts" feature described at [1] will require a persistent state marker to handle the manual commit case. I don't mean to bloat scope - just raising a future concern. [1] http://www.selenic.com/pipermail/mercurial/2013-November/046246.html
On 01/17/2014 08:27 AM, Gregory Szorc wrote: > On 1/16/14, 6:48 AM, Augie Fackler wrote: >> On Wed, Jan 08, 2014 at 05:40:17PM -0800, >> pierre-yves.david@ens-lyon.org wrote: >>> # HG changeset patch >>> # User Pierre-Yves David <pierre-yves.david@fb.com> >>> # Date 1389230606 28800 >>> # Wed Jan 08 17:23:26 2014 -0800 >>> # Node ID 01daa33a1bd975621e9e54ef359f306746c69e23 >>> # Parent 693ba13bf57b3654617cc77e485df7f68235227b >>> backout: add a message after backout that need manual commit >> I think the message makes sense, but I think I'd rather it just said >> "changeset %s backed out - do not forget to commit" rather than >> including the potentially-confusing "in-place" statement. Thoughts? >> >> I've queued patches 1 and 2. >> > Should backout write an "unfinished state" marker (similar to how > rebase, graft, etc work)? Don't think so, neither merge nor shelve needs it. > While I haven't written a patch yet, I'm pretty sure the "annotated > backouts" feature described at [1] will require a persistent state > marker to handle the manual commit case. I don't mean to bloat scope - > just raising a future concern. > > [1] http://www.selenic.com/pipermail/mercurial/2013-November/046246.html Yeah out of scope but interresting feature.
Patch
diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -463,15 +463,21 @@ def backout(ui, repo, node=None, rev=Non bheads = repo.branchheads(branch) rctx = scmutil.revsingle(repo, hex(parent)) if not opts.get('merge') and op1 != node: try: ui.setconfig('ui', 'forcemerge', opts.get('tool', '')) - stats = mergemod.update(repo, parent, True, True, False, node, False) + stats = mergemod.update(repo, parent, True, True, False, + node, False) repo.setparents(op1, op2) hg._showstats(repo, stats) if stats[3]: - repo.ui.status(_("use 'hg resolve' to retry unresolved file merges\n")) + repo.ui.status(_("use 'hg resolve' to retry unresolved " + "file merges\n")) + else: + msg = _("changeset %s backed out in place, " + "do not forget to commit.\n") + ui.status(msg % short(node)) return stats[3] > 0 finally: ui.setconfig('ui', 'forcemerge', '') else: hg.clean(repo, node, show_stats=False) diff --git a/tests/test-backout.t b/tests/test-backout.t --- a/tests/test-backout.t +++ b/tests/test-backout.t @@ -187,10 +187,11 @@ backout should not back out subsequent c update: (current) without --merge $ hg backout -d '3 0' 1 --tool=true 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + changeset 22bca4c721e5 backed out in place, do not forget to commit. $ hg locate b b $ hg update -C tip 1 files updated, 0 files merged, 0 files removed, 0 files unresolved $ hg locate b @@ -322,10 +323,11 @@ named branches adding file2 without --merge $ hg backout -r 1 --tool=true 0 files updated, 0 files merged, 1 files removed, 0 files unresolved + changeset bf1602f437f3 backed out in place, do not forget to commit. $ hg branch branch2 $ hg status -A R file1 C default diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t --- a/tests/test-subrepo.t +++ b/tests/test-subrepo.t @@ -467,10 +467,11 @@ update backout calls revert internally with minimal opts, which should not raise KeyError $ hg backout ".^" 0 files updated, 0 files merged, 0 files removed, 0 files unresolved + changeset c373c8102e68 backed out in place, do not forget to commit. $ hg up -C # discard changes 1 files updated, 0 files merged, 0 files removed, 0 files unresolved pull