This is indeed a very important quality-of-life feature and I'm +1 on adding it to Mercurial.

I have a few questions/remarks however:

• Just to make sure we're on the same page, you're suggesting to auto-rebase the stack i.e. what's between the commit to amend and the current working directory parent and nothing more, right? We probably want a non-default mode without the auto-rebase, I can see that being useful.
• I remember sprint discussions wherein people warned that the semantics of --rev were not clear in the context of amend and suggested using something like --into-rev. I personally think --rev feels natural to me and I don't have strong objections to using it, but we should maybe ask the community (with a poll?) and keep the option open while implementing it.
• Maybe plug into hg continue/abort and suggest using it first, then hg amend --continue/abort?

As a nit, I'd say the .t test should be using (known-bad-output !) and/or (missing-correct-output !) to better signal what is expected than the TODO.

In D10893#166662, @Alphare wrote:

This is indeed a very important quality-of-life feature and I'm +1 on adding it to Mercurial.

Thanks for the quick review. I sent this out early to get feedback on the idea -- particularly on the fact that we'll have hg amend --continue and hg amend --abort -- before I wasted time going too far in the wrong direction.

I have a few questions/remarks however:

• Just to make sure we're on the same page, you're suggesting to auto-rebase the stack i.e. what's between the commit to amend and the current working directory parent and nothing more, right? We probably want a non-default mode without the auto-rebase, I can see that being useful.

Yes, exactly, I had intended for it to auto-rebase. Sure, I can add a --no-rebase flag too. When would you want that?

By the way, I suppose we may want a hg amend --stop as well, but I probably won't implement that any time soon.

• I remember sprint discussions wherein people warned that the semantics of --rev were not clear in the context of amend and suggested using something like --into-rev. I personally think --rev feels natural to me and I don't have strong objections to using it, but we should maybe ask the community (with a poll?) and keep the option open while implementing it.

I initially called it hg amend --into, but then someone (in an unrelated discussion) mentioned hg amend -i yesterday and I thought that users might confuse hg amend --into and hg amend -i. I think most users think of the feature as hg amend -r, so I switched to that.

My reason for initially calling it --into is that the command works on two revisions: the working copy and the target commit. Most hg commands default to working on the working copy and use -r to change that (for example, hg branch and hg topic change the branch name in the working copy by default, but you can change the branch/topic of other commits by passing -r). hg amend defaults to amending from the working copy and into the parent of the working copy. It seems unlikely that we'll add support for amending from an arbitrary commit into an ancestor of that commit. If we ever do that, we can make that hg amend --from abc123 --into abc123^^ and keep -r as a deprecated alias for --into.

Sounds okay?

• Maybe plug into hg continue/abort and suggest using it first, then hg amend --continue/abort?

As a nit, I'd say the .t test should be using (known-bad-output !) and/or (missing-correct-output !) to better signal what is expected than the TODO.

Ah, good point. Those are new enough that I haven't gotten used to using them yet. Will update the patch.

In D10893#166666, @martinvonz wrote:

Thanks for the quick review. I sent this out early to get feedback on the idea -- particularly on the fact that we'll have hg amend --continue and hg amend --abort -- before I wasted time going too far in the wrong direction.

Now that I think of it I'm not entirely certain what the intermediate step for amend will be used for. Will it just be in case of rebase conflicts or is there more to it?

Yes, exactly, I had intended for it to auto-rebase.

I meant to clarify that it would not auto-rebase more than the stack (like other heads that are descendants of the amended changeset).

Sure, I can add a --no-rebase flag too. When would you want that?

This would be for small adjustments that have no real impact on what I'm currently working on, I can just throw the amends one after the other. It's pretty niche, but not hard to implement so I thought I'd ask.

By the way, I suppose we may want a hg amend --stop as well, but I probably won't implement that any time soon.

That would be a good idea, maybe we could discuss why it's hard to implement it (if it's the case), but that shouldn't be a blocker for this patch.

I initially called it hg amend --into, but then someone (in an unrelated discussion) mentioned hg amend -i yesterday and I thought that users might confuse hg amend --into and hg amend -i. I think most users think of the feature as hg amend -r, so I switched to that.

Good point.

My reason for initially calling it --into is that the command works on two revisions: the working copy and the target commit. Most hg commands default to working on the working copy and use -r to change that (for example, hg branch and hg topic change the branch name in the working copy by default, but you can change the branch/topic of other commits by passing -r). hg amend defaults to amending from the working copy and into the parent of the working copy. It seems unlikely that we'll add support for amending from an arbitrary commit into an ancestor of that commit. If we ever do that, we can make that hg amend --from abc123 --into abc123^^ and keep -r as a deprecated alias for --into.
Sounds okay?

This sounds okay to me.

• Maybe plug into hg continue/abort and suggest using it first, then hg amend --continue/abort?

As a nit, I'd say the .t test should be using (known-bad-output !) and/or (missing-correct-output !) to better signal what is expected than the TODO.

Ah, good point. Those are new enough that I haven't gotten used to using them yet. Will update the patch.

Thanks for updating the patch. I'm going to let a few days go by just to let people give their thoughts as this is a pretty impactful feature, but this looks good to me overall.

In D10893#166668, @Alphare wrote:

In D10893#166666, @martinvonz wrote:

Thanks for the quick review. I sent this out early to get feedback on the idea -- particularly on the fact that we'll have hg amend --continue and hg amend --abort -- before I wasted time going too far in the wrong direction.

Now that I think of it I'm not entirely certain what the intermediate step for amend will be used for. Will it just be in case of rebase conflicts or is there more to it?

Steps:

1. Create temporary commit. Once we support hg amend -r target -i, we'll need to create *two* temporary commits -- one to be rebased and folded in to the target, and one for the leftover changes to later be uncommitted and left in the working copy.
2. Rebase the temporary commit on top of the target
3. Fold the temporary commit into the target (using rebase --collapse)
4. Rebase descendants on top of the new folded commit
5. Update to the new tip commit
6. Uncommit the temporary working copy if there was one

Conflicts can happen in step 2 or step 4.

Yes, exactly, I had intended for it to auto-rebase.

I meant to clarify that it would not auto-rebase more than the stack (like other heads that are descendants of the amended changeset).

I see. Yes, that's what I meant. It would rebase the commits between the target and the working copy parent. It would not rebase other descendants of the target. It would also not rebase descendants of the working copy parent.

Sure, I can add a --no-rebase flag too. When would you want that?

This would be for small adjustments that have no real impact on what I'm currently working on, I can just throw the amends one after the other. It's pretty niche, but not hard to implement so I thought I'd ask.

I'm not sure I follow. Let's say you have this history:

@ C
|
o B
|
o A

You then make some change and run hg amend -r B --no-rebase. You get this:

@ C
|
x B
|
| B'
|/
o A

Presumably the working copy is now clean again? However, that means that you can't make changes that build on top of the previous amend.

