[PATCH evolve-ext] fold: add argument to handle ambiguous case

Durham Goode durham at fb.com
Tue Jun 2 22:32:04 CDT 2015



On 5/31/15 1:03 PM, Pierre-Yves David wrote:
>
>
> On 05/30/2015 02:28 AM, Gilles Moris wrote:
>> Le 30/05/2015 03:29, Pierre-Yves David a écrit :
>>>
>>>
>>> On 05/28/2015 03:26 PM, Matt Mackall wrote:
>>>
>>> [reordered]
>>>
>>> > It seems to me that both --exact and --chain are unnecessarily
>>> > complicated. We should really consider an interface that's simply:
>>> >
>>> >   hg fold start::end
>>> >
>>> > with no magic defaults.
>>>
>>> Lets write some history here...
>>>
>>> In the dawn of time, the behavior was:
>>>   hg fold <revset>
>>>  -> fold everything in <revset> together.
>>>   (This match the `hg fold start::end` from Matt.)
>>>
>>> There was two issue with this UI:
>>>
>>> 1) It pretty much requires the knowledge of revset to be used
>>> efficiently. This put the barrier of entry higher than necessary for a
>>> fairly basic command.
>>>
>>> 2) The most common case turned out to be, I want to fold the last N
>>> commits. This happen commonly when one is working toward something,
>>> making frequent wip commit along the way to be able to easily go back
>>> to a check point. So the common case was `hg fold X::.`
>>>
>>> (This two principle seems important to: (1) do not requires revset
>>> knowledge to ensure a low barrier of entry (2) default case should be
>>> easy).
>>>
>>>
>>> So, we move the default to :
>>>
>>>   hg fold <rev>
>>>  -> fold everything between '.' and <rev>.
>>>
>>>   hg fold --exact <revset>
>>>   -> fold everything in the revset.
>>>
>>> I think that putting the advanced use case of 'folding anywhere' is an
>>> okay choice.
>>>
>>> Most of mercurial command behave according the working directory parent
>>>
>>> - commit -> make a commit as child as '.'
>>> - amend -> add content to '.'
>>> - uncommit -> remove content to '.'
>>> - hg split -> split '.' into multiple changeset (in the future)
>>> - diff -> show difference with '.'
>>> - revert -> restore file as in '.'
>>> - merge -> merge '.' with something else
>>> - rebase -> rebase '.' somewhere else (by default)
>>> - graft -> get a new commit on top of '.'
>>> - histedit -> histedit from '.' to X
>>>   (would be nice to have an '--exact' here actually)
>>> - hg evolve -> evolve stuff related to '.'
>>>
>>> So I think it at most sensible (and at best consistent) with the other
>>> commands than the default involves the working copy.
>>>
>>> [former message start]
>>>
>>>>> hg fold 9 # will fold 9 and 10 together, unchanged
>>>>
>>>> Ok.
>>>>
>>>>> hg fold 8 + 7 # before was folding 7,8,9,10, now prints an error
>>>>
>>>> What is the user story where this is what's wanted? If the answer is
>>>> "never" (which I suspect), then we should just make this do what's
>>>> expected: fold the specified versions with each other.
>>>
>>> I believe "8+7" is only used here as an example. It is a place holder
>>> for 'anycomplicatedrevset()'. In that case, the revset result could be
>>> said "visually obvious" but the internal code have no clue about.
>>>
>>> My personal philosophy is:
>>>
>>>   "We should lever change behavior accord length of a revset result"
>>>
>>> When I run a command with '::myfeature - public()' I dunno if this is
>>> going to return 0, 1, 2 or N changeset. But I still want to be able to
>>> build alias for it and run command without double checking revset
>>> lengh. Therefore, command cannot change there behavior regarding
>>> revset query result because that will blow in my face all the time.
>>>
>>>>> hg fold 8 + 7 --chain # folds 7,8,9,10 together
>>>>
>>>> Why would anybody type that rather than:
>>>>
>>>>   hg fold --chain 7
>>>
>>> CF, previous comment. Nobody is going to type 7+8. But revset result
>>> will be 7+8
>>>
>>>> Adding + 8 doesn't add any additional info except maybe "I'm trying to
>>>> do something different and am about to have a sad."
>>>
>>
>> I am not convinced that it needs 2 flags, or even 1.
>>
>> Forgetting about the current UI, we could expect that providing a single
>> commit would fold up to the working dir. If a complex revset is
>> provided, I guess the power user could do "first(revset)". If we really
>> want a flag for safety, we could use -f/--from: "hg fold -f 7".
>> Then if a revset yields multiple commits, I would expect the --exact
>> behavior.
>
> I'll insist on the fact we cannot rely on 'number of commit' because 
> revset is not "predicable enough".
>
> Basic user will start provided simple revsets with "non-predicable" 
> result, and you should not expect them to be smart enough to add 
> "first" like element to it until it suddenly break for unclear (to 
> them reason) and stop trusting the tool.
>
> Moreover, You have way to force the 'multiple -> one' way. But you 
> have no way to go the other way. Lets look at this command command:
>
> - hg fold --exact 'draft() and ::mybook'
>
> Current behavior (with --exact)
>
> revs | result
> -------------
>  0-1 | abort 'nothing to fold here'
>  2+  | fold all revs together
>
> This is a safe sensible and predictable behavior.
>
> With 'auto detect revset size proposal'
>
>
> revs | result
> -------------
>  0   | abort 'no rev'
>  0-1 | fold 'revs::.'
>  2+  | fold all revs together
>
> This is unpredictable behavior that will blow in people face.
>
> So, think of the children, do not relying on the size of the revset 
> result.
>
If a user specifies a single commit (via revset, or node, or whatever), 
can we error out and say "you only specified one commit. did you mean: 
hg fold X::." ?  That keeps the syntax precise (fold exactly what I say, 
don't compute any closures), and teaches the user about revsets.


More information about the Mercurial-devel mailing list