From: Phillip Wood <phillip.wood@talktalk.net>
Date: Wed, 31 Jan 2018 17:58:53 GMT
Subject: Re: [PATCH 2/8] sequencer: introduce the `merge` command
Message-ID: <640c350d-090c-ad0f-e6be-fc577bd184c2@talktalk.net>
In-Reply-To: <nycvar.QRO.7.76.6.1801311441430.35@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>

On 31/01/18 13:48, Johannes Schindelin wrote:
> Hi Jake & Phillip,
> 
> On Mon, 29 Jan 2018, Johannes Schindelin wrote:
> 
>> On Sat, 20 Jan 2018, Jacob Keller wrote:
>>
>>> On Fri, Jan 19, 2018 at 6:45 AM, Phillip Wood <phillip.wood@talktalk.net> wrote:
>>>> On 18/01/18 15:35, Johannes Schindelin wrote:
>>>>>
>>>>> This patch adds the `merge` command, with the following syntax:
>>>>>
>>>>>       merge <commit> <rev> <oneline>
>>>>
>>>> I'm concerned that this will be confusing for users. All of the other
>>>> rebase commands replay the changes in the commit hash immediately
>>>> following the command name. This command instead uses the first
>>>> commit to specify the message which is different to both 'git merge'
>>>> and the existing rebase commands. I wonder if it would be clearer to
>>>> have 'merge -C <commit> <rev> ...' instead so it's clear which
>>>> argument specifies the message and which the remote head to merge.
>>>> It would also allow for 'merge -c <commit> <rev> ...' in the future
>>>> for rewording an existing merge message and also avoid the slightly
>>>> odd 'merge - <rev> ...'. Where it's creating new merges I'm not sure
>>>> it's a good idea to encourage people to only have oneline commit
>>>> messages by making it harder to edit them, perhaps it could take
>>>> another argument to mean open the editor or not, though as Jake said
>>>> I guess it's not that common.
>>>
>>> I actually like the idea of re-using commit message options like -C,
>>> -c,  and -m, so we could do:
>>>
>>> merge -C <commit> ... to take message from commit
>>
>> That is exactly how the Git garden shears do it.
>>
>> I found it not very readable. That is why I wanted to get away from it in
>> --recreate-merges.
> 
> I made up my mind. Even if it is not very readable, it is still better
> than the `merge A B` where the order of A and B magically determines their
> respective roles.
> 
>>> merge -c <commit> ...  to take the message from commit and open editor to edit
>>> merge -m "<message>" ... to take the message from the quoted test
>>> merge ... to merge and open commit editor with default message
> 
> I will probably implement -c, but not -m, and will handle the absence of
> the -C and -c options to construct a default merge message which can then
> be edited.

That sounds like a good plan (-c can always be added later), I'm really
pleased you changed your mind on this, having the -C may be a bit ugly
but I think it is valuable to have some way of distinguishing the
message commit from the merge heads.

> The -m option just opens such a can of worms with dequoting, that's why I
> do not want to do that.
> 
> BTW I am still trying to figure out how to present the oneline of the
> commit to merge (which is sometimes really helpful because the label might
> be less than meaningful) while *still* allowing for octopus merges.
> 
> So far, what I have is this:
> 
> 	merge <original> <to-merge> <oneline>
> 
> and for octopus:
> 
> 	merge <original> "<to-merge> <to-merge2>..." <oneline>...
> 
> I think with the -C syntax, it would become something like
> 
> 	merge -C <original> <to-merge> # <oneline>
> 
> and
> 
> 	merge -C <original> <to-merge> <to-merge2>...
> 	# Merging: <oneline>
> 	# Merging: <oneline2>
> 	# ...
> 
> The only qualm I have about this is that `#` really *is* a valid ref name.
> (Seriously, it is...). So that would mean that I'd have to disallow `#`
> as a label specificially.
> 
> Thoughts?

As ':' is not a valid ref if you want a separator you could have

	merge -C <original> <to-merge> : <oneline>

personally I'm not sure what value having a separator adds in this case.
I think in the octopus case have a separate comment line for the subject
of each merge head is a good idea - maybe the two head merge could just
have the subject of the remote head in a comment below. I wonder if
having the subject of the commit that is going to be used for the
message may be a useful prompt in some cases but that's just making
things more complicated.

Best Wishes

Phillip

> Ciao,
> Dscho
>