You then make some further changes in the working copy. What do you do now? If you do hg amend -r B, you would presumably get divergence. Should we allow hg amend -r B'? I had been thinking that we restrict hg amend -r to only amend into an ancestor just to keep it simple and because that seems like what people want in 99% of cases.

By the way, I suppose we may want a hg amend --stop as well, but I probably won't implement that any time soon.

That would be a good idea, maybe we could discuss why it's hard to implement it (if it's the case), but that shouldn't be a blocker for this patch.

It might be easy and I might do it if it seems easy.

• Maybe plug into hg continue/abort and suggest using it first, then hg amend --continue/abort?

Sorry, didn't see this comment earlier. That's what the patch currently does :) Well, there's no support for continuing, but the test case uses hg abort and there's no --abort flag yet. (I just noticed that the hg status -v still refers to hg amend --continue/--abort. I'll try to figure out how to make it not do that.)

As a nit, I'd say the .t test should be using (known-bad-output !) and/or (missing-correct-output !) to better signal what is expected than the TODO.

Ah, good point. Those are new enough that I haven't gotten used to using them yet. Will update the patch.

Thanks for updating the patch. I'm going to let a few days go by just to let people give their thoughts as this is a pretty impactful feature, but this looks good to me overall.

Makes sense. I'll work on making the command actually do something useful in the meantime :)

In D10893#166666, @martinvonz wrote:

In D10893#166662, @Alphare wrote:

This is indeed a very important quality-of-life feature and I'm +1 on adding it to Mercurial.

+1

• I remember sprint discussions wherein people warned that the semantics of --rev were not clear in the context of amend and suggested using something like --into-rev. I personally think --rev feels natural to me and I don't have strong objections to using it, but we should maybe ask the community (with a poll?) and keep the option open while implementing it.

I initially called it hg amend --into, but then someone (in an unrelated discussion) mentioned hg amend -i yesterday and I thought that users might confuse hg amend --into and hg amend -i. I think most users think of the feature as hg amend -r, so I switched to that.
My reason for initially calling it --into is that the command works on two revisions: the working copy and the target commit. Most hg commands default to working on the working copy and use -r to change that (for example, hg branch and hg topic change the branch name in the working copy by default, but you can change the branch/topic of other commits by passing -r). hg amend defaults to amending from the working copy and into the parent of the working copy. It seems unlikely that we'll add support for amending from an arbitrary commit into an ancestor of that commit. If we ever do that, we can make that hg amend --from abc123 --into abc123^^ and keep -r as a deprecated alias for --into.
Sounds okay?

-r seems reasonable to me. The --from abc --into xyz form sounds more like a non contiguous hg fold to me. I haven't given any thought to if adding that functionality there would make the UI too awkward, given that you can select multiple source revs to fold.

The other general thought I had on this is maybe absorb functionality can be leveraged to avoid the merge conflicts when rebasing, but I don't understand that code at all in order to help. I suppose you'd need to have the "absorb this line into a specific revision" functionality implemented that's been talked about before. But the functionality in general seems more absorb-y than amend-y to me.

In D10893#166678, @mharbison72 wrote:

In D10893#166666, @martinvonz wrote:

In D10893#166662, @Alphare wrote:

This is indeed a very important quality-of-life feature and I'm +1 on adding it to Mercurial.

+1

• I remember sprint discussions wherein people warned that the semantics of --rev were not clear in the context of amend and suggested using something like --into-rev. I personally think --rev feels natural to me and I don't have strong objections to using it, but we should maybe ask the community (with a poll?) and keep the option open while implementing it.

I initially called it hg amend --into, but then someone (in an unrelated discussion) mentioned hg amend -i yesterday and I thought that users might confuse hg amend --into and hg amend -i. I think most users think of the feature as hg amend -r, so I switched to that.
My reason for initially calling it --into is that the command works on two revisions: the working copy and the target commit. Most hg commands default to working on the working copy and use -r to change that (for example, hg branch and hg topic change the branch name in the working copy by default, but you can change the branch/topic of other commits by passing -r). hg amend defaults to amending from the working copy and into the parent of the working copy. It seems unlikely that we'll add support for amending from an arbitrary commit into an ancestor of that commit. If we ever do that, we can make that hg amend --from abc123 --into abc123^^ and keep -r as a deprecated alias for --into.
Sounds okay?

-r seems reasonable to me. The --from abc --into xyz form sounds more like a non contiguous hg fold to me. I haven't given any thought to if adding that functionality there would make the UI too awkward, given that you can select multiple source revs to fold.

Well, hg amend and hg fold sound the same to me :) To me, the difference seems to be that hg fold is about folding many commits, while hg amend is about only two. Also, adding the feature to hg fold is going to be more work because it's not in core.

The other general thought I had on this is maybe absorb functionality can be leveraged to avoid the merge conflicts when rebasing, but I don't understand that code at all in order to help. I suppose you'd need to have the "absorb this line into a specific revision" functionality implemented that's been talked about before. But the functionality in general seems more absorb-y than amend-y to me.

I suspect it would be hard to convince absorb to do this operation. As you said, the "absorb this line into a specific revision" functionality doesn't exist. I think it will be much easier to use rebase internally for this.

In D10893#166684, @martinvonz wrote:

In D10893#166678, @mharbison72 wrote:

-r seems reasonable to me. The --from abc --into xyz form sounds more like a non contiguous hg fold to me. I haven't given any thought to if adding that functionality there would make the UI too awkward, given that you can select multiple source revs to fold.

Well, hg amend and hg fold sound the same to me :) To me, the difference seems to be that hg fold is about folding many commits, while hg amend is about only two.

Ah, OK. The distinction to me is hg amend works on wdir() + . and hg fold works on 2 or more commits. This is just extending the former's . to ::.. This doesn't propose to handle --from abc --into xyz- all I meant was if it does become a thing some day, it sounds like maybe a different command anyway. I could also see the -r arg here accepting multiple revisions when --interactive is given someday, to allow the user to amend in bits and pieces while rebuilding the stack one commit at a time, and that would be outside your definition of hg amend. I'd still expect that to be the same command as this, but I'm not trying to feature creep it :-)

Also, adding the feature to hg fold is going to be more work because it's not in core.

Hmm, yeah. Are you planning on implementing the --from abc --into xyz case in the near term? I probably just wasn't clear enough that I thought this (currently out of scope) functionality is what might belong elsewhere.

The other general thought I had on this is maybe absorb functionality can be leveraged to avoid the merge conflicts when rebasing, but I don't understand that code at all in order to help. I suppose you'd need to have the "absorb this line into a specific revision" functionality implemented that's been talked about before. But the functionality in general seems more absorb-y than amend-y to me.

I suspect it would be hard to convince absorb to do this operation. As you said, the "absorb this line into a specific revision" functionality doesn't exist. I think it will be much easier to use rebase internally for this.

Yeah, after I hit "send", I wondered if absorbing to an ancestor of a commit that last changed the line being absorbed would cause merge conflicts, and require extra work.

