Patchwork histedit: make --keep work more like graft and use default phase for copies

login
register
mail settings
Submitter Mads Kiilerich
Date Nov. 18, 2016, 4:16 p.m.
Message ID <19ad769b648182e92a49.1479485766@madski>
Download mbox | patch
Permalink /patch/17638/
State Accepted
Headers show

Comments

Mads Kiilerich - Nov. 18, 2016, 4:16 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1479485469 -3600
#      Fri Nov 18 17:11:09 2016 +0100
# Node ID 19ad769b648182e92a49015d85389fe2a8303c18
# Parent  1156ec81f70907ff843ca30bb81b4ef59b9b7068
histedit: make --keep work more like graft and use default phase for copies
Augie Fackler - Nov. 19, 2016, 1:44 a.m.
On Fri, Nov 18, 2016 at 05:16:06PM +0100, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1479485469 -3600
> #      Fri Nov 18 17:11:09 2016 +0100
> # Node ID 19ad769b648182e92a49015d85389fe2a8303c18
> # Parent  1156ec81f70907ff843ca30bb81b4ef59b9b7068
> histedit: make --keep work more like graft and use default phase for copies

queued, thanks

>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -91,7 +91,8 @@ ones) until after it has completed all t
>  probably perform several strip operations when it's done. For the above example,
>  it had to run strip twice. Strip can be slow depending on a variety of factors,
>  so you might need to be a little patient. You can choose to keep the original
> -revisions by passing the ``--keep`` flag.
> +revisions by passing the ``--keep`` flag. This will work more like graft and
> +not preserve/copy the original phase.
>
>  The ``edit`` operation will drop you back to a command prompt,
>  allowing you to edit files freely, or even use ``hg record`` to commit
> @@ -466,7 +467,7 @@ class histeditaction(object):
>          rulectx = repo[self.node]
>
>          editor = self.commiteditor()
> -        commit = commitfuncfor(repo, rulectx)
> +        commit = commitfuncfor(repo, rulectx, self.state.keep)
>
>          commit(text=rulectx.description(), user=rulectx.user(),
>                 date=rulectx.date(), extra=rulectx.extra(), editor=editor)
> @@ -489,7 +490,7 @@ class histeditaction(object):
>              return ctx, []
>          return ctx, [(self.node, (ctx.node(),))]
>
> -def commitfuncfor(repo, src):
> +def commitfuncfor(repo, src, keep):
>      """Build a commit function for the replacement of <src>
>
>      This function ensure we apply the same treatment to all changesets.
> @@ -503,8 +504,9 @@ def commitfuncfor(repo, src):
>      def commitfunc(**kwargs):
>          phasebackup = repo.ui.backupconfig('phases', 'new-commit')
>          try:
> -            repo.ui.setconfig('phases', 'new-commit', phasemin,
> -                              'histedit')
> +            if not keep:
> +                repo.ui.setconfig('phases', 'new-commit', phasemin,
> +                                  'histedit')
>              extra = kwargs.get('extra', {}).copy()
>              extra['histedit_source'] = src.hex()
>              kwargs['extra'] = extra
> @@ -531,7 +533,7 @@ def applychanges(ui, repo, ctx, opts):
>              repo.ui.setconfig('ui', 'forcemerge', '', 'histedit')
>      return stats
>
> -def collapse(repo, first, last, commitopts, skipprompt=False):
> +def collapse(repo, first, last, commitopts, skipprompt=False, keep=False):
>      """collapse the set of revisions from first to last as new one.
>
>      Expected commit options are:
> @@ -545,7 +547,7 @@ def collapse(repo, first, last, commitop
>      if not ctxs:
>          return None
>      for c in ctxs:
> -        if not c.mutable():
> +        if not c.mutable() and not keep:
>              raise error.ParseError(
>                  _("cannot fold into public change %s") % node.short(c.node()))
>      base = first.parents()[0]
> @@ -669,16 +671,15 @@ class fold(histeditaction):
>              return
>          else:
>              c = repo[prev.node]
> -        if not c.mutable():
> +        if not c.mutable() and not self.state.keep:
>              raise error.ParseError(
>                  _("cannot fold into public change %s") % node.short(c.node()))
>
> -
>      def continuedirty(self):
>          repo = self.repo
>          rulectx = repo[self.node]
>
> -        commit = commitfuncfor(repo, rulectx)
> +        commit = commitfuncfor(repo, rulectx, self.state.keep)
>          commit(text='fold-temp-revision %s' % node.short(self.node),
>                 user=rulectx.user(), date=rulectx.date(),
>                 extra=rulectx.extra())
> @@ -752,9 +753,10 @@ class fold(histeditaction):
>          phasebackup = repo.ui.backupconfig('phases', 'new-commit')
>          try:
>              phasemin = max(ctx.phase(), oldctx.phase())
> -            repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit')
> +            if not self.state.keep:
> +                repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit')
>              n = collapse(repo, ctx, repo[newnode], commitopts,
> -                         skipprompt=self.skipprompt())
> +                         skipprompt=self.skipprompt(), keep=self.state.keep)
>          finally:
>              repo.ui.restoreconfig(phasebackup)
>          if n is None:
> diff --git a/tests/test-histedit-edit.t b/tests/test-histedit-edit.t
> --- a/tests/test-histedit-edit.t
> +++ b/tests/test-histedit-edit.t
> @@ -453,6 +453,7 @@ rollback should not work after a histedi
>    $ echo foo >> a
>    $ hg ci -m 'extend a'
>    $ hg phase --public 1
> +
>  Attempting to fold a change into a public change should not work:
>    $ cat > ../edit.sh <<EOF
>    > cat "\$1" | sed s/pick/fold/ > tmp
> @@ -480,3 +481,19 @@ Attempting to fold a change into a publi
>    #  f, fold = use commit, but combine it with the one above
>    #  r, roll = like fold, but discard this commit's description
>    #
> +
> +but it should work more like graft when using --keep
> +
> +  $ HGEDITOR="sh ../edit.sh" hg histedit 2 --keep
> +  saved backup bundle to $TESTTMP/r0/.hg/strip-backup/17b1aa9a4a0b-8d4a5be3-backup.hg (glob)
> +
> +  $ hg log -G -T '{rev} {node|short} {phase} {desc|firstline}'
> +  @  3 311eafe309d8 draft add b
> +  |
> +  | o  2 0012be4a27ea draft extend a
> +  | |
> +  | o  1 18aa70c8ad22 public add b
> +  |/
> +  o  0 0efcea34f18a public a
> +
> +
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pierre-Yves David - Nov. 25, 2016, 1:52 p.m.
On 11/19/2016 02:44 AM, Augie Fackler wrote:
> On Fri, Nov 18, 2016 at 05:16:06PM +0100, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1479485469 -3600
>> #      Fri Nov 18 17:11:09 2016 +0100
>> # Node ID 19ad769b648182e92a49015d85389fe2a8303c18
>> # Parent  1156ec81f70907ff843ca30bb81b4ef59b9b7068
>> histedit: make --keep work more like graft and use default phase for copies
>
> queued, thanks

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.

