Thread (287 messages) 287 messages, 11 authors, 2018-10-08

Re: [PATCH 2/8] sequencer: introduce the `merge` command

From: Phillip Wood <hidden>
Date: 2018-01-31 17:59:03

On 31/01/18 13:48, Johannes Schindelin wrote:
Hi Jake & Phillip,

On Mon, 29 Jan 2018, Johannes Schindelin wrote:
quoted
On Sat, 20 Jan 2018, Jacob Keller wrote:
quoted
On Fri, Jan 19, 2018 at 6:45 AM, Phillip Wood [off-list ref] wrote:
quoted
On 18/01/18 15:35, Johannes Schindelin wrote:
quoted
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.
quoted
quoted
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
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help