At any rate, please don't take any of this as a NACK or discouragement- I think the functionality is useful in whatever form. I'm just thinking out loud because I'm not sure what all is planned, and trying to get the right UI metaphor in my mind. Since it's marked experimental, if people eventually feel it's more related to absorb from the user POV, we could just dispatch that arg on absorb to the rebase-based implementation you're working on, similar to how the amend command is routing it to a completely different implementation here.

In D10893#166756, @mharbison72 wrote:

In D10893#166684, @martinvonz wrote:

In D10893#166678, @mharbison72 wrote:

Also, adding the feature to hg fold is going to be more work because it's not in core.

Hmm, yeah. Are you planning on implementing the --from abc --into xyz case in the near term? I probably just wasn't clear enough that I thought this (currently out of scope) functionality is what might belong elsewhere.

Nope, no plan to do that. I just used it to explain the argument that hg amend operates on two "commits" (the working copy and another commit) and it would be consistent with at least hg branch and hg topic for hg amend -r to be about which commit to use instead of the working copy (presumably moving the changes from that commit into its parent).

The other general thought I had on this is maybe absorb functionality can be leveraged to avoid the merge conflicts when rebasing, but I don't understand that code at all in order to help. I suppose you'd need to have the "absorb this line into a specific revision" functionality implemented that's been talked about before. But the functionality in general seems more absorb-y than amend-y to me.

I suspect it would be hard to convince absorb to do this operation. As you said, the "absorb this line into a specific revision" functionality doesn't exist. I think it will be much easier to use rebase internally for this.

Yeah, after I hit "send", I wondered if absorbing to an ancestor of a commit that last changed the line being absorbed would cause merge conflicts, and require extra work.
At any rate, please don't take any of this as a NACK or discouragement- I think the functionality is useful in whatever form. I'm just thinking out loud because I'm not sure what all is planned, and trying to get the right UI metaphor in my mind. Since it's marked experimental, if people eventually feel it's more related to absorb from the user POV, we could just dispatch that arg on absorb to the rebase-based implementation you're working on, similar to how the amend command is routing it to a completely different implementation here.

Sure, no problem :) We have talked internally at work about some kind of interactive hg absorb that lets you decide which commit each line should go into. That sounds similar to what you're thinking of.

In D10893#166757, @martinvonz wrote:

We have talked internally at work about some kind of interactive hg absorb that lets you decide which commit each line should go into. That sounds similar to what you're thinking of.

Yes, possibly. There's already a -i mode, but it seems to just be a pre-filter for what gets run through the absorb algorithm. I'm not sure what revision selection in that mode would look like. I found the -e mode that allows selecting the revisions to be a bit daunting the first few times I used it, and it doesn't allow resolving ambiguity for lines that could be applied to multiple revisions.

In D10893#166669, @martinvonz wrote:

In D10893#166668, @Alphare wrote:

In D10893#166666, @martinvonz wrote:

Sure, I can add a --no-rebase flag too. When would you want that?

This would be for small adjustments that have no real impact on what I'm currently working on, I can just throw the amends one after the other. It's pretty niche, but not hard to implement so I thought I'd ask.

I'm not sure I follow. Let's say you have this history:

@ C
|
o B
|
o A

You then make some change and run hg amend -r B --no-rebase. You get this:

@ C
|
x B
|
| B'
|/
o A

Presumably the working copy is now clean again? However, that means that you can't make changes that build on top of the previous amend.
You then make some further changes in the working copy. What do you do now? If you do hg amend -r B, you would presumably get divergence. Should we allow hg amend -r B'? I had been thinking that we restrict hg amend -r to only amend into an ancestor just to keep it simple and because that seems like what people want in 99% of cases.

