Re: [PATCH] rebase -i: improve error message when picking merge

["Rubén Justo" <rjusto@gmail.com>]

On Fri, Apr 05, 2024 at 10:30:37AM +0100, phillip.wood123@gmail.com wrote:
> Hi Rubén
> 
> Thanks for trying this out.
> 
> On 04/04/2024 20:44, Rubén Justo wrote:
> > On Mon, Feb 26, 2024 at 10:58:07AM +0000, Phillip Wood via GitGitGadget wrote:
> > The change makes sense, but this is confusing to me:
> > 
> > With this ...
> > 
> > 	$ GIT_EDITOR='echo pick 17381ab62a >' ./git rebase -i HEAD
> > 	error: 'pick' does not accept merge commits, please use 'merge -C'
> > 	error: invalid line 1: pick 17381ab62a
> > 	You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'.
> > 	Or you can abort the rebase with 'git rebase --abort'.
> > 
> > ... I find these repeated messages confusing:
> > 
> > 	$ GIT_EDITOR=: ./git rebase --edit-todo
> > 	error: 'pick' does not accept merge commits, please use 'merge -C'
> > 	error: invalid line 1: pick 17381ab62a
> 
> The two lines above are printed when "rebase --edit-todo" loads the todo
> list for the user to edit. With a real editor rather than ":" we would then
> print
> 
>     hint: Waiting for your editor to close the file ...
> 
> Then hopefully the user would fix the errors. If not then when the editor
> had finished we'd print the remaining errors  as below.
> 
> > 	error: 'pick' does not accept merge commits, please use 'merge -C'
> > 	error: invalid line 1: pick 17381ab62a
> > 	You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'.
> > 	Or you can abort the rebase with 'git rebase --abort'.
> 
> I think that printing the error messages when the todo list is parsed before
> it is given to the user to edit is helpful as it reminds the user what the
> problem is. Your test looks confusing because it doesn't really simulate the
> user editing the todo list.

Certainly, the test was not clear to express my confusion.

The ones that are printed _before_ the editor is run are the ones that
confuse me, because when the user exits the editor we leave those
lines there:

	$ GIT_EDITOR='sed -i s/pick/merge/' ./git rebase --edit
	error: 'pick' does not accept merge commits, please use 'merge -C'
	error: invalid line 1: pick 17381ab62a

But maybe it is my interpretation.  Your reasoning of giving it as a
help to the user makes sense.

Thank you for your explanations.

> Best Wishes
> 
> Phillip