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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 31, 2018 at 5:48 AM, Johannes Schindelin
<Johannes.Schindelin@xxxxxx> 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@xxxxxxxxxxxx> 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.
>
> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux