<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Nov 25, 2016, at 8:52 AM, Pierre-Yves David <<a href="mailto:pierre-yves.david@ens-lyon.org" class="">pierre-yves.david@ens-lyon.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On 11/19/2016 02:44 AM, Augie Fackler wrote:</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">On Fri, Nov 18, 2016 at 05:16:06PM +0100, Mads Kiilerich wrote:<br class=""><blockquote type="cite" class=""># HG changeset patch<br class=""># User Mads Kiilerich <<a href="mailto:madski@unity3d.com" class="">madski@unity3d.com</a>><br class=""># Date 1479485469 -3600<br class="">#      Fri Nov 18 17:11:09 2016 +0100<br class=""># Node ID 19ad769b648182e92a49015d85389fe2a8303c18<br class=""># Parent  1156ec81f70907ff843ca30bb81b4ef59b9b7068<br class="">histedit: make --keep work more like graft and use default phase for copies<br class=""></blockquote><br class="">queued, thanks<br class=""></blockquote><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Can we get a bit more explanation from Mads about why we would like to change the behavior? I'm open to a change but the current behavior make sense to me so I wonder what is the usercase to change it. In addition, `hg rebase --keep` is to preserve the source phases too¹ so this changes introduce and inconsistency here.</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">[1] except when source is public, in that case draft is used.</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div><div>IIRC Mads was reporting that it was preserving public phase, which struck me as an obvious bug.</div><br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">More comment below.</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class="">diff --git a/hgext/histedit.py b/hgext/histedit.py<br class="">--- a/hgext/histedit.py<br class="">+++ b/hgext/histedit.py<br class="">@@ -91,7 +91,8 @@ ones) until after it has completed all t<br class="">probably perform several strip operations when it's done. For the above example,<br class="">it had to run strip twice. Strip can be slow depending on a variety of factors,<br class="">so you might need to be a little patient. You can choose to keep the original<br class="">-revisions by passing the ``--keep`` flag.<br class="">+revisions by passing the ``--keep`` flag. This will work more like graft and<br class="">+not preserve/copy the original phase.<br class=""><br class="">The ``edit`` operation will drop you back to a command prompt,<br class="">allowing you to edit files freely, or even use ``hg record`` to commit<br class="">@@ -466,7 +467,7 @@ class histeditaction(object):<br class="">        rulectx = repo[self.node]<br class=""><br class="">        editor = self.commiteditor()<br class="">-        commit = commitfuncfor(repo, rulectx)<br class="">+        commit = commitfuncfor(repo, rulectx, self.state.keep)<br class=""><br class="">        commit(text=rulectx.description(), user=rulectx.user(),<br class="">               date=rulectx.date(), extra=rulectx.extra(), editor=editor)<br class="">@@ -489,7 +490,7 @@ class histeditaction(object):<br class="">            return ctx, []<br class="">        return ctx, [(self.node, (ctx.node(),))]<br class=""><br class="">-def commitfuncfor(repo, src):<br class="">+def commitfuncfor(repo, src, keep):<br class="">    """Build a commit function for the replacement of <src><br class=""><br class="">    This function ensure we apply the same treatment to all changesets.<br class="">@@ -503,8 +504,9 @@ def commitfuncfor(repo, src):<br class="">    def commitfunc(**kwargs):<br class="">        phasebackup = repo.ui.backupconfig('phases', 'new-commit')<br class="">        try:<br class="">-            repo.ui.setconfig('phases', 'new-commit', phasemin,<br class="">-                              'histedit')<br class="">+            if not keep:<br class="">+                repo.ui.setconfig('phases', 'new-commit', phasemin,<br class="">+                                  'histedit')<br class="">            extra = kwargs.get('extra', {}).copy()<br class="">            extra['histedit_source'] = src.hex()<br class="">            kwargs['extra'] = extra<br class="">@@ -531,7 +533,7 @@ def applychanges(ui, repo, ctx, opts):<br class="">            repo.ui.setconfig('ui', 'forcemerge', '', 'histedit')<br class="">    return stats<br class=""><br class="">-def collapse(repo, first, last, commitopts, skipprompt=False):<br class="">+def collapse(repo, first, last, commitopts, skipprompt=False, keep=False):<br class="">    """collapse the set of revisions from first to last as new one.<br class=""><br class="">    Expected commit options are:<br class="">@@ -545,7 +547,7 @@ def collapse(repo, first, last, commitop<br class="">    if not ctxs:<br class="">        return None<br class="">    for c in ctxs:<br class="">-        if not c.mutable():<br class="">+        if not c.mutable() and not keep:<br class=""></blockquote></blockquote><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">It seems like this changeset is introducing a secondary changes, it makes it possible to `hg histedit --keep` public changeset. It sounds like an independent changes that does not alter any existing behavior. It would be better to have it as a separate changes.</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div><div>That honestly seems like it should always have been allowed? In a perfect world yes, this should have been a separate change, but I don’t think it’s worth churning the stack for something so minor.</div><br class=""><blockquote type="cite" class=""><div class=""><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class="">            raise error.ParseError(<br class="">                _("cannot fold into public change %s") % node.short(c.node()))<br class="">    base = first.parents()[0]<br class="">@@ -669,16 +671,15 @@ class fold(histeditaction):<br class="">            return<br class="">        else:<br class="">            c = repo[prev.node]<br class="">-        if not c.mutable():<br class="">+        if not c.mutable() and not self.state.keep:<br class="">            raise error.ParseError(<br class="">                _("cannot fold into public change %s") % node.short(c.node()))<br class=""><br class="">-<br class="">    def continuedirty(self):<br class="">        repo = self.repo<br class="">        rulectx = repo[self.node]<br class=""><br class="">-        commit = commitfuncfor(repo, rulectx)<br class="">+        commit = commitfuncfor(repo, rulectx, self.state.keep)<br class="">        commit(text='fold-temp-revision %s' % node.short(self.node),<br class="">               user=rulectx.user(), date=rulectx.date(),<br class="">               extra=rulectx.extra())<br class="">@@ -752,9 +753,10 @@ class fold(histeditaction):<br class="">        phasebackup = repo.ui.backupconfig('phases', 'new-commit')<br class="">        try:<br class="">            phasemin = max(ctx.phase(), oldctx.phase())<br class="">-            repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit')<br class="">+            if not self.state.keep:<br class="">+                repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit')<br class="">            n = collapse(repo, ctx, repo[newnode], commitopts,<br class="">-                         skipprompt=self.skipprompt())<br class="">+                         skipprompt=self.skipprompt(), keep=self.state.keep)<br class="">        finally:<br class="">            repo.ui.restoreconfig(phasebackup)<br class="">        if n is None:<br class="">diff --git a/tests/test-histedit-edit.t b/tests/test-histedit-edit.t<br class="">--- a/tests/test-histedit-edit.t<br class="">+++ b/tests/test-histedit-edit.t<br class="">@@ -453,6 +453,7 @@ rollback should not work after a histedi<br class="">  $ echo foo >> a<br class="">  $ hg ci -m 'extend a'<br class="">  $ hg phase --public 1<br class="">+<br class="">Attempting to fold a change into a public change should not work:<br class="">  $ cat > ../edit.sh <<EOF<br class="">  > cat "\$1" | sed s/pick/fold/ > tmp<br class="">@@ -480,3 +481,19 @@ Attempting to fold a change into a publi<br class="">  #  f, fold = use commit, but combine it with the one above<br class="">  #  r, roll = like fold, but discard this commit's description<br class="">  #<br class="">+<br class="">+but it should work more like graft when using --keep<br class="">+<br class="">+  $ HGEDITOR="sh ../edit.sh" hg histedit 2 --keep<br class="">+  saved backup bundle to $TESTTMP/r0/.hg/strip-backup/17b1aa9a4a0b-8d4a5be3-backup.hg (glob)<br class="">+<br class="">+  $ hg log -G -T '{rev} {node|short} {phase} {desc|firstline}'<br class="">+  @  3 311eafe309d8 draft add b<br class="">+  |<br class="">+  | o  2 0012be4a27ea draft extend a<br class="">+  | |<br class="">+  | o  1 18aa70c8ad22 public add b<br class="">+  |/<br class="">+  o  0 0efcea34f18a public a<br class="">+<br class="">+<br class="">_______________________________________________<br class="">Mercurial-devel mailing list<br class=""><a href="mailto:Mercurial-devel@mercurial-scm.org" class="">Mercurial-devel@mercurial-scm.org</a><br class="">https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel<br class=""></blockquote>_______________________________________________<br class="">Mercurial-devel mailing list<br class=""><a href="mailto:Mercurial-devel@mercurial-scm.org" class="">Mercurial-devel@mercurial-scm.org</a><br class="">https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel<br class=""><br class=""></blockquote><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">--<span class="Apple-converted-space"> </span></span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Pierre-Yves David</span></div></blockquote></div><br class=""></body></html>