Page MenuHomePhabricator

martinvonz (Martin von Zweigbergk)
User

Projects

User Details

User Since
Jun 28 2017, 5:28 PM (221 w, 5 d)

Recent Activity

Fri, Sep 24

martinvonz closed D11496: errors: use InputError for some invalid revsets and such.
Fri, Sep 24, 8:06 AM
martinvonz closed D11498: errors: use InputError for bad path arguments to `hg annotate`.
Fri, Sep 24, 8:06 AM
martinvonz closed D11497: errors: use InputError for bad --similarity value.
Fri, Sep 24, 8:06 AM
martinvonz committed rHG12966768595a: errors: use InputError for bad path arguments to `hg annotate`.
errors: use InputError for bad path arguments to `hg annotate`
Fri, Sep 24, 8:06 AM
martinvonz committed rHG5b89626c11e9: errors: use InputError for bad --similarity value.
errors: use InputError for bad --similarity value
Fri, Sep 24, 8:06 AM
martinvonz committed rHG7d908ee19b5b: errors: use InputError for some invalid revsets and such.
errors: use InputError for some invalid revsets and such
Fri, Sep 24, 8:05 AM

Thu, Sep 23

martinvonz created D11496: errors: use InputError for some invalid revsets and such.
Thu, Sep 23, 2:26 PM
martinvonz created D11498: errors: use InputError for bad path arguments to `hg annotate`.
Thu, Sep 23, 2:26 PM
martinvonz created D11497: errors: use InputError for bad --similarity value.
Thu, Sep 23, 2:25 PM

Tue, Sep 21

D11453: log: if there is a merge state, show conflictother for . [WIP] now requires changes to proceed.
Tue, Sep 21, 12:51 PM

Fri, Sep 17

martinvonz closed D11445: dirstate: fix compilation warnings in `dirstate_item_set_possibly_dirty()`.
Fri, Sep 17, 11:16 AM
martinvonz closed D11444: dirstate: make dirstate flags char be unsigned.
Fri, Sep 17, 11:16 AM
martinvonz committed rHG2018753014be: dirstate: fix compilation warnings in `dirstate_item_set_possibly_dirty()`.
dirstate: fix compilation warnings in `dirstate_item_set_possibly_dirty()`
Fri, Sep 17, 11:16 AM
martinvonz committed rHGec178161a8d1: dirstate: make dirstate flags char be unsigned.
dirstate: make dirstate flags char be unsigned
Fri, Sep 17, 11:15 AM
martinvonz added a comment to D11444: dirstate: make dirstate flags char be unsigned.

@martinvonz clangformat complains:

@@ -8,3 +8,18 @@
   >   cmp $f $f.formatted || diff -u $f $f.formatted
   >   rm $f.formatted
   > done
+  mercurial/cext/parsers.c mercurial/cext/parsers.c.formatted differ: char 3857, line 147
+  --- mercurial/cext/parsers.c        2021-09-17 15:05:49.010806214 +0000
+  +++ mercurial/cext/parsers.c.formatted      2021-09-17 15:06:34.139000016 +0000
+  @@ -144,8 +144,9 @@
+
+   static inline bool dirstate_item_c_added(dirstateItemObject *self)
+   {
+  -   unsigned char mask = (dirstate_flag_wc_tracked | dirstate_flag_p1_tracked |
+  -                dirstate_flag_p2_tracked);
+  +   unsigned char mask =
+  +       (dirstate_flag_wc_tracked | dirstate_flag_p1_tracked |
+  +        dirstate_flag_p2_tracked);
+      unsigned char target = dirstate_flag_wc_tracked;
+      return (self->flags & mask) == target;
+   }

I can just amend this if you want.

Fri, Sep 17, 11:11 AM
martinvonz added a comment to D11445: dirstate: fix compilation warnings in `dirstate_item_set_possibly_dirty()`.

We should just drop the if :

