| Submitter | Greg Ward |
|---|---|
| Date | June 22, 2014, 11:52 p.m. |
| Message ID | <4ab7a80fc11f275c03d4.1403481156@lucifer.gerg.ca> |
| Download | mbox | patch |
| Permalink | /patch/5041/ |
| State | Changes Requested |
| Headers | show |
Comments
On 06/22/2014 04:52 PM, Greg Ward wrote: > # HG changeset patch > # User Greg Ward <greg@gerg.ca> > # Date 1403481137 14400 > # Sun Jun 22 19:52:17 2014 -0400 > # Node ID 4ab7a80fc11f275c03d4ddb94936a0688b71e6bc > # Parent 2fbba0bf7e7c8cbff1f94bc95c4d6214df85ef81 > fold: take an explicit list of revisions (BC) No thanks. This use the be the default and ended up being very confusing. The vast majority of fold operation involve the working directory and an ancestors (something a descendant). The default should reflect that. Also, we should not requires user to have a PhD in revset to use Mercurial. So, I'm deeply convince the default should be "fold between . and REV". How ever I agree than having different behavior for `hg fold REV` and `hg fold --rev REV` sucks. I also agree that the "do an in memory fold of unrelated changeset" usecase is nice. We should think about a dedicated flag for this case.
On 23 June 2014, Pierre-Yves David said: > On 06/22/2014 04:52 PM, Greg Ward wrote: > ># HG changeset patch > ># User Greg Ward <greg@gerg.ca> > ># Date 1403481137 14400 > ># Sun Jun 22 19:52:17 2014 -0400 > ># Node ID 4ab7a80fc11f275c03d4ddb94936a0688b71e6bc > ># Parent 2fbba0bf7e7c8cbff1f94bc95c4d6214df85ef81 > >fold: take an explicit list of revisions (BC) > > No thanks. Darn. I guess I misunderstood our previous discussion about this. > This use the be the default and ended up being very confusing. The > vast majority of fold operation involve the working directory and an > ancestors (something a descendant). The default should reflect that. > Also, we should not requires user to have a PhD in revset to use Mercurial. "REV::" is not a PhD-level revset. > So, I'm deeply convince the default should be "fold between . and REV". OK, fine. I happen to disagree, but whatever. > How ever I agree than having different behavior for `hg fold REV` > and `hg fold --rev REV` sucks. OK, I'll see if I can fix that. > I also agree that the "do an in > memory fold of unrelated changeset" usecase is nice. We should think > about a dedicated flag for this case. --only? Example usage: hg fold --only R1 R2 ... Rn combines R1, R2, and Rn to make a new changeset. Greg
Greg Ward writes: > On 23 June 2014, Pierre-Yves David said: >> On 06/22/2014 04:52 PM, Greg Ward wrote: >> ># HG changeset patch >> ># User Greg Ward <greg@gerg.ca> >> ># Date 1403481137 14400 >> ># Sun Jun 22 19:52:17 2014 -0400 >> ># Node ID 4ab7a80fc11f275c03d4ddb94936a0688b71e6bc >> ># Parent 2fbba0bf7e7c8cbff1f94bc95c4d6214df85ef81 >> >fold: take an explicit list of revisions (BC) >> >> No thanks. > > Darn. I guess I misunderstood our previous discussion about this. > >> This use the be the default and ended up being very confusing. The >> vast majority of fold operation involve the working directory and an >> ancestors (something a descendant). The default should reflect that. >> Also, we should not requires user to have a PhD in revset to use Mercurial. > > "REV::" is not a PhD-level revset. > >> So, I'm deeply convince the default should be "fold between . and REV". > > OK, fine. I happen to disagree, but whatever. I strongly agree with Greg on this one. 'hg fold' is horribly confusing. I posit that your sample space is too small. >> How ever I agree than having different behavior for `hg fold REV` >> and `hg fold --rev REV` sucks. > > OK, I'll see if I can fix that. > >> I also agree that the "do an in >> memory fold of unrelated changeset" usecase is nice. We should think >> about a dedicated flag for this case. What is this? Git? If you're so strongly convinced that 'hg fold' should default to .^::. then why don't you make an alias (or potentially, a revset alias)?
On Mon, Jun 23, 2014 at 09:02:14PM -0400, Greg Ward wrote: > On 23 June 2014, Pierre-Yves David said: > > On 06/22/2014 04:52 PM, Greg Ward wrote: > > ># HG changeset patch > > ># User Greg Ward <greg@gerg.ca> > > ># Date 1403481137 14400 > > ># Sun Jun 22 19:52:17 2014 -0400 > > ># Node ID 4ab7a80fc11f275c03d4ddb94936a0688b71e6bc > > ># Parent 2fbba0bf7e7c8cbff1f94bc95c4d6214df85ef81 > > >fold: take an explicit list of revisions (BC) > > > > No thanks. > > Darn. I guess I misunderstood our previous discussion about this. > > > This use the be the default and ended up being very confusing. The > > vast majority of fold operation involve the working directory and an > > ancestors (something a descendant). The default should reflect that. > > Also, we should not requires user to have a PhD in revset to use Mercurial. > > "REV::" is not a PhD-level revset. > > > So, I'm deeply convince the default should be "fold between . and REV". > > OK, fine. I happen to disagree, but whatever. I agree with Greg here - the magic default of 'hg fold $REV' -> fold together $REV::. feels a little spooky. This feels like the right case for an alias to me. (It's been stated by many that histedits behavior in this area is confusing, and with the benefit of hindsight and the existence of evolution I'm starting to come around on that point.) > > > How ever I agree than having different behavior for `hg fold REV` > > and `hg fold --rev REV` sucks. > > OK, I'll see if I can fix that. > > > I also agree that the "do an in > > memory fold of unrelated changeset" usecase is nice. We should think > > about a dedicated flag for this case. > > --only? Example usage: > > hg fold --only R1 R2 ... Rn > > combines R1, R2, and Rn to make a new changeset. > > Greg > -- > Greg Ward http://www.gerg.ca > <greg@gerg.ca> @gergdotca > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On 24 June 2014, Augie Fackler said: > I agree with Greg here - the magic default of 'hg fold $REV' -> fold > together $REV::. feels a little spooky. This feels like the right case > for an alias to me. OR if it's as important a use case as Pierre-Yves claims, add an option. E.g. make hg fold [-r] REV... (as implemented in my patch) the default. Then add hg fold --from REV to restore the current (REV::.) behaviour. Hmmm. Time for a poll? Here are the options: 1) status quo: "hg fold REV" and "hg fold -r REV" are different 2) my proposal: "hg fold [-r] REV...", with --from option to satisfy people who like the current "hg fold REV" semantics 3) Pierre-Yves' proposal: "hg fold [-r] REV" same as current "hg fold REV", with "hg fold --only REV..." option to satisfy people who like the current "hg fold -r REV..." semantics I'm pretty sure no one will vote for #1. Go ahead, surprise me! Sounds like Sean, Augie, and I are for #2. Clearly Pierre-Yves likes #3. Who else? Vote early, vote often. Greg
El 26/06/2014 14:47, "Greg Ward" <greg@gerg.ca> escribió: > > On 24 June 2014, Augie Fackler said: > > I agree with Greg here - the magic default of 'hg fold $REV' -> fold > > together $REV::. feels a little spooky. This feels like the right case > > for an alias to me. > > OR if it's as important a use case as Pierre-Yves claims, add an > option. E.g. make > > hg fold [-r] REV... > > (as implemented in my patch) the default. Then add > > hg fold --from REV > > to restore the current (REV::.) behaviour. > > Hmmm. Time for a poll? Here are the options: > > 1) status quo: "hg fold REV" and "hg fold -r REV" are different > > 2) my proposal: "hg fold [-r] REV...", with --from option > to satisfy people who like the current "hg fold REV" semantics > > 3) Pierre-Yves' proposal: "hg fold [-r] REV" same as current > "hg fold REV", with "hg fold --only REV..." option to > satisfy people who like the current "hg fold -r REV..." semantics > > I'm pretty sure no one will vote for #1. Go ahead, surprise me! > > Sounds like Sean, Augie, and I are for #2. > > Clearly Pierre-Yves likes #3. Who else? > > Vote early, vote often. I think I prefer #2. Cheers, Angel
Greg Ward writes: > On 24 June 2014, Augie Fackler said: >> I agree with Greg here - the magic default of 'hg fold $REV' -> fold >> together $REV::. feels a little spooky. This feels like the right case >> for an alias to me. > > OR if it's as important a use case as Pierre-Yves claims, add an > option. E.g. make > > hg fold [-r] REV... > > (as implemented in my patch) the default. Then add > > hg fold --from REV > > to restore the current (REV::.) behaviour. > > Hmmm. Time for a poll? Here are the options: > > 1) status quo: "hg fold REV" and "hg fold -r REV" are different > > 2) my proposal: "hg fold [-r] REV...", with --from option > to satisfy people who like the current "hg fold REV" semantics > > 3) Pierre-Yves' proposal: "hg fold [-r] REV" same as current > "hg fold REV", with "hg fold --only REV..." option to > satisfy people who like the current "hg fold -r REV..." semantics > > I'm pretty sure no one will vote for #1. Go ahead, surprise me! > > Sounds like Sean, Augie, and I are for #2. There's a big of hack we could do here: a) $ hg fold b) $ hg fold SINGLEREV c) $ hg fold REVSET Currently, (a) doesn't make sense so we could have it default to folding with the parent '.'. Also, (b), I think, only makes sense if you want to fold from '.' to SINGLEREV. That leaves (c) to be an arbitrary fold that could work with an in-memory implementation. > Clearly Pierre-Yves likes #3. Who else? No one. Pierre-Yves is dangerously close to: https://yourlogicalfallacyis.com/anecdotal https://yourlogicalfallacyis.com/appeal-to-nature
On Thu, 2014-06-26 at 13:55 -0500, Sean Farley wrote: > Greg Ward writes: > > > On 24 June 2014, Augie Fackler said: > >> I agree with Greg here - the magic default of 'hg fold $REV' -> fold > >> together $REV::. feels a little spooky. This feels like the right case > >> for an alias to me. > > > > OR if it's as important a use case as Pierre-Yves claims, add an > > option. E.g. make > > > > hg fold [-r] REV... > > > > (as implemented in my patch) the default. Then add > > > > hg fold --from REV > > > > to restore the current (REV::.) behaviour. > > > > Hmmm. Time for a poll? Here are the options: > > > > 1) status quo: "hg fold REV" and "hg fold -r REV" are different > > > > 2) my proposal: "hg fold [-r] REV...", with --from option > > to satisfy people who like the current "hg fold REV" semantics > > > > 3) Pierre-Yves' proposal: "hg fold [-r] REV" same as current > > "hg fold REV", with "hg fold --only REV..." option to > > satisfy people who like the current "hg fold -r REV..." semantics > > > > I'm pretty sure no one will vote for #1. Go ahead, surprise me! > > > > Sounds like Sean, Augie, and I are for #2. [snip] > > Clearly Pierre-Yves likes #3. Who else? > > No one. Pierre-Yves is dangerously close to: I like #3, and I have two votes from a git users that it would be a good thing. I also got a vote for #3 from two seasoned git users to whom I described this feature. I have one vote against from a user who is new to git and hg and has used the current behaviour in error and doesn't know how to recover from it easily. I think there is a general principle that could be invoked here, though: explicit is better than implicit. It would be best if there is no implicit magic with "." that can confuse people like my n00b who doesn't know how to easily recover from these folds If you really wanted to fold with ".", then you have to explicitly say it, at least by default. Looks like I just talked myself into #2 as well... Huh. - Jordi G. H.
TL;DR; new user (1) are working copy centric and should (2) should not be exposed to "revset" when unnecessary. On 06/24/2014 02:02 AM, Greg Ward wrote: > On 23 June 2014, Pierre-Yves David said: >> On 06/22/2014 04:52 PM, Greg Ward wrote: >>> # HG changeset patch >>> # User Greg Ward <greg@gerg.ca> >>> # Date 1403481137 14400 >>> # Sun Jun 22 19:52:17 2014 -0400 >>> # Node ID 4ab7a80fc11f275c03d4ddb94936a0688b71e6bc >>> # Parent 2fbba0bf7e7c8cbff1f94bc95c4d6214df85ef81 >>> fold: take an explicit list of revisions (BC) >> >> No thanks. > > Darn. I guess I misunderstood our previous discussion about this. > >> This use the be the default and ended up being very confusing. The >> vast majority of fold operation involve the working directory and an >> ancestors (something a descendant). The default should reflect that. >> Also, we should not requires user to have a PhD in revset to use Mercurial. > > "REV::" is not a PhD-level revset. Ok, it's not a super complicated revset, but it is a revset (or at least a "magic" invocation. The principle I'm defending here is "simple action should be simple to do". Fold is a command that also target new user with basic knownledge of mercurial. I strongly believe those users should not be exposed to ".^::." if we can avoid it. As a side benefit, delaying the exposure to "x::y" lower the odds user will confuse it with "x:y" and saws his own leg by mistake. >> So, I'm deeply convince the default should be "fold between . and REV". > > OK, fine. I happen to disagree, but whatever. The logic here is that people usually works from a working copy perspective. commit: record they working copy, merge: add other change in they working copy, update: change content of working copy to anyther changeset, rebase: (by default) move changesets under working copy to another head, graft: get a commit and put it on the working copy parent, tag: tags the working copy parent revert: put content in the working copy parent (we can also add uncommit to the list) You have to be a fairly advance user of Mercurial to start thinking about the whole graph and performs more abstract action to change its shape. I believe such users will have no trouble using a special flag in such case. The first version of fold had the behavior you describe, but seeing tens of Logilab people using it convinced me that the current default was a better choice for new users. The few usage of `hg fold` I'm seeing at facebook goes in the same direction. >> How ever I agree than having different behavior for `hg fold REV` >> and `hg fold --rev REV` sucks. > > OK, I'll see if I can fix that. > >> I also agree that the "do an in >> memory fold of unrelated changeset" usecase is nice. We should think >> about a dedicated flag for this case. > > --only? Example usage: > > hg fold --only R1 R2 ... Rn > > combines R1, R2, and Rn to make a new changeset. "--only" would work. JordiGH is proposing "--exact" I'm fan of neither but I think I like --exact more.
On 06/24/2014 07:57 PM, Augie Fackler wrote: > On Mon, Jun 23, 2014 at 09:02:14PM -0400, Greg Ward wrote: >> On 23 June 2014, Pierre-Yves David said: >>> On 06/22/2014 04:52 PM, Greg Ward wrote: >>>> # HG changeset patch >>>> # User Greg Ward <greg@gerg.ca> >>>> # Date 1403481137 14400 >>>> # Sun Jun 22 19:52:17 2014 -0400 >>>> # Node ID 4ab7a80fc11f275c03d4ddb94936a0688b71e6bc >>>> # Parent 2fbba0bf7e7c8cbff1f94bc95c4d6214df85ef81 >>>> fold: take an explicit list of revisions (BC) >>> >>> No thanks. >> >> Darn. I guess I misunderstood our previous discussion about this. >> >>> This use the be the default and ended up being very confusing. The >>> vast majority of fold operation involve the working directory and an >>> ancestors (something a descendant). The default should reflect that. >>> Also, we should not requires user to have a PhD in revset to use Mercurial. >> >> "REV::" is not a PhD-level revset. >> >>> So, I'm deeply convince the default should be "fold between . and REV". >> >> OK, fine. I happen to disagree, but whatever. > > I agree with Greg here - the magic default of 'hg fold $REV' -> fold > together $REV::. feels a little spooky. This feels like the right case > for an alias to me. > > (It's been stated by many that histedits behavior in this area is > confusing, and with the benefit of hindsight and the existence of > evolution I'm starting to come around on that point.) Do you mean the fact that histedit will work on "REV::."? I see this as a bit different. First, now that we use "REV::." instead of "heads(REV)::." the behavior is much less disturbing. Second, I feel like the current behavior is not too bad. In most case people want to rewrite the history they are based on, so having that by default sounds sensible. What histedit currently lack is a --exact flag (kind of like rebase have a --rev). Third, histedit is an advanced command for advance users. Such people have already a deeper understanding of DVCS graph, more confidence in doing graph change unrelated to working directory parent and have less trouble with revset. The `hg fold` command is target at basic users too. (I had a fourth but I got interrupted and lost it)
On 06/26/2014 01:47 PM, Greg Ward wrote: > On 24 June 2014, Augie Fackler said: >> I agree with Greg here - the magic default of 'hg fold $REV' -> fold >> together $REV::. feels a little spooky. This feels like the right case >> for an alias to me. > > OR if it's as important a use case as Pierre-Yves claims, add an > option. E.g. make > > hg fold [-r] REV... > > (as implemented in my patch) the default. Then add > > hg fold --from REV > > to restore the current (REV::.) behaviour. > > Hmmm. Time for a poll? Here are the options: In my opinion, this thread as already growth far too much. It is time to slow down and take the smallest sensible step. Lets fix the very confusing behavior between `hg fold REV` and `hg fold --rev REV` and keep the two options. From there we can see how often people get to add this --exact flag and how angry they when they have to do it. Because for now I just have the Logilab data set where about every body gave me a strange look when introduced to `hg fold .^::.` We'll definitely revisit the UI before freezing it. hopefully with more data at that point.
Patch
diff --git a/hgext/evolve.py b/hgext/evolve.py --- a/hgext/evolve.py +++ b/hgext/evolve.py @@ -2072,32 +2072,22 @@ lockmod.release(lock, wlock) @command('^fold|squash', - [('r', 'rev', [], _("explicitly specify the full set of revision to fold")), + [('r', 'rev', [], _('revisions to fold'), _('REV')), ] + commitopts + commitopts2, # allow to choose the seed ? - _('rev')) + _('[-r] REV...')) def fold(ui, repo, *revs, **opts): - """Fold multiple revisions into a single one + """combine multiple revisions into a single successor - The revisions from your current working directory to the given one are folded - into a single successor revision. - - you can alternatively use --rev to explicitly specify revisions to be folded, - ignoring the current working directory parent. + The specified revisions are combined to create a single successor + revision that includes the changes from all of them. """ revs = list(revs) - if revs: - if opts.get('rev', ()): - raise util.Abort("cannot specify both --rev and a target revision") - targets = scmutil.revrange(repo, revs) - revs = repo.revs('(%ld::.) or (.::%ld)', targets, targets) - elif 'rev' in opts: - revs = scmutil.revrange(repo, opts['rev']) - else: - revs = () + revs.extend(opts['rev']) if not revs: - ui.write_err('no revision to fold\n') - return 1 + raise util.Abort('no revisions specified') + revs = scmutil.revrange(repo, revs) + roots = repo.revs('roots(%ld)', revs) if len(roots) > 1: raise util.Abort("set has multiple roots") diff --git a/tests/test-evolve.t b/tests/test-evolve.t --- a/tests/test-evolve.t +++ b/tests/test-evolve.t @@ -614,12 +614,9 @@ $ rm *.orig $ hg fold - no revision to fold - [1] - $ hg fold 6 --rev 10 - abort: cannot specify both --rev and a target revision + abort: no revisions specified [255] - $ hg fold 6 # want to run hg fold 6 + $ hg fold 6:: 2 changesets folded 1 files updated, 0 files merged, 0 files removed, 0 files unresolved $ glog @@ -735,7 +732,7 @@ Test fold with commit messages $ cd ../work - $ hg fold .^ --message "Folding with custom commit message" + $ hg fold -r '(.^)::' --message "Folding with custom commit message" 2 changesets folded 1 files updated, 0 files merged, 0 files removed, 0 files unresolved $ glog @@ -754,7 +751,7 @@ > commit message > EOF - $ hg fold .^ --logfile commit-message + $ hg fold .^ -r . --logfile commit-message 2 changesets folded 1 files updated, 0 files merged, 0 files removed, 0 files unresolved $ hg qlog diff --git a/tests/test-tutorial.t b/tests/test-tutorial.t --- a/tests/test-tutorial.t +++ b/tests/test-tutorial.t @@ -473,25 +473,22 @@ The tutorial part is not written yet but can use `hg fold`: $ hg help fold - hg fold rev + hg fold [-r] REV... aliases: squash - Fold multiple revisions into a single one + combine multiple revisions into a single successor - The revisions from your current working directory to the given one are - folded into a single successor revision. - - you can alternatively use --rev to explicitly specify revisions to be - folded, ignoring the current working directory parent. + The specified revisions are combined to create a single successor revision + that includes the changes from all of them. options: - -r --rev VALUE [+] explicitly specify the full set of revision to fold - -m --message TEXT use text as commit message - -l --logfile FILE read commit message from file - -d --date DATE record the specified date as commit date - -u --user USER record the specified user as committer + -r --rev REV [+] revisions to fold + -m --message TEXT use text as commit message + -l --logfile FILE read commit message from file + -d --date DATE record the specified date as commit date + -u --user USER record the specified user as committer [+] marked option can be specified multiple times