Re: [PATCH v2] sequencer: rectify empty hint in call of require_clean_work_tree()

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 9, 2023, 01:24 UTC
Message-ID: <xmqqcyzx6ma1.fsf@gitster.g>
In-Reply-To: <20230807170935.2336730-1-oswald.buddenhagen@gmx.de>

Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:

> The canonical way to represent "no error hint" is making it null, which
> shortcuts the error() call altogether. This fixes the output by removing
> the line which said just "error:".

Sorry, but I forgot to point out a rather obvious thing in my review. We would want to see a reproduction recipe described here in the proposed log message at least. Even better is an addition to an existing test to ensure that there is no such empty "error:" line, which will make sure that we will notice when anybody by mistake (this includes a mismerge of other topics) breaks this fix.

Thanks.

Show 16 quoted lines

>  sequencer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index cc9821ece2..d15a7409d8 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -6182,7 +6182,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
>  	if (checkout_onto(r, opts, onto_name, &oid, orig_head))
>  		goto cleanup;
>  
> -	if (require_clean_work_tree(r, "rebase", "", 1, 1))
> +	if (require_clean_work_tree(r, "rebase", NULL, 1, 1))
>  		goto cleanup;
>  
>  	todo_list_write_total_nr(&new_todo);