Re: [PATCH 2/8] sequencer: introduce the `merge` command
From: Jacob Keller <hidden>
Date: 2018-02-01 06:40:33
On Wed, Jan 31, 2018 at 5:48 AM, Johannes Schindelin [off-list ref] 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 commitThat 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 messageI 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. The -m option just opens such a can of worms with dequoting, that's why I do not want to do that.
I agree, I don't see a need for "-m".
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>I like this, especially given you added the "#" for one of the other new commands as well, (reset I think?)
and
merge -C <original> <to-merge> <to-merge2>...
# Merging: <oneline>
# Merging: <oneline2>
# ...I really like this, since you can show each oneline for all the to-merges for an octopus.
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 specifically. Thoughts?
I think it's fine to disallow # as a label. Thanks, Jake
Ciao, Dscho