[1] except when source is public, in that case draft is used.

More comment below.

>> diff --git a/hgext/histedit.py b/hgext/histedit.py
>> --- a/hgext/histedit.py
>> +++ b/hgext/histedit.py
>> @@ -91,7 +91,8 @@ ones) until after it has completed all t
>>  probably perform several strip operations when it's done. For the above example,
>>  it had to run strip twice. Strip can be slow depending on a variety of factors,
>>  so you might need to be a little patient. You can choose to keep the original
>> -revisions by passing the ``--keep`` flag.
>> +revisions by passing the ``--keep`` flag. This will work more like graft and
>> +not preserve/copy the original phase.
>>
>>  The ``edit`` operation will drop you back to a command prompt,
>>  allowing you to edit files freely, or even use ``hg record`` to commit
>> @@ -466,7 +467,7 @@ class histeditaction(object):
>>          rulectx = repo[self.node]
>>
>>          editor = self.commiteditor()
>> -        commit = commitfuncfor(repo, rulectx)
>> +        commit = commitfuncfor(repo, rulectx, self.state.keep)
>>
>>          commit(text=rulectx.description(), user=rulectx.user(),
>>                 date=rulectx.date(), extra=rulectx.extra(), editor=editor)
>> @@ -489,7 +490,7 @@ class histeditaction(object):
>>              return ctx, []
>>          return ctx, [(self.node, (ctx.node(),))]
>>
>> -def commitfuncfor(repo, src):
>> +def commitfuncfor(repo, src, keep):
>>      """Build a commit function for the replacement of <src>
>>
>>      This function ensure we apply the same treatment to all changesets.
>> @@ -503,8 +504,9 @@ def commitfuncfor(repo, src):
>>      def commitfunc(**kwargs):
>>          phasebackup = repo.ui.backupconfig('phases', 'new-commit')
>>          try:
>> -            repo.ui.setconfig('phases', 'new-commit', phasemin,
>> -                              'histedit')
>> +            if not keep:
>> +                repo.ui.setconfig('phases', 'new-commit', phasemin,
>> +                                  'histedit')
>>              extra = kwargs.get('extra', {}).copy()
>>              extra['histedit_source'] = src.hex()
>>              kwargs['extra'] = extra
>> @@ -531,7 +533,7 @@ def applychanges(ui, repo, ctx, opts):
>>              repo.ui.setconfig('ui', 'forcemerge', '', 'histedit')
>>      return stats
>>
>> -def collapse(repo, first, last, commitopts, skipprompt=False):
>> +def collapse(repo, first, last, commitopts, skipprompt=False, keep=False):
>>      """collapse the set of revisions from first to last as new one.
>>
>>      Expected commit options are:
>> @@ -545,7 +547,7 @@ def collapse(repo, first, last, commitop
>>      if not ctxs:
>>          return None
>>      for c in ctxs:
>> -        if not c.mutable():
>> +        if not c.mutable() and not keep:

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.

>>              raise error.ParseError(
>>                  _("cannot fold into public change %s") % node.short(c.node()))
>>      base = first.parents()[0]
>> @@ -669,16 +671,15 @@ class fold(histeditaction):
>>              return
>>          else:
>>              c = repo[prev.node]
>> -        if not c.mutable():
>> +        if not c.mutable() and not self.state.keep:
>>              raise error.ParseError(
>>                  _("cannot fold into public change %s") % node.short(c.node()))
>>
>> -
>>      def continuedirty(self):
>>          repo = self.repo
>>          rulectx = repo[self.node]
>>
>> -        commit = commitfuncfor(repo, rulectx)
>> +        commit = commitfuncfor(repo, rulectx, self.state.keep)
>>          commit(text='fold-temp-revision %s' % node.short(self.node),
>>                 user=rulectx.user(), date=rulectx.date(),
>>                 extra=rulectx.extra())
>> @@ -752,9 +753,10 @@ class fold(histeditaction):
>>          phasebackup = repo.ui.backupconfig('phases', 'new-commit')
>>          try:
>>              phasemin = max(ctx.phase(), oldctx.phase())
>> -            repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit')
>> +            if not self.state.keep:
>> +                repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit')
>>              n = collapse(repo, ctx, repo[newnode], commitopts,
>> -                         skipprompt=self.skipprompt())
>> +                         skipprompt=self.skipprompt(), keep=self.state.keep)
>>          finally:
>>              repo.ui.restoreconfig(phasebackup)
>>          if n is None:
>> diff --git a/tests/test-histedit-edit.t b/tests/test-histedit-edit.t
>> --- a/tests/test-histedit-edit.t
>> +++ b/tests/test-histedit-edit.t
>> @@ -453,6 +453,7 @@ rollback should not work after a histedi
>>    $ echo foo >> a
>>    $ hg ci -m 'extend a'
>>    $ hg phase --public 1
>> +
>>  Attempting to fold a change into a public change should not work:
>>    $ cat > ../edit.sh <<EOF
>>    > cat "\$1" | sed s/pick/fold/ > tmp
>> @@ -480,3 +481,19 @@ Attempting to fold a change into a publi
>>    #  f, fold = use commit, but combine it with the one above
>>    #  r, roll = like fold, but discard this commit's description
>>    #
>> +
>> +but it should work more like graft when using --keep
>> +
>> +  $ HGEDITOR="sh ../edit.sh" hg histedit 2 --keep
>> +  saved backup bundle to $TESTTMP/r0/.hg/strip-backup/17b1aa9a4a0b-8d4a5be3-backup.hg (glob)
>> +
>> +  $ hg log -G -T '{rev} {node|short} {phase} {desc|firstline}'
>> +  @  3 311eafe309d8 draft add b
>> +  |
>> +  | o  2 0012be4a27ea draft extend a
>> +  | |
>> +  | o  1 18aa70c8ad22 public add b
>> +  |/
>> +  o  0 0efcea34f18a public a
>> +
>> +
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Augie Fackler - Nov. 27, 2016, 3:02 p.m.
> On Nov 25, 2016, at 8:52 AM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
> 
> 
> On 11/19/2016 02:44 AM, Augie Fackler wrote:
>> On Fri, Nov 18, 2016 at 05:16:06PM +0100, Mads Kiilerich wrote:
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski@unity3d.com <mailto:madski@unity3d.com>>
>>> # Date 1479485469 -3600
>>> #      Fri Nov 18 17:11:09 2016 +0100
>>> # Node ID 19ad769b648182e92a49015d85389fe2a8303c18
>>> # Parent  1156ec81f70907ff843ca30bb81b4ef59b9b7068
>>> histedit: make --keep work more like graft and use default phase for copies
>> 
>> queued, thanks
> 
> 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.
> 
> [1] except when source is public, in that case draft is used.

