[PATCH rfc] merge: present merge part labels to user in prompts

[Simon Farnsworth]

On 16/03/2016, 18:30, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org> wrote:


>On 03/16/2016 04:59 PM, Simon Farnsworth wrote:
>> # HG changeset patch
>> # User Simon Farnsworth <simonfar at fb.com>
>> # Date 1458172710 0
>> #      Wed Mar 16 23:58:30 2016 +0000
>> # Node ID 85202d019e4f6d4093fdbb35b0aff105db1767b4
>> # Parent  70c2f8a982766b512e9d7f41f2d93fdb92f5481f
>> merge: present merge part labels to user in prompts
>>
>> "local", "remote" and "other" are not always clear; we rename them in
>> conflict markers to try and clarify what's meant by each label. Present the
>> conflict marker names in the merge prompts, too.
>
>I did a very very fast pass through this here is a couple of high level 
>feedback:
>
>- It is usually good to provide example of the output change you do in 
>the commit description. This help people getting and idea of what is the 
>change doing without digging down to the tests,

Will do.

>
>- This changesets seems to be doingg multiple things:
>   - use labels for changed/deleted prompt
>   - adding the labels to the mergestate file (and support in 
>debugmergestate)
>   - reused the label in the merge state when using `hg resolve`
>
>   You should probably slice this out in multiple patch. to make each 
>patch simpler and clearer o review.

Will do. The single patch thing is a consequence of my personal workflow (build the feature, then split it into sensible parts).

>
>- As we are at changing this prompt message should we get ride of the 
>"remote" terms (as we already have local/other). (there might be good 
>reason not to)

Digging through the history, it looks like "remote" was another attempt to be friendlier than "other". I think we should consistently use local/other, and rely on commands using friendly labels.

Will get rid of "remote".

>
>- There might be a lot of file in a merge state, I would move the 
>'labels:' section of debugmergestate before them

Done.

>
>- I'm not sure about the '[...]' do we use that anywhere? why not ()?

I'm not committed to [...]; the reason I'm using it is that we use (l)ocal to tell you that "l" is an acceptable response to a prompt; I don't want someone surprised when they see "(l)ocal (dest)", type in "dest" at the prompt, and get a bad answer.

>
>- Do we alway have only two labels? We never allow to change the "base" 
>label?

We do permit changes to the base label, AFAICT, we just don't use it outside of conflict markers. This is like the option to have different bases for each file in conflict; it's allowed by the code, we just don't use it.

As we never prompt with the "base" label, I don't use it, but I do preserve it if present, in case future changes introduce a way to use "base".

Simon