There is a bunch of small usecase for --no-rebase (that should probably be named --no-evolve

In simple case, the user knows he will amend various bits of the current working copy in different commit of the stack and prefer to run the evolution only once (to reduce conflict resolution and/or to reduce markers creation).

In more advanced case, this might be actually needed to avoid a loop of complicated and unnecessary conflict resolution. Lets imagine the following scenario:

@ C
|
o B
|
o A

• There are change to amend in A, but they will need adjustement to actually work in A and they will conflict with some content in B (then C)
• There are change to amend in B, but they will conflict with the change in A and with some change in C

Without --no-evolve, this would mean the following operation

• hg amend -i --rev A → this requires conflict resolution when evolving B, then C; conflict is resolved with non final content
• hg amend -i --rev B → this requires conflict resolution when evolving C; conflict is resolved with non final content
• hg update A; hack hack hack; hg amend; hg next this requires conflict resolution when evolving B,
• hack hack hack; hg amend; hg next this requires conflict resolution when evolving C,

With --no-evolve, This can simply be:

• hg amend -i --rev A --no-evolve → no conflict resolution yet
• hg amend -i --rev B --no-evolve → no conflict resolution yet
• hg update A; hack hack hack; hg amend; hg next this requires conflict resolution when evolving B; with final content
• hack hack hack; hg amend; hg next this requires conflict resolution when evolving C,

So the --no-evolve reduced the number of conflict and all the conflict we had to resolve were "meaningful", using "final content"

In D10893#166881, @marmoute wrote:

In D10893#166669, @martinvonz wrote:

In D10893#166668, @Alphare wrote:

In D10893#166666, @martinvonz wrote:

Sure, I can add a --no-rebase flag too. When would you want that?

This would be for small adjustments that have no real impact on what I'm currently working on, I can just throw the amends one after the other. It's pretty niche, but not hard to implement so I thought I'd ask.

I'm not sure I follow. Let's say you have this history:

@ C
|
o B
|
o A

You then make some change and run hg amend -r B --no-rebase. You get this:

@ C
|
x B
|
| B'
|/
o A

Presumably the working copy is now clean again? However, that means that you can't make changes that build on top of the previous amend.
You then make some further changes in the working copy. What do you do now? If you do hg amend -r B, you would presumably get divergence. Should we allow hg amend -r B'? I had been thinking that we restrict hg amend -r to only amend into an ancestor just to keep it simple and because that seems like what people want in 99% of cases.

There is a bunch of small usecase for --no-rebase (that should probably be named --no-evolve
In simple case, the user knows he will amend various bits of the current working copy in different commit of the stack and prefer to run the evolution only once (to reduce conflict resolution and/or to reduce markers creation).
In more advanced case, this might be actually needed to avoid a loop of complicated and unnecessary conflict resolution. Lets imagine the following scenario:

@ C
|
o B
|
o A

• There are change to amend in A, but they will need adjustement to actually work in A and they will conflict with some content in B (then C)
• There are change to amend in B, but they will conflict with the change in A and with some change in C

Without --no-evolve, this would mean the following operation

• hg amend -i --rev A → this requires conflict resolution when evolving B, then C; conflict is resolved with non final content
• hg amend -i --rev B → this requires conflict resolution when evolving C; conflict is resolved with non final content
• hg update A; hack hack hack; hg amend; hg next this requires conflict resolution when evolving B,
• hack hack hack; hg amend; hg next this requires conflict resolution when evolving C,

With --no-evolve, This can simply be:

• hg amend -i --rev A --no-evolve → no conflict resolution yet

I imagine that the selected changes are removed from the working copy after this step. Is that what you were thinking as well? Or were you thinking that they would remain in the working copy as well (more like hg rebase --keep if you think of the working copy contents as a commit)?

• hg amend -i --rev B --no-evolve → no conflict resolution yet
• hg update A; hack hack hack; hg amend; hg next this requires conflict resolution when evolving B; with final content
• hack hack hack; hg amend; hg next this requires conflict resolution when evolving C,

So the --no-evolve reduced the number of conflict and all the conflict we had to resolve were "meaningful", using "final content"

Makes sense. On the other hand, if the changes you want to amend into A require you to edit the patch (pressing "e" in the curses UI), then it seems like you'd get additional conflicts by using --no-rebase/--no-evolve. So both --rebase and --no-rebase seem to be useful for avoiding conflicts in some cases.

In D10893#166899, @martinvonz wrote:

In D10893#166881, @marmoute wrote:

In D10893#166669, @martinvonz wrote:

In D10893#166668, @Alphare wrote:

In D10893#166666, @martinvonz wrote:

With --no-evolve, This can simply be:

• hg amend -i --rev A --no-evolve → no conflict resolution yet

I imagine that the selected changes are removed from the working copy after this step. Is that what you were thinking as well? Or were you thinking that they would remain in the working copy as well (more like hg rebase --keep if you think of the working copy contents as a commit)?

That is a very good question, that I have not been able to have a clear opinion on so far. (I did not though about it too much either).

I can see argument for both, "if it is integrated somewhere else, it should no longer be in the wdir() diff" and "This is asking for trouble, we better leave it around".

In the usecase were a small type fix is cast-away in an ancestor and the change is not really relevant to the WC, dropping it is "cleaner" and won't introduce conflict.
On the other hand If the usecase is to send something more conflictful and/or more relevant to the WC, keeping it around might make sense ?

I suggest we start with the path of least resistance (probably keeping it in the WC then) and add a # XXX TODO comment near the experimental config so that we don't forget about that before getting the feature out of experiment.

• hg amend -i --rev B --no-evolve → no conflict resolution yet
• hg update A; hack hack hack; hg amend; hg next this requires conflict resolution when evolving B; with final content
• hack hack hack; hg amend; hg next this requires conflict resolution when evolving C,

So the --no-evolve reduced the number of conflict and all the conflict we had to resolve were "meaningful", using "final content"

Makes sense. On the other hand, if the changes you want to amend into A require you to edit the patch (pressing "e" in the curses UI), then it seems like you'd get additional conflicts by using --no-rebase/--no-evolve. So both --rebase and --no-rebase seem to be useful for avoiding conflicts in some cases.

note: In the case I describe above, this would not be simply a "on the fly patch edition" (pressing "e" in the curses UI) since this would probably involves running tests and writing possibly other codes to make it work fine.

I don't think I understand how the "e" editing can introduce more conflict. Can you elaborate ?

To some extend, my opinion on --no-evolve is that there will certainly be case where it make sense to do less work, and we should have the option around to make sure we can handle these case (and get a better understanding of what make sens in these cases)

In D10893#166901, @marmoute wrote:

In D10893#166899, @martinvonz wrote:

In D10893#166881, @marmoute wrote:

In D10893#166669, @martinvonz wrote:

In D10893#166668, @Alphare wrote:

In D10893#166666, @martinvonz wrote:

With --no-evolve, This can simply be:

• hg amend -i --rev A --no-evolve → no conflict resolution yet

I imagine that the selected changes are removed from the working copy after this step. Is that what you were thinking as well? Or were you thinking that they would remain in the working copy as well (more like hg rebase --keep if you think of the working copy contents as a commit)?

That is a very good question, that I have not been able to have a clear opinion on so far. (I did not though about it too much either).
I can see argument for both, "if it is integrated somewhere else, it should no longer be in the wdir() diff" and "This is asking for trouble, we better leave it around".
In the usecase were a small type fix is cast-away in an ancestor and the change is not really relevant to the WC, dropping it is "cleaner" and won't introduce conflict.
On the other hand If the usecase is to send something more conflictful and/or more relevant to the WC, keeping it around might make sense ?
I suggest we start with the path of least resistance (probably keeping it in the WC then) and add a # XXX TODO comment near the experimental config so that we don't forget about that before getting the feature out of experiment.

I'll start with not even having the flag :P When automatically rebasing the descendants, it seems more clear that the changes should not be left in the working copy. I was planning on explicitly rebasing them and not leaving them in the working copy, not relying on them disappearing only because the changes were already present in an ancestor afterwards. To clarify, I was planning for it to do this:

1. Create temporary commit from working copy
2. Rebase temporary commit to target (not using --keep)
3. Fold the rebased commit into the target
4. Rebase descendants, not including the temporary commit from step 1

One could imagine using --keep in step two and include the temporary commit in step 4 and just rely on it becoming empty. However, if there were conflicts to resolve in step 2, then you'd run into conflicts again in step 4. It seems better to avoid that.

• hg amend -i --rev B --no-evolve → no conflict resolution yet
• hg update A; hack hack hack; hg amend; hg next this requires conflict resolution when evolving B; with final content
• hack hack hack; hg amend; hg next this requires conflict resolution when evolving C,

So the --no-evolve reduced the number of conflict and all the conflict we had to resolve were "meaningful", using "final content"

Makes sense. On the other hand, if the changes you want to amend into A require you to edit the patch (pressing "e" in the curses UI), then it seems like you'd get additional conflicts by using --no-rebase/--no-evolve. So both --rebase and --no-rebase seem to be useful for avoiding conflicts in some cases.

note: In the case I describe above, this would not be simply a "on the fly patch edition" (pressing "e" in the curses UI) since this would probably involves running tests and writing possibly other codes to make it work fine.
I don't think I understand how the "e" editing can introduce more conflict. Can you elaborate ?

It can introduce more conflicts under the assumption above (that at least I had) about hg amend -r not leaving the changes in the working copy as well. Let's say you have some file that contains content "a" in commit A. The file is unchanged between A and the working copy parent. Your working copy now contains "c" in this file. You realize that the contents in commit A should have been "b", so you run hg amend -r A -i and edit the diff from "change 'a' to 'c'" to "change 'a' to 'b'". When the user then confirms their selection, I was planning to create a commit for "change 'a' to 'b'" and another commit on top for "change 'b' to 'c'" (just like how hg ci -i && hg ci would create two commits). I was planning for the first change to go through steps 2 and 3 and 4 from above. However, the "change 'b' to 'c'" bit would also be rebased in step 4 and would apply cleanly. If we leave the changes in the working copy as well, then they'd conflict when they were rebased onto the folded commit (whether that rebasing was done by hg amend -r A -i --rebase or by hg amend -r A -i; hg evolve).

This series is now ready for review again

In D10893#168955, @martinvonz wrote:

This series is now ready for review again

I'll try to have a look soon (realistically Thursday, in all cases this week).

In D10893#166902, @martinvonz wrote:

I'll start with not even having the flag :P When automatically rebasing the descendants, it seems more clear that the changes should not be left in the working copy. I was planning on explicitly rebasing them and not leaving them in the working copy, not relying on them disappearing only because the changes were already present in an ancestor afterwards. To clarify, I was planning for it to do this:

1. Create temporary commit from working copy
2. Rebase temporary commit to target (not using --keep)
3. Fold the rebased commit into the target
4. Rebase descendants, not including the temporary commit from step 1

One could imagine using --keep in step two and include the temporary commit in step 4 and just rely on it becoming empty. However, if there were conflicts to resolve in step 2, then you'd run into conflicts again in step 4. It seems better to avoid that.

Looks like we could have a --no-evolve flag that stop after step 3 and be good. This should fit well with what seems to be the current split of your patch. Also I am not sure why you need a temporary commit here. We should be able to have:

1. update to target,
2. amend the target,
3. evolve affected range (unless --no-evolve) is specified,

(note: further discussion got me to have a more idea about what could be done for --no-evolve, but this the above proposal is a good example a "minimal/simple" approach that still make sense, so I am keeping the comment)

• hg amend -i --rev B --no-evolve → no conflict resolution yet
• hg update A; hack hack hack; hg amend; hg next this requires conflict resolution when evolving B; with final content
• hack hack hack; hg amend; hg next this requires conflict resolution when evolving C,

So the --no-evolve reduced the number of conflict and all the conflict we had to resolve were "meaningful", using "final content"

Makes sense. On the other hand, if the changes you want to amend into A require you to edit the patch (pressing "e" in the curses UI), then it seems like you'd get additional conflicts by using --no-rebase/--no-evolve. So both --rebase and --no-rebase seem to be useful for avoiding conflicts in some cases.

note: In the case I describe above, this would not be simply a "on the fly patch edition" (pressing "e" in the curses UI) since this would probably involves running tests and writing possibly other codes to make it work fine.
I don't think I understand how the "e" editing can introduce more conflict. Can you elaborate ?

It can introduce more conflicts under the assumption above (that at least I had) about hg amend -r not leaving the changes in the working copy as well. Let's say you have some file that contains content "a" in commit A. The file is unchanged between A and the working copy parent. Your working copy now contains "c" in this file. You realize that the contents in commit A should have been "b", so you run hg amend -r A -i and edit the diff from "change 'a' to 'c'" to "change 'a' to 'b'".

That's a pretty advance usecases but that shed some light how what the hg amend core behavior is: alter content of underlying changeset, do not alter¹ the working copy. This apply to both hg amend and hg amend --extract (also known as uncommit). So whatever we goes for should probably leave the working copy untouched.

([1] note: this end up being untrue for hg amend --patch, but that is a quite special case, and we should probably get a hg amend --patch --rev 'wc()' working too.)

When the user then confirms their selection, I was planning to create a commit for "change 'a' to 'b'" and another commit on top for "change 'b' to 'c'" (just like how hg ci -i && hg ci would create two commits).

I am not why we are creating explicit commit here, especially since this seems to introduce more complication than a "preserve the working copy content" approach.

> I was planning for the first change to go through steps 2 and 3 and 4 from above. However, the "change 'b' to 'c'" bit would also be rebased in step 4 and would apply cleanly. If we leave the changes in the working copy as well, then they'd conflict when they were rebased onto the folded commit (whether that rebasing was done by hg amend -r A -i --rebase or by hg amend -r A -i; hg evolve).

Thanks, I now see where the conflict comes from. However I don't see what would be the alternative to "leave the changes in the working copy" here ? Since current working copy content (mostly the final target) is different from what we amended into A.

Can you clarify what are the different alternative you have in mind here ?

In D10893#170047, @marmoute wrote:

In D10893#166902, @martinvonz wrote:

I'll start with not even having the flag :P When automatically rebasing the descendants, it seems more clear that the changes should not be left in the working copy. I was planning on explicitly rebasing them and not leaving them in the working copy, not relying on them disappearing only because the changes were already present in an ancestor afterwards. To clarify, I was planning for it to do this:

1. Create temporary commit from working copy
2. Rebase temporary commit to target (not using --keep)
3. Fold the rebased commit into the target
4. Rebase descendants, not including the temporary commit from step 1

One could imagine using --keep in step two and include the temporary commit in step 4 and just rely on it becoming empty. However, if there were conflicts to resolve in step 2, then you'd run into conflicts again in step 4. It seems better to avoid that.

Looks like we could have a --no-evolve flag that stop after step 3 and be good.

Sure, but that can also be added after the series. I agree about adding such a flag (I'm not sure if I think it should be --no-evolve or --no-rebase), but I don't think it's important to get it added in this initial series.

This should fit well with what seems to be the current split of your patch. Also I am not sure why you need a temporary commit here. We should be able to have:

1. update to target,

The problem is that this step may result in conflicts, and we don't think we have a good way of dealing with that. It seems much easier to create the temporary commit. When we add support for hg amend -i -r, we will create two temporary commits.

2. amend the target,
3. evolve affected range (unless --no-evolve) is specified,

(note: further discussion got me to have a more idea about what could be done for --no-evolve, but this the above proposal is a good example a "minimal/simple" approach that still make sense, so I am keeping the comment)

the hg amend core behavior is: alter content of underlying changeset, do not alter¹ the working copy.

(It looks like you forgot to include footnote [1] that you point to here.)

Yes, that's true for plain hg amend. Note that it's not necessarily true (or at least it shouldn't necessarily be true in my opinion) for hg amend -r. For example, let's say you have this history:

@ commit 3, file content: A
|
o commit 2, file content: B
|
o commit 1, file content: A

You now realize that you want the contents in commit 1 to be "B", so you make that change in the working copy and run hg amend -r 1. I think users wouldn't expect "B" to remain in the working copy afterwards.

This apply to bot hg amend and hg amend --extract (also known as uncommit). So whatever we goes for should probably leave the working copy untouched.
(note: this end up being untrue for hg amend --patch, but that is a quite special case, and we should probably get a hg amend --patch --rev 'wc()' working too.)

(I have never used hg amend --patch. Context for others: hg amend --patch and hg amend --extract exist in the evolve extension's version of the command, but do not exist in core's version.)

When the user then confirms their selection, I was planning to create a commit for "change 'a' to 'b'" and another commit on top for "change 'b' to 'c'" (just like how hg ci -i && hg ci would create two commits).

I am not why we are creating explicit commit here, especially since this seems to introduce more complication than a "preserve the working copy content" approach.

I don't think I follow what the "preserve the working copy content" approach is. Can you explain?

> I was planning for the first change to go through steps 2 and 3 and 4 from above. However, the "change 'b' to 'c'" bit would also be rebased in step 4 and would apply cleanly. If we leave the changes in the working copy as well, then they'd conflict when they were rebased onto the folded commit (whether that rebasing was done by hg amend -r A -i --rebase or by hg amend -r A -i; hg evolve).
Thanks, I now see where the conflict comes from. However I don't see what would be the alternative to "leave the changes in the working copy" here ? Since current working copy content (mostly the final target) is different from what we amended into A.
Can you clarify what are the different alternative you have in mind here ?

My proposal is the 4 steps I described. Sorry, I think I don't understand what you're asking me to clarify.

In D10893#170283, @Alphare wrote:

Sorry I forgot to post this before the week-end.
@martinvonz broader picture question: what is your expected timeline for this getting into core? I'm seeing new questions coming up and it's looking like at least the UX for this new feature needs to be discussed some more.
I wanted to get this feature in before the freeze, but now that I see that it would be rushed at best, I'm hoping we can move this discussion to early August after the freeze to actually give the underlying questions a real shot. We've decided the same for the new dirstate-v2 format internally at Octobus for example.

Sure, let's wait until after the freeze.

In D10893#170063, @martinvonz wrote:

In D10893#170047, @marmoute wrote:

In D10893#166902, @martinvonz wrote:

I'll start with not even having the flag :P When automatically rebasing the descendants, it seems more clear that the changes should not be left in the working copy. I was planning on explicitly rebasing them and not leaving them in the working copy, not relying on them disappearing only because the changes were already present in an ancestor afterwards. To clarify, I was planning for it to do this:

1. Create temporary commit from working copy
2. Rebase temporary commit to target (not using --keep)
3. Fold the rebased commit into the target
4. Rebase descendants, not including the temporary commit from step 1

One could imagine using --keep in step two and include the temporary commit in step 4 and just rely on it becoming empty. However, if there were conflicts to resolve in step 2, then you'd run into conflicts again in step 4. It seems better to avoid that.

Looks like we could have a --no-evolve flag that stop after step 3 and be good.

Sure, but that can also be added after the series. I agree about adding such a flag (I'm not sure if I think it should be --no-evolve or --no-rebase), but I don't think it's important to get it added in this initial series.

It is important to consider it early in the series because it help having a deeper look at what the command/action tries to do and achieve and validate this is going in the right direction. History rewriting is a complex concept and being able to express what we do with a limited set of consistent action is important to make sure we build something users will have confidence into and steer people away from visible and invisible danger. One of the big invisible danger is the risk of creating workflow that seamlessly leads to N² markers creations which this "new" command could lead to. So think about the step by step here early is important as we need to consider if (1) it could exists (2) it could make sense (3) it might be sensible as a default.

Thanks for starting this discussion "early" (before sending more code) so that we could start discussing this early. These discussion around the corner case are very insightful about the use-cases and what might/should be done to solve them.

This last round of discussion make me think that this might not be the right UX to achieve our goal here. However I won't have time to elaborate much more because (1) I need some quiet time to think about it more (2) writing the ideas down will probably cost me multiple hours, hours that I currently do not have as the Windows/py3 testing and issue6528/6538 have been time consuming. I'll come back to you about that.

This should fit well with what seems to be the current split of your patch. Also I am not sure why you need a temporary commit here. We should be able to have:

1. update to target,

The problem is that this step may result in conflicts, and we don't think we have a good way of dealing with that. It seems much easier to create the temporary commit. When we add support for hg amend -i -r, we will create two temporary commits.

We have had all the necessary information to abort a hg update for years and a GSOC student working on that topic, I though we had hg update --abort for a couple of years already, but it doesn't seems like it. sight

2. amend the target,
3. evolve affected range (unless --no-evolve) is specified,

(note: further discussion got me to have a more idea about what could be done for --no-evolve, but this the above proposal is a good example a "minimal/simple" approach that still make sense, so I am keeping the comment)
the hg amend core behavior is: alter content of underlying changeset, do not alter¹ the working copy.

(It looks like you forgot to include footnote [1] that you point to here.)
Yes, that's true for plain hg amend.

I think this is an important property of hg amend and we should probably not break that expectation from users.

Note that it's not necessarily true (or at least it shouldn't necessarily be true in my opinion) for hg amend -r. For example, let's say you have this history:

@ commit 3, file content: A
|
o commit 2, file content: B
|
o commit 1, file content: A

You now realize that you want the contents in commit 1 to be "B", so you make that change in the working copy and run hg amend -r 1. I think users wouldn't expect "B" to remain in the working copy afterwards.

That does not seems like the main usecase of hg amend --rev (or anything that would fit the use case, I have been thinking we discuss). In my mind the current usecase is "I have a bunch of changes in my working copy and I want to amend them in existing commit. However, I don't want them amended in my direct parent, but in other ancestors. That is a usecase similar to hg absorb but with manual selection instead of having it automatically selected. For different usecase I feel like other command/UI are probably better suited.

For the case you describe using update/amend, histedit or anything else seems more suitable

This apply to bot hg amend and hg amend --extract (also known as uncommit). So whatever we goes for should probably leave the working copy untouched.
(note: this end up being untrue for hg amend --patch, but that is a quite special case, and we should probably get a hg amend --patch --rev 'wc()' working too.)

(I have never used hg amend --patch. Context for others: hg amend --patch and hg amend --extract exist in the evolve extension's version of the command, but do not exist in core's version.)

Ah, I did not realized at first that this series was about touching the hostile-fork of amend. This probably explain some of the confusion around "why don't you use rewriteutil" later since evolve's rewriteutil¹ is not around.

For the record since the two code bases has already diverged, I would prefer to see effort to converge them again than further diverging modification to the forked code. The evolve extensions is also still my preferred place for history rewriting UX experiment as it allow broader testing, with a shorter feedback cycle.
That said, I will keep doing my best to help reviewing this MR

[1] most of it is flagged as ready to upstream if someone is motivated.

When the user then confirms their selection, I was planning to create a commit for "change 'a' to 'b'" and another commit on top for "change 'b' to 'c'" (just like how hg ci -i && hg ci would create two commits).

I am not why we are creating explicit commit here, especially since this seems to introduce more complication than a "preserve the working copy content" approach.

I don't think I follow what the "preserve the working copy content" approach is. Can you explain?

This is what we are discussing above. hg amend is a command that leave the working copy content untouched, and I think it is important is stay so.

> I was planning for the first change to go through steps 2 and 3 and 4 from above. However, the "change 'b' to 'c'" bit would also be rebased in step 4 and would apply cleanly. If we leave the changes in the working copy as well, then they'd conflict when they were rebased onto the folded commit (whether that rebasing was done by hg amend -r A -i --rebase or by hg amend -r A -i; hg evolve).
Thanks, I now see where the conflict comes from. However I don't see what would be the alternative to "leave the changes in the working copy" here ? Since current working copy content (mostly the final target) is different from what we amended into A.
Can you clarify what are the different alternative you have in mind here ?

My proposal is the 4 steps I described. Sorry, I think I don't understand what you're asking me to clarify.

I think the "would also be rebased in step 4" make me think you had an alternative in mind. However if I understand your answer right, you did not had any alternative in mind.

In D10893#170300, @marmoute wrote:

Thanks for starting this discussion "early" (before sending more code) so that we could start discussing this early. These discussion around the corner case are very insightful about the use-cases and what might/should be done to solve them.
This last round of discussion make me think that this might not be the right UX to achieve our goal here. However I won't have time to elaborate much more because (1) I need some quiet time to think about it more (2) writing the ideas down will probably cost me multiple hours, hours that I currently do not have as the Windows/py3 testing and issue6528/6538 have been time consuming. I'll come back to you about that.

Okay, I'll work on other things until then. Just let me know when.

In D10893#170300, @marmoute wrote:

This last round of discussion make me think that this might not be the right UX to achieve our goal here. However I won't have time to elaborate much more because (1) I need some quiet time to think about it more (2) writing the ideas down will probably cost me multiple hours, hours that I currently do not have as the Windows/py3 testing and issue6528/6538 have been time consuming. I'll come back to you about that.

Actually, do you mind sharing at least sentence or so about what you think might be the right UX, even if it's not completely clear to you yet? I just want to get a sense of what you have in mind.

I am also big +1 on adding the feature. I think we can split this into multiple smaller features to split out discussion and ease review.

1. A patch that introduces amend --rev but aborts if there are conflicts.
2. Abort, continue and stop functionality and allowing amend --rev to work even if there are conflicts
3. Auto rebase children functionality

@martinvonz what do you think?

In D10893#171439, @martinvonz wrote:

In D10893#170300, @marmoute wrote:

This last round of discussion make me think that this might not be the right UX to achieve our goal here. However I won't have time to elaborate much more because (1) I need some quiet time to think about it more (2) writing the ideas down will probably cost me multiple hours, hours that I currently do not have as the Windows/py3 testing and issue6528/6538 have been time consuming. I'll come back to you about that.

Actually, do you mind sharing at least sentence or so about what you think might be the right UX, even if it's not completely clear to you yet? I just want to get a sense of what you have in mind.

Ping on this.

In D10893#172447, @martinvonz wrote:

In D10893#171439, @martinvonz wrote:

In D10893#170300, @marmoute wrote:

This last round of discussion make me think that this might not be the right UX to achieve our goal here. However I won't have time to elaborate much more because (1) I need some quiet time to think about it more (2) writing the ideas down will probably cost me multiple hours, hours that I currently do not have as the Windows/py3 testing and issue6528/6538 have been time consuming. I'll come back to you about that.

Actually, do you mind sharing at least sentence or so about what you think might be the right UX, even if it's not completely clear to you yet? I just want to get a sense of what you have in mind.

Ping on this.

I don't have a simple sentence (nor a simple idea) about this ready to use. I did not have a single second to even think about this discussion as I have been busy with 5.9-freeze related stuff for the past 3 weeks. Since 5.9 is unfortunately not out of the woods yet, do not hold you breath on this.

In D10893#172904, @marmoute wrote:

In D10893#172447, @martinvonz wrote:

In D10893#171439, @martinvonz wrote:

In D10893#170300, @marmoute wrote:

This last round of discussion make me think that this might not be the right UX to achieve our goal here. However I won't have time to elaborate much more because (1) I need some quiet time to think about it more (2) writing the ideas down will probably cost me multiple hours, hours that I currently do not have as the Windows/py3 testing and issue6528/6538 have been time consuming. I'll come back to you about that.

Actually, do you mind sharing at least sentence or so about what you think might be the right UX, even if it's not completely clear to you yet? I just want to get a sense of what you have in mind.

Ping on this.

I don't have a simple sentence (nor a simple idea) about this ready to use. I did not have a single second to even think about this discussion as I have been busy with 5.9-freeze related stuff for the past 3 weeks. Since 5.9 is unfortunately not out of the woods yet, do not hold you breath on this.

It seems that the freeze is now over, so I just wanted to make sure that this is not forgotten. I don't mean to be pushy, I just thought it's better to ping this now than to wait a month only to find out that it was simply forgotten.

In D10893#173733, @martinvonz wrote:

In D10893#172904, @marmoute wrote:

In D10893#172447, @martinvonz wrote:

In D10893#171439, @martinvonz wrote:

In D10893#170300, @marmoute wrote:

This last round of discussion make me think that this might not be the right UX to achieve our goal here. However I won't have time to elaborate much more because (1) I need some quiet time to think about it more (2) writing the ideas down will probably cost me multiple hours, hours that I currently do not have as the Windows/py3 testing and issue6528/6538 have been time consuming. I'll come back to you about that.

Actually, do you mind sharing at least sentence or so about what you think might be the right UX, even if it's not completely clear to you yet? I just want to get a sense of what you have in mind.

Ping on this.

I don't have a simple sentence (nor a simple idea) about this ready to use. I did not have a single second to even think about this discussion as I have been busy with 5.9-freeze related stuff for the past 3 weeks. Since 5.9 is unfortunately not out of the woods yet, do not hold you breath on this.

It seems that the freeze is now over, so I just wanted to make sure that this is not forgotten. I don't mean to be pushy, I just thought it's better to ping this now than to wait a month only to find out that it was simply forgotten.

I have not forgotten and I meant to get to that last week. Unfortunately some IRL situation came up and I have not be able to spend time on this (and even writing this message took a week longer that I wanted). Sorry for the delay. The second half of ext week should be more "normal" and I hope to get to that at that time.

In D10893#174783, @marmoute wrote:

I have not forgotten and I meant to get to that last week. Unfortunately some IRL situation came up and I have not be able to spend time on this (and even writing this message took a week longer that I wanted). Sorry for the delay. The second half of ext week should be more "normal" and I hope to get to that at that time.

It's been six weeks since this comment. I'm calling it: I'll entertain objections with reasons in the next 2 days, and if I hear none I'm queueing this series (pending a rebase and a couple of comment resolutions on 11039).

In D10893#170300, @marmoute wrote:

This last round of discussion make me think that this might not be the right UX to achieve our goal here. However I won't have time to elaborate much more because (1) I need some quiet time to think about it more (2) writing the ideas down will probably cost me multiple hours, hours that I currently do not have as the Windows/py3 testing and issue6528/6538 have been time consuming. I'll come back to you about that.

Thanks for your patience, I meant to follow up sooner but August end up filled
with fire-fighting project stuff (Windows' py3 compat, critical regression) and
September went South for unrelated IRL reason. Combined with another Octobusian
woe, I then had to catch up with about 3 months of missing work time. However, I
took advantage of the mini-sprint time to write down a handful of satellite
ideas that I needed to shape my thinking. Writing is always a slow and expensive process for me.
So let me do a brain dump of the current state of mind about this question.

What are we trying to do here ?

Let's take a step back about the goal here, as far as I understand, the use
case we have here is :

I have changes in my working copy, and I would like to move (some of) them within an existing changeset.

The problem is "not new" and already has some available options:

Existing solutions

hg amend

The hg amend command fit well the simplest case where "the existing
changesets" is the parent of the working copy. I don't think we need to
elaborate on that

commit + hg histedit's roll

A simple alternative that I currently use on a regular basis is to do a regular
commit with an explicit name (like "roll me into foo_bar() introduction"),
then eventually runs hg histedit and use the commit message to roll these
changesets into their target.

This work reasonably smoothly, but is still a bit more manual than we would like

git's fixup/autosquash

Git has a "official" version of the above method. You can use `git commit --fixup
fb2f677` to create a "fixup" commit for fb2f677. Then later run `git rebase
-i --autosquash` to automatically "roll" these changesets into their target.

hg absorb

The hg absorb command picks the uncommitted changes in the working copy and
insert them within ancestors of the working copy. The command amends multiple
changesets at once and "automatically select targets for each line changed

the full manual way : update variants

You can also do this manually, (I something use this variants)

One way to do this is:

• hg shelve -i # shelve the change you do not want to amend elsewhere
• hg up --merge TARGET
• hg amend
• hg evolve --rev X::Y # optional step, usually done later
• hg update Y'
• hg unshelve # if needed

This works but is quite manual.

the full manual way : commit variants

There is an alternate manual version of the "commit + histedit" solution above.

• hg commit -m "roll me into foo_bar() introduction"
• …
• keep hacking for a while
• …
• hg update XXXX # update to foo_bar() introduction
• hg pick YYYY # the roll me changeset (note: I actuall want hg pick --exact¹ here)
• hg fold --from .^
• …
• hg next | hg evolve | hg update elsewhere,
• …

I use this variant the most because it fit the "incremental work and dealing
with merge later" the best. However it is currently a bit too manual.
Especially in the lack of --extract support, mean some manual intervention is
requires for the children of the "roll me" changesets to be evolved at the right
location.

This variant provides a good decomposition of what we needs to do here, and it
quite useful to think about the UX we want.

[1] https://www.mercurial-scm.org/wiki/CEDUserInterface#Insertion_.2F_extraction

Other ?

There are probably alternative that I missed, please feel free to jump in.

Concerns I have the current proposal

In summary, the current proposal concerns me in the following areas:

• doing automatic evolution of the TARGET::SOURCE by default:
  • It introduce new merge related logic into hg amend make the command significantly more complex
  • It doesn't provide a "minimal step by step" user experience
  • it can lead to N² marker creation for which we do not have good mitigation implemented yet

• behavior regarding changes amended away is unclear:
  • What should happen to WC content in "step by step" operation ?
  • What should happen to WC content when amending a non-descendant ?
  • This might introduce a significant change to the behavior of hg amend regarding the working directory which was previously unaffected
  • should we have an explicit commit or not?

regarding automatic evolution

hg amend is not currently triggering any merge or long operation, so it currently needs
neither logic about merge nor logic about abort/continue/stop. The current
proposal means we will have to pull that complexity in with all the associated
flags. This is not always a bad idea, but we have to be careful. Maybe `hg
amend` is not the right location to do this. I wrote a bit more about this
topic here :
https://www.mercurial-scm.org/wiki/Pierre-YvesDavid#Combining_feature_by_adding_.60hg_command_--flags.60

The current lack of a clear definition for that "minimal" step should be is
also of great concern to me. I feel like we need to have a clear vision of what the
atomic building block of the UX are to build a consistent experience across
all commands.

Since this is a recurring topic I started to put some of my thinking on that topic in writing here:
https://www.mercurial-scm.org/wiki/Pierre-YvesDavid#Evolution_UX_and_small_iteration

regarding the fate of changes amended away

Right now, hg amend is never¹ altering the content of file in the working
copy. The (part of) the file moves from dirty to clean, but the actual file
content is never changed. That is a property that I feel important. This create
a clear envelope to the command behavior that users relies on.

In an ideal (simple) case this could be true with the current proposal : a
simple change is moved down the stack without any conflict, and with its
descendant rebased it will still be available as is in the parent changeset and
therefore in the working copy. However there is a bunch a of cases where this
become more complicated:

• If we amend changes into a revision that is not a descendant, we could not get it back "after the rebase". So the working copy would get modified. This might be, or not be what the user wants, but this is significant change to how hg amend behave.

• If there is conflict down (and up) the line, we might ends up with a different version of the change in the working copy.

• For a "minimalist" operation, the change would only be amended inside the TARGET changeset without rebasing its descendant. If we simple "updated the change away" we would lose it from the working copy. Which will clearly be an issue in some cases.

[1] actually, --interactive's edit and --patch might, but they are special case where the edit is explicit

Proposal to move forward.

Okay, so that was a lot of questions, but what should we do? How could we move forward?

regarding the minimal step

As I said before we should focus on having something providing a minimal usable brick that fit the use case.
I current see the minimal step for this to be :

1. commit the change you want to insert elsewhere
2. relocate the change on TARGET while keeping the current working directory parent (so stay on the obsolete changeset).
3. fold the change into TARGET

And that's all. After this change:

• descendant of TARGET are orphan,
• the working copy is based on an obsolete changeset (whose latest successors are TARGET')

From there the user can decide what fits them better:

• continue the operation with a different part of the working copy,
• update back to the parent changeset to drop the change they just amended away out of the working copy
• to stabilize everything to solve the orphanage,
• to create more commit on top of the current obsolete changeset and deal with the merge later.
• whatever else …

For a smoother experience we would need to start some implementation of the
"extract" concept¹ to help hg update and hg evolve to deal with the
temporary changeset properly.

[1] https://www.mercurial-scm.org/wiki/CEDUserInterface#Insertion_.2F_extraction

What UI should we go for ?

Since the command still involves some merging and abortable operation, I am
still a bit nervous to include it into hg amend as a start. (or into `hg
commit` as git did).

So starting with a new commands hg fixup seems safer to experiment with the
feature, its behavior and the various new flags we will need for it.

It might make sense to include the "simple" version of it in existing commands
to help with usage and discoverability, but it seems too early to think about
it.

I put more though about this topic here:
https://www.mercurial-scm.org/wiki/Pierre-YvesDavid#Feature_discoverability.2C_command_vs_flags.2C_etc

Implementation concerns

Just so that we do not forget about them, I already did a handful of comments on
the current implementation (see commend within the series).

The deepest concerns I have it the extensive use of subcommand calls. The
command "API" are user-focused and carry backward compatibility baggage. This
often make then a bit odd and slippery to call from normal code. In addition,
having a lot of actual code relying on the command API make it harder for the
command to change in UI since it will have to deal with consequences on both user input and
code input when updating it. And since the code input goes through different
paths and can use more subtle tricks in terms of value it passes, this often
mean subtle failure. This has been a notable pain point in past refactoring.

I would rather see dedicated functions with a clear semantic and API put to use.
The evolve extension has a toolbox of those. Some have already be upstreamed,
some are ready to be.