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

From: phillip.wood123@gmail.com <phillip.wood123@gmail.com>
Date: Apr 5, 2024, 09:30 UTC
Message-ID: <dbb4615c-443f-474f-a33c-cfb4486d436b@gmail.com>
In-Reply-To: <57462d2e-e255-4d35-b778-176f03675df0@gmail.com>

Hi Rubén

Thanks for trying this out.

On 04/04/2024 20:44, Rubén Justo wrote:

Show 16 quoted lines

> 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.

Best Wishes

Phillip