self->flags |= dirstate_flag_possibly_dirty;
Py_RETURN_NONE
Fri, Sep 17, 10:56 AM
martinvonz updated the summary of D11445: dirstate: fix compilation warnings in `dirstate_item_set_possibly_dirty()`.
Fri, Sep 17, 10:56 AM

Thu, Sep 16

martinvonz created D11444: dirstate: make dirstate flags char be unsigned.
Thu, Sep 16, 8:07 PM
martinvonz created D11445: dirstate: fix compilation warnings in `dirstate_item_set_possibly_dirty()`.
Thu, Sep 16, 8:07 PM

Mon, Aug 30

martinvonz closed D11376: fix: again allow formatting the working copy while merging.
Mon, Aug 30, 5:17 AM
martinvonz committed rHG86a60679cf61: fix: again allow formatting the working copy while merging.
fix: again allow formatting the working copy while merging
Mon, Aug 30, 5:17 AM

Aug 27 2021

martinvonz created D11376: fix: again allow formatting the working copy while merging.
Aug 27 2021, 5:08 PM

Aug 23 2021

martinvonz added a comment to D11177: fix: use `set_possibly_dirty` instead of `normallookup`.

I noticed that this broke hg fix in some cases. I wrote a test case to show the difference (replace set_possibly_dirty by normallookup to see the different behavior)

Aug 23 2021, 4:44 PM

Aug 21 2021

martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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.

Aug 21 2021, 12:28 PM

Aug 2 2021

martinvonz added a comment to D11237: tests: rename test-clone-uncompressed.t.

I have been assuming that changes can still be submitted during a change freeze, but they are just going to get ignored for a few days. None of my changes are particularly intended to go on 5.9.

Aug 2 2021, 10:41 AM
martinvonz added a comment to D11237: tests: rename test-clone-uncompressed.t.

This patch seems to be for default. I think we're supposed to be in code freeze (though lots of patches have still been queued). Should we mark patches (like this one) that are not for stable as "Changes Requested" so they're not accidentally queued during the freeze?

Aug 2 2021, 10:35 AM

Jul 30 2021

martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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.

Jul 30 2021, 12:26 PM

Jul 24 2021

martinvonz closed D11210: fix: use scmutil.movedirstate() instead of reimplementing.
Jul 24 2021, 5:52 AM
martinvonz closed D11209: fix: rewrite writeworkingdir() to explicitly not work with merges.
Jul 24 2021, 5:52 AM
martinvonz committed rHG66ad7e32011f: fix: use scmutil.movedirstate() instead of reimplementing.
fix: use scmutil.movedirstate() instead of reimplementing
Jul 24 2021, 5:52 AM
martinvonz closed D11208: tests: demonstrate bug in `hg fix` with incorrectly dirty working copy.
Jul 24 2021, 5:52 AM
martinvonz committed rHG3feda1e779d4: fix: rewrite writeworkingdir() to explicitly not work with merges.
fix: rewrite writeworkingdir() to explicitly not work with merges
Jul 24 2021, 5:52 AM
martinvonz committed rHG184d83ef2e59: tests: demonstrate bug in `hg fix` with incorrectly dirty working copy.
tests: demonstrate bug in `hg fix` with incorrectly dirty working copy
Jul 24 2021, 5:52 AM

Jul 23 2021

martinvonz requested review of D11210: fix: use scmutil.movedirstate() instead of reimplementing.
Jul 23 2021, 3:24 AM
martinvonz updated the summary of D11210: fix: use scmutil.movedirstate() instead of reimplementing.
Jul 23 2021, 3:24 AM
martinvonz added a comment to D11210: fix: use scmutil.movedirstate() instead of reimplementing.

This is slower, but probably doesn't matter?

Jul 23 2021, 3:19 AM
martinvonz created D11210: fix: use scmutil.movedirstate() instead of reimplementing.
Jul 23 2021, 3:17 AM
martinvonz created D11209: fix: rewrite writeworkingdir() to explicitly not work with merges.
Jul 23 2021, 3:17 AM
martinvonz created D11208: tests: demonstrate bug in `hg fix` with incorrectly dirty working copy.
Jul 23 2021, 3:17 AM

