D817: histedit: add move histedit action

quark (Jun Wu) phabricator at mercurial-scm.org
Tue Sep 26 13:34:00 EDT 2017


quark added inline comments.

INLINE COMMENTS

> ryanmce wrote in histedit.py:861
> `hide` is really only FB-internal naming. In regular/core hg, this would still likely be a `strip`.

Strictly speaking, it is "rebase", which is not 100% equal to graft + hide. The latter does not create an obsmarker.

> ryanmce wrote in histedit.py:867-868
> Thinking about this more, I think this is pretty non-obvious to an average user.
> 
> In fact, the "move" versus "pick" difference is really unclear in general. Why must I "pick" something in the set, even if I'm actually moving it up or down, and "move" something from outside? If I "move" something up or down a slot, it's unclear why I want to use "pick" instead of "move" to an average user.
> 
> It seems to me that before we do this we should take a step back and talk about the end state we want to get to.
> 
> In my "idealized" world, histedit might have:
> 
> - `keep` (instead of "pick"), which I think is more clear. "pick" could still be a backwards-compatible name though (git's "pick" comes from "cherry-pick", and copying it to histedit seems like a mistake to me, in hindsight.
> - I like "copy" as-implemented in the previous patch. I think it's pretty clear and a nice four-letter name.
> - some kind of verb that allows me to take whatever commit, from wherever, and apply it where I am now with no unreasonable restrictions. Like a superset of "keep" and "move" (or whatever we name that). We could call it "grab" (?). `grab` is a term that we used for a while to "move" things from one point to another. It kind of evokes "graft" with it's prefix but is clearly different and implies taking it away from wherever it is.
> - (maybe) a "safe" way to move from somewhere else. I'm not sold on the "move" name the more I think about it because, desptie nicely being 4 letters, it doesn't really make clear to me how it is to be used.

How about just letting `pick` accept external commits? We can gate the feature by a config option if it looks risky.

> histedit.py:880
> +                msg = _('%s "%s" is an ancestor of base changeset "%s"')
> +                raise error.ParseError(
> +                    msg % (self.verb, node.short(self.node),

`error.Abort` may be more appropriate. `ParseError` is more about a syntax error (ex. having `[^0-9a-f]` in commit hash).

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D817

To: mbthomas, #hg-reviewers, ryanmce
Cc: quark, ryanmce, mercurial-devel


More information about the Mercurial-devel mailing list