Re: [PATCH 0/2] Fix --rebase-merges with custom commentChar

[Junio C Hamano <gitster@pobox.com>]

Aaron Schrab <aaron@schrab.com> writes:

> Subject: [PATCH v2] sequencer: use configured comment character
>
> Use the configured comment character when generating comments about
> branches in a todo list.  Failure to honor this configuration causes a
> failure to parse the resulting todo list.

OK.

>
> Note that the comment_line_char has already been resolved by this point,
> even if the user has configured the comment character to be selected
> automatically.

Isn't this a slight lie?

The core.commentchar=auto setting is noticed by everybody (including
the users of the sequencer machinery), but it is honored only by
builtin/commit.c::prepare_to_commit() that is called by
builtin/commit.c::cmd_commit(), i.e. the implementation of "git
commit" that should not be used as a subroutine by other commands,
and by nothing else.  If the user has core.commentchar=auto, the
comment_line_char is left to the default '#' in the sequencer
codepath.

I think the patch is still correct and safe, but the reason why it
is so is not because we chose a suitable character (that is how I
read what "has already been resolved by this point" means) by
calling builtin/commit.c::adjust_comment_line_char().  Isn't it
because the "script" the function is working on does not have a line
that came from arbitrary end-user input that may happen to begin
with '#', hence the default '#' is safe to use?

> Signed-off-by: Aaron Schrab <aaron@schrab.com>
> ---
>  sequencer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 4034c0461b..caf91af29d 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -3991,7 +3991,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
>  		entry = oidmap_get(&state.commit2label, &commit->object.oid);
>  
>  		if (entry)
> -			fprintf(out, "\n# Branch %s\n", entry->string);
> +			fprintf(out, "\n%c Branch %s\n", comment_line_char, entry->string);
>  		else
>  			fprintf(out, "\n");