Jul 19 2021

martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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.

Jul 19 2021, 1:52 PM

Jul 16 2021

martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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.

Jul 16 2021, 7:45 PM
martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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.

Jul 16 2021, 4:14 PM

Jul 15 2021

martinvonz added inline comments to D11037: amend: make `hg amend -r` rebase temporary commit onto target commit.
Jul 15 2021, 8:40 PM
martinvonz updated the diff for D11039: amend: make `hg amend -r` rebase descendants of target onto amended target.
Jul 15 2021, 8:36 PM
martinvonz updated the diff for D11038: amend: make `hg amend -r` fold temporary commit into target commit.
Jul 15 2021, 8:36 PM
martinvonz updated the diff for D11037: amend: make `hg amend -r` rebase temporary commit onto target commit.
Jul 15 2021, 8:36 PM
martinvonz added inline comments to D10893: amend: add a useless initial version of `amend -r REV `.
Jul 15 2021, 8:27 PM
martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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.

Jul 15 2021, 8:12 PM

Jul 12 2021

martinvonz added a comment to D11039: amend: make `hg amend -r` rebase descendants of target onto amended target.

Can we test that further descendants (potentially not linear) are not rebased automatically? This can be done in a separate, simpler test case as an additional changeset.

Jul 12 2021, 2:43 PM
martinvonz updated the diff for D11039: amend: make `hg amend -r` rebase descendants of target onto amended target.
Jul 12 2021, 2:43 PM
martinvonz updated the diff for D11038: amend: make `hg amend -r` fold temporary commit into target commit.
Jul 12 2021, 2:43 PM
martinvonz updated the diff for D11037: amend: make `hg amend -r` rebase temporary commit onto target commit.
Jul 12 2021, 2:43 PM
martinvonz updated the diff for D10893: amend: add a useless initial version of `amend -r REV `.
Jul 12 2021, 2:43 PM
martinvonz added a comment to D11040: amend: make `hg amend -r` update to the rewritten working copy parent.

I won't actually need this. I'm going to make the command update to the old working copy parent before doing the rebasing, which will result in rebase taking care of the update for us.

Jul 12 2021, 2:21 PM
martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

I'll add tests for amending into siblings and descendants too, along with code changes necessary to make that work.

Jul 12 2021, 12:47 PM
martinvonz added inline comments to D11037: amend: make `hg amend -r` rebase temporary commit onto target commit.
Jul 12 2021, 11:49 AM

Jul 9 2021

martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

This series is now ready for review again

Jul 9 2021, 4:28 PM
martinvonz created D11040: amend: make `hg amend -r` update to the rewritten working copy parent.
Jul 9 2021, 4:28 PM
martinvonz created D11039: amend: make `hg amend -r` rebase descendants of target onto amended target.
Jul 9 2021, 4:28 PM
martinvonz created D11038: amend: make `hg amend -r` fold temporary commit into target commit.
Jul 9 2021, 4:28 PM
martinvonz created D11037: amend: make `hg amend -r` rebase temporary commit onto target commit.
Jul 9 2021, 4:28 PM
martinvonz updated the summary of D10893: amend: add a useless initial version of `amend -r REV `.
Jul 9 2021, 4:28 PM

Jul 8 2021

