Re: [PATCH] rebase -x: sanity check command

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 28, 2019, 18:23 UTC
Message-ID: <xmqqd0ogbpzn.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190128102620.18222-1-phillip.wood@talktalk.net>

Phillip Wood <phillip.wood@talktalk.net> writes:

Show 9 quoted lines

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> If the user gives an empty argument to --exec then the rebase starts to
> run before erroring out with
>
>   error: missing arguments for exec
>   error: invalid line 2: exec
>   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'.

Hmph. I do agree that the above makes an unfortunate end-user experience, but I would sort-of imagine that it would even be nicer for such an empty exec to behave as if it were "exec false" but with less severe error message, i.e. a way for the user to say "I want to break the sequence here and get an interactive session". We may not even need to add the "break" insn if we go that way and there is one less thing for users to learn. I dunno, but I tend to prefer giving a useful and safe behaviour to interactive users other than erroring out, when there _is_ such a safe behaviour that is obvious from the situation, and I feel that an empty "exec" is such a case.

Show 6 quoted lines

> Also check that the command does not contain any newlines as the
> todo-list format is unable to cope with multiline commands. Note that
> this changes the behavior, before this change one could do
>
> git rebase --exec='echo one
> exec echo two'

It is very good to check the input, regardless of what an empty "exec" should do.