IIRC Mads was reporting that it was preserving public phase, which struck me as an obvious bug.

> 
> More comment below.
> 
>>> diff --git a/hgext/histedit.py b/hgext/histedit.py
>>> --- a/hgext/histedit.py
>>> +++ b/hgext/histedit.py
>>> @@ -91,7 +91,8 @@ ones) until after it has completed all t
>>> probably perform several strip operations when it's done. For the above example,
>>> it had to run strip twice. Strip can be slow depending on a variety of factors,
>>> so you might need to be a little patient. You can choose to keep the original
>>> -revisions by passing the ``--keep`` flag.
>>> +revisions by passing the ``--keep`` flag. This will work more like graft and
>>> +not preserve/copy the original phase.
>>> 
>>> The ``edit`` operation will drop you back to a command prompt,
>>> allowing you to edit files freely, or even use ``hg record`` to commit
>>> @@ -466,7 +467,7 @@ class histeditaction(object):
>>>         rulectx = repo[self.node]
>>> 
>>>         editor = self.commiteditor()
>>> -        commit = commitfuncfor(repo, rulectx)
>>> +        commit = commitfuncfor(repo, rulectx, self.state.keep)
>>> 
>>>         commit(text=rulectx.description(), user=rulectx.user(),
>>>                date=rulectx.date(), extra=rulectx.extra(), editor=editor)
>>> @@ -489,7 +490,7 @@ class histeditaction(object):
>>>             return ctx, []
>>>         return ctx, [(self.node, (ctx.node(),))]
>>> 
>>> -def commitfuncfor(repo, src):
>>> +def commitfuncfor(repo, src, keep):
>>>     """Build a commit function for the replacement of <src>
>>> 
>>>     This function ensure we apply the same treatment to all changesets.
>>> @@ -503,8 +504,9 @@ def commitfuncfor(repo, src):
>>>     def commitfunc(**kwargs):
>>>         phasebackup = repo.ui.backupconfig('phases', 'new-commit')
>>>         try:
>>> -            repo.ui.setconfig('phases', 'new-commit', phasemin,
>>> -                              'histedit')
>>> +            if not keep:
>>> +                repo.ui.setconfig('phases', 'new-commit', phasemin,
>>> +                                  'histedit')
>>>             extra = kwargs.get('extra', {}).copy()
>>>             extra['histedit_source'] = src.hex()
>>>             kwargs['extra'] = extra
>>> @@ -531,7 +533,7 @@ def applychanges(ui, repo, ctx, opts):
>>>             repo.ui.setconfig('ui', 'forcemerge', '', 'histedit')
>>>     return stats
>>> 
>>> -def collapse(repo, first, last, commitopts, skipprompt=False):
>>> +def collapse(repo, first, last, commitopts, skipprompt=False, keep=False):
>>>     """collapse the set of revisions from first to last as new one.
>>> 
>>>     Expected commit options are:
>>> @@ -545,7 +547,7 @@ def collapse(repo, first, last, commitop
>>>     if not ctxs:
>>>         return None
>>>     for c in ctxs:
>>> -        if not c.mutable():
>>> +        if not c.mutable() and not keep:
> 
> 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.

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.

> 
>>>             raise error.ParseError(
>>>                 _("cannot fold into public change %s") % node.short(c.node()))
>>>     base = first.parents()[0]
>>> @@ -669,16 +671,15 @@ class fold(histeditaction):
>>>             return
>>>         else:
>>>             c = repo[prev.node]
>>> -        if not c.mutable():
>>> +        if not c.mutable() and not self.state.keep:
>>>             raise error.ParseError(
>>>                 _("cannot fold into public change %s") % node.short(c.node()))
>>> 
>>> -
>>>     def continuedirty(self):
>>>         repo = self.repo
>>>         rulectx = repo[self.node]
>>> 
>>> -        commit = commitfuncfor(repo, rulectx)
>>> +        commit = commitfuncfor(repo, rulectx, self.state.keep)
>>>         commit(text='fold-temp-revision %s' % node.short(self.node),
>>>                user=rulectx.user(), date=rulectx.date(),
>>>                extra=rulectx.extra())
>>> @@ -752,9 +753,10 @@ class fold(histeditaction):
>>>         phasebackup = repo.ui.backupconfig('phases', 'new-commit')
>>>         try:
>>>             phasemin = max(ctx.phase(), oldctx.phase())
>>> -            repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit')
>>> +            if not self.state.keep:
>>> +                repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit')
>>>             n = collapse(repo, ctx, repo[newnode], commitopts,
>>> -                         skipprompt=self.skipprompt())
>>> +                         skipprompt=self.skipprompt(), keep=self.state.keep)
>>>         finally:
>>>             repo.ui.restoreconfig(phasebackup)
>>>         if n is None:
>>> diff --git a/tests/test-histedit-edit.t b/tests/test-histedit-edit.t
>>> --- a/tests/test-histedit-edit.t
>>> +++ b/tests/test-histedit-edit.t
>>> @@ -453,6 +453,7 @@ rollback should not work after a histedi
>>>   $ echo foo >> a
>>>   $ hg ci -m 'extend a'
>>>   $ hg phase --public 1
>>> +
>>> Attempting to fold a change into a public change should not work:
>>>   $ cat > ../edit.sh <<EOF
>>>   > cat "\$1" | sed s/pick/fold/ > tmp
>>> @@ -480,3 +481,19 @@ Attempting to fold a change into a publi
>>>   #  f, fold = use commit, but combine it with the one above
>>>   #  r, roll = like fold, but discard this commit's description
>>>   #
>>> +
>>> +but it should work more like graft when using --keep
>>> +
>>> +  $ HGEDITOR="sh ../edit.sh" hg histedit 2 --keep
>>> +  saved backup bundle to $TESTTMP/r0/.hg/strip-backup/17b1aa9a4a0b-8d4a5be3-backup.hg (glob)
>>> +
>>> +  $ hg log -G -T '{rev} {node|short} {phase} {desc|firstline}'
>>> +  @  3 311eafe309d8 draft add b
>>> +  |
>>> +  | o  2 0012be4a27ea draft extend a
>>> +  | |
>>> +  | o  1 18aa70c8ad22 public add b
>>> +  |/
>>> +  o  0 0efcea34f18a public a
>>> +
>>> +
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel@mercurial-scm.org
>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>> 
> 
> -- 
> Pierre-Yves David

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -91,7 +91,8 @@  ones) until after it has completed all t
 probably perform several strip operations when it's done. For the above example,
 it had to run strip twice. Strip can be slow depending on a variety of factors,
 so you might need to be a little patient. You can choose to keep the original
