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 is part of the effort to reimplement `--preserve-merges` with >> a substantially improved design, a design that has been developed in the >> Git for Windows project to maintain the dozens of Windows-specific patch >> series on top of upstream Git. >> >> The previous patch implemented the `label`, `bud` and `reset` commands >> to label commits and to reset to a labeled commits. 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 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 This also, I think, allows us to not need to put the oneline on the end, meaning we wouldn't have to quote the parent commit arguments since we could use option semantics? > > One thought that just struck me - if a merge or reset command specifies > an invalid label is it rescheduled so that it's still in the to-do list > when the user edits it after rebase stops? > > In the future it might be nice if the label, reset and merge commands > were validated when the to-do list is parsed so that the user gets > immediate feedback if they try to create a label that is not a valid ref > name or that they have a typo in a name given to reset or merge rather > than the rebase stopping later. >