martinvonz committed rHGc8f8d2dba6c7: tests: add test case for issue 6262.
tests: add test case for issue 6262
Jul 8 2021, 7:54 AM
martinvonz committed rHGdebc29900b97: rewriteutil: look up common predecessor on unfiltered repo.
rewriteutil: look up common predecessor on unfiltered repo
Jul 8 2021, 7:53 AM
martinvonz committed rHG93ca7d3278b9: tests: demonstrate crash when common predecessor of divergence is hidden.
tests: demonstrate crash when common predecessor of divergence is hidden
Jul 8 2021, 7:53 AM
martinvonz committed rHG6765602a4e3d: tests: add test case for issue 6262.
tests: add test case for issue 6262
Jul 8 2021, 7:45 AM
martinvonz closed D10916: tests: add test case for issue 6262.
Jul 8 2021, 5:02 AM
martinvonz closed D10917: rewriteutil: look up common predecessor on unfiltered repo.
Jul 8 2021, 5:02 AM
martinvonz committed rHGf97fc3c6f7e4: tests: add test case for issue 6262.
tests: add test case for issue 6262
Jul 8 2021, 5:02 AM
martinvonz closed D11009: tests: demonstrate crash when common predecessor of divergence is hidden.
Jul 8 2021, 5:02 AM
martinvonz committed rHG69b35ea9d5f9: rewriteutil: look up common predecessor on unfiltered repo.
rewriteutil: look up common predecessor on unfiltered repo
Jul 8 2021, 5:02 AM
martinvonz committed rHG19e740216161: tests: demonstrate crash when common predecessor of divergence is hidden.
tests: demonstrate crash when common predecessor of divergence is hidden
Jul 8 2021, 5:02 AM

Jul 7 2021

martinvonz retitled D10916: tests: add test case for issue 6262 from tests: demonstrate crash when trying to rewrite pruned part of a split to tests: add test case for issue 6262.
Jul 7 2021, 2:22 PM
martinvonz updated the diff for D10917: rewriteutil: look up common predecessor on unfiltered repo.
Jul 7 2021, 2:22 PM
martinvonz created D11009: tests: demonstrate crash when common predecessor of divergence is hidden.
Jul 7 2021, 2:22 PM
martinvonz added a comment to D10917: rewriteutil: look up common predecessor on unfiltered repo.

This looks to me like we're moving from one bad output to a different bad output that is arguably more confusing. Maybe I'm not seeing the improvement or there is something else baking behind this?

This is wrongly reported divergence, which is bad, but not the point of this patch.
Before this patch we were crashing while (wrongly) reporting that divergence, now we no longer crash while reporting the divergence. The fact the report is wrong is orthogonal, we should not crash while reporting divergence.

Put another way (explaining to Alphare): Even though this patch doesn't immediately help the user, it helps us hg developers by not forcing us to remember to fix this once we've fixed issue 6262.

Since issue6262 will eventually be fixed, this won't be tested anymore. Would you follow up with a test covering this message + case ?

Good point. I'll try to write a(nother) test case that results in a hidden common predecessor.

Jul 7 2021, 1:25 PM
martinvonz added a comment to D10917: rewriteutil: look up common predecessor on unfiltered repo.

This looks to me like we're moving from one bad output to a different bad output that is arguably more confusing. Maybe I'm not seeing the improvement or there is something else baking behind this?

This is wrongly reported divergence, which is bad, but not the point of this patch.
Before this patch we were crashing while (wrongly) reporting that divergence, now we no longer crash while reporting the divergence. The fact the report is wrong is orthogonal, we should not crash while reporting divergence.

Put another way (explaining to Alphare): Even though this patch doesn't immediately help the user, it helps us hg developers by not forcing us to remember to fix this once we've fixed issue 6262.

Since issue6262 will eventually be fixed, this won't be tested anymore. Would you follow up with a test covering this message + case ?

Jul 7 2021, 12:32 PM
martinvonz added a comment to D10917: rewriteutil: look up common predecessor on unfiltered repo.

This looks to me like we're moving from one bad output to a different bad output that is arguably more confusing. Maybe I'm not seeing the improvement or there is something else baking behind this?

This is wrongly reported divergence, which is bad, but not the point of this patch.
Before this patch we were crashing while (wrongly) reporting that divergence, now we no longer crash while reporting the divergence. The fact the report is wrong is orthogonal, we should not crash while reporting divergence.

Jul 7 2021, 12:05 PM

Jul 1 2021

martinvonz added a comment to D10917: rewriteutil: look up common predecessor on unfiltered repo.

It does not looks like this is done at the right level. The code responsible for the processors walking should be the one adjusting the filter level.