-revisions by passing the ``--keep`` flag.
+revisions by passing the ``--keep`` flag. This will work more like graft and
+not preserve/copy the original phase.
 
 The ``edit`` operation will drop you back to a command prompt,
 allowing you to edit files freely, or even use ``hg record`` to commit
@@ -466,7 +467,7 @@  class histeditaction(object):
         rulectx = repo[self.node]
 
         editor = self.commiteditor()
-        commit = commitfuncfor(repo, rulectx)
+        commit = commitfuncfor(repo, rulectx, self.state.keep)
 
         commit(text=rulectx.description(), user=rulectx.user(),
                date=rulectx.date(), extra=rulectx.extra(), editor=editor)
@@ -489,7 +490,7 @@  class histeditaction(object):
             return ctx, []
         return ctx, [(self.node, (ctx.node(),))]
 
-def commitfuncfor(repo, src):
+def commitfuncfor(repo, src, keep):
     """Build a commit function for the replacement of <src>
 
     This function ensure we apply the same treatment to all changesets.
@@ -503,8 +504,9 @@  def commitfuncfor(repo, src):
     def commitfunc(**kwargs):
         phasebackup = repo.ui.backupconfig('phases', 'new-commit')
         try:
-            repo.ui.setconfig('phases', 'new-commit', phasemin,
-                              'histedit')
+            if not keep:
+                repo.ui.setconfig('phases', 'new-commit', phasemin,
+                                  'histedit')
             extra = kwargs.get('extra', {}).copy()
             extra['histedit_source'] = src.hex()
             kwargs['extra'] = extra
@@ -531,7 +533,7 @@  def applychanges(ui, repo, ctx, opts):
             repo.ui.setconfig('ui', 'forcemerge', '', 'histedit')
     return stats
 
-def collapse(repo, first, last, commitopts, skipprompt=False):
+def collapse(repo, first, last, commitopts, skipprompt=False, keep=False):
     """collapse the set of revisions from first to last as new one.
 
     Expected commit options are:
