Aaron Schrab <aaron@xxxxxxxxxx> 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@xxxxxxxxxx> > --- > 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");