Which code is that? Do you mean that it should be done at a higher or lower level?

lower level

I still don't understand. Do you mean that find_new_divergence_from() should not return filtered nodeids?

No, given the information returned by find_new_divergence_from() it seems unavoidable that filtered revision might be returned as the "common-predecessor" value. (sidenote: that the common-precessors should probably be a list, and the return of find_new_divergence_from and _find_new_divergence should probably be list too).
My point is that we are looking for divergence within the current filter and that we should not pass unfiltered repository to the function that looks for that divergence. Otherwise it might compute things on the wrong set of revisions.
However, function along the chain should be prepared for the "common-predecessor" nodes to be potentially filtered. In this case this means turning this line

return (repo[r], repo[div[0]], repo[div[1]])

to

return (repo[r], repo[div[0]], unfi[div[1]])
Jul 1 2021, 1:05 PM
martinvonz retitled D10917: rewriteutil: look up common predecessor on unfiltered repo from rewriteutil: check for divergence on unfiltered repo to rewriteutil: look up common predecessor on unfiltered repo.
Jul 1 2021, 1:05 PM

Jun 30 2021

martinvonz added a comment to D10917: rewriteutil: look up common predecessor on unfiltered repo.

It does not looks like this is done at the right level. The code responsible for the processors walking should be the one adjusting the filter level.

Which code is that? Do you mean that it should be done at a higher or lower level?

lower level

Jun 30 2021, 11:26 AM
martinvonz added a comment to D10917: rewriteutil: look up common predecessor on unfiltered repo.

It does not looks like this is done at the right level. The code responsible for the processors walking should be the one adjusting the filter level.

Jun 30 2021, 10:58 AM

Jun 29 2021

martinvonz created D10917: rewriteutil: look up common predecessor on unfiltered repo.
Jun 29 2021, 5:20 PM
martinvonz created D10916: tests: add test case for issue 6262.
Jun 29 2021, 5:20 PM

Jun 28 2021

martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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.

Jun 28 2021, 1:17 PM
martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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
Jun 28 2021, 12:19 PM

Jun 23 2021

martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

I'll update this patch and add several more after it in a day or two. (I should have marked it RFC to start with.)

Jun 23 2021, 1:43 PM
martinvonz closed D10892: rebase: use str-keyed opts in remaining places.
Jun 23 2021, 8:43 AM
martinvonz closed D10891: rebase: keep str-keyed opts long enough to make `action` a str.
Jun 23 2021, 8:42 AM
martinvonz committed rHGc9cedb546262: rebase: use str-keyed opts in remaining places.
rebase: use str-keyed opts in remaining places
Jun 23 2021, 8:42 AM
martinvonz committed rHGde54d11040e7: rebase: keep str-keyed opts long enough to make `action` a str.
rebase: keep str-keyed opts long enough to make `action` a str
Jun 23 2021, 8:42 AM

Jun 22 2021

martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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.

Jun 22 2021, 9:00 PM
martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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.

Jun 22 2021, 7:00 PM
martinvonz committed rHG50cd14dbd3b3: benchmarks: restore `output` variable lost in D10884.
benchmarks: restore `output` variable lost in D10884
Jun 22 2021, 6:22 PM
martinvonz closed D10894: benchmarks: restore `output` variable lost in D10884.
Jun 22 2021, 6:22 PM
martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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?

Jun 22 2021, 1:01 PM
martinvonz updated the diff for D10893: amend: add a useless initial version of `amend -r REV `.
Jun 22 2021, 12:30 PM
martinvonz added a comment to D10893: amend: add a useless initial version of `amend -r REV `.

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

Jun 22 2021, 12:20 PM
martinvonz added a comment to D10894: benchmarks: restore `output` variable lost in D10884.

Doesn't this lose the ui.silent feature?

Jun 22 2021, 12:06 PM
martinvonz created D10894: benchmarks: restore `output` variable lost in D10884.
Jun 22 2021, 11:32 AM