@@ -545,7 +547,7 @@  def collapse(repo, first, last, commitop
     if not ctxs:
         return None
     for c in ctxs:
-        if not c.mutable():
+        if not c.mutable() and not keep:
             raise error.ParseError(
                 _("cannot fold into public change %s") % node.short(c.node()))
     base = first.parents()[0]
@@ -669,16 +671,15 @@  class fold(histeditaction):
             return
         else:
             c = repo[prev.node]
-        if not c.mutable():
+        if not c.mutable() and not self.state.keep:
             raise error.ParseError(
                 _("cannot fold into public change %s") % node.short(c.node()))
 
-
     def continuedirty(self):
         repo = self.repo
         rulectx = repo[self.node]
 
-        commit = commitfuncfor(repo, rulectx)
+        commit = commitfuncfor(repo, rulectx, self.state.keep)
         commit(text='fold-temp-revision %s' % node.short(self.node),
                user=rulectx.user(), date=rulectx.date(),
                extra=rulectx.extra())
@@ -752,9 +753,10 @@  class fold(histeditaction):
         phasebackup = repo.ui.backupconfig('phases', 'new-commit')
         try:
             phasemin = max(ctx.phase(), oldctx.phase())
-            repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit')
+            if not self.state.keep:
+                repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit')
             n = collapse(repo, ctx, repo[newnode], commitopts,
-                         skipprompt=self.skipprompt())
+                         skipprompt=self.skipprompt(), keep=self.state.keep)
         finally:
             repo.ui.restoreconfig(phasebackup)
         if n is None:
