Re: [PATCH] sequencer: use configured comment character

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 29, 2018, 14:12 UTC
Message-ID: <nycvar.QRO.7.76.6.1806291607501.74@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqh8lmwsso.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Thu, 28 Jun 2018, Junio C Hamano wrote:

Show 25 quoted lines

> Aaron Schrab <aaron@schrab.com> writes:
> 
> > Use configured comment character when generating comments about branches
> > in an instruction sheet.  Failure to honor this configuration causes a
> > failure to parse the resulting instruction sheet.
> >
> > 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");
> 
> Would this interact OK with core.commentchar set to "auto"?

The idea of "auto" is:

	If set to "auto", `git-commit` would select a character that is not
	the beginning character of any line in existing commit messages.

As there are no pre-existing lines in that script (apart from the ones we are about to add with the todo_help), the setting "auto" is pretty moot and we will fall back to the default comment char (or, if there was a previous core.commentChar that was parsed, that one).

In short: the code is fine, but yes, I had to convince myself by looking through the code. (Hinting at a possible improvement of the commit message.)

Ciao, Dscho