diff --git a/tests/test-histedit-edit.t b/tests/test-histedit-edit.t
--- a/tests/test-histedit-edit.t
+++ b/tests/test-histedit-edit.t
@@ -453,6 +453,7 @@  rollback should not work after a histedi
   $ echo foo >> a
   $ hg ci -m 'extend a'
   $ hg phase --public 1
+
 Attempting to fold a change into a public change should not work:
   $ cat > ../edit.sh <<EOF
   > cat "\$1" | sed s/pick/fold/ > tmp
@@ -480,3 +481,19 @@  Attempting to fold a change into a publi
   #  f, fold = use commit, but combine it with the one above
   #  r, roll = like fold, but discard this commit's description
   #
+
+but it should work more like graft when using --keep
+
+  $ HGEDITOR="sh ../edit.sh" hg histedit 2 --keep
+  saved backup bundle to $TESTTMP/r0/.hg/strip-backup/17b1aa9a4a0b-8d4a5be3-backup.hg (glob)
+
+  $ hg log -G -T '{rev} {node|short} {phase} {desc|firstline}'
+  @  3 311eafe309d8 draft add b
+  |
+  | o  2 0012be4a27ea draft extend a
+  | |
+  | o  1 18aa70c8ad22 public add b
+  |/
+  o  0 0efcea34f18a public a
+  
+