Re: [PATCH] Allow custom "comment char"

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 15, 2013, 19:12 UTC
Message-ID: <7v622y45wy.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1358275827-5244-1-git-send-email-ralf.thielow@gmail.com>

Ralf Thielow <ralf.thielow@gmail.com> writes:

Show 22 quoted lines

> From: Junio C Hamano <gitster@pobox.com>
>
> Some users do want to write a line that begin with a pound sign, #,
> in their commit log message.  Many tracking system recognise
> a token of #<bugid> form, for example.
>
> The support we offer these use cases is not very friendly to the end
> users.  They have a choice between
>
>  - Don't do it.  Avoid such a line by rewrapping or indenting; and
>
>  - Use --cleanup=whitespace but remove all the hint lines we add.
>
> Give them a way to set a custom comment char, e.g.
>
>     $ git -c core.commentchar="%" commit
>
> so that they do not have to do either of the two workarounds.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
> ---

It would have helped if you said you finished the NEEDSWORK: in builtin/branch.c in the earlier draft with strbuf_commented_* functions ;-)

Looks like a good progress overall, except for nits here and there.

Show 13 quoted lines

> diff --git a/builtin/notes.c b/builtin/notes.c
> index 453457a..5e84e35 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -92,10 +92,7 @@ static const char * const git_notes_get_ref_usage[] = {
>  };
>  
>  static const char note_template[] =
> -	"\n"
> -	"#\n"
> -	"# Write/edit the notes for the following object:\n"
> -	"#\n";
> +	"Write/edit the notes for the following object:";

I think this (and its use site that manually adds "\n#\n") is a symptom of strbuf_commented_add*() function not designed right. When it iterates over lines and adds each of them in a commented out form, it could check if the line is an empty one and refrain from adding a trailing SP if that is the case. Then this can become

    "\nWrite/edit the notes...\n\n";

You have to create the "\n" blank line at the beginning manually, but that is logically outside the commented out block, so it is not a problem.

Show 10 quoted lines

> @@ -181,11 +172,16 @@ static void create_note(const unsigned char *object, struct msg_arg *msg,
>  			write_or_die(fd, msg->buf.buf, msg->buf.len);
>  		else if (prev && !append_only)
>  			write_note_data(fd, prev);
> -		write_or_die(fd, note_template, strlen(note_template));
> +
> +		strbuf_addf(&buf, "\n%c\n", comment_line_char);
> +		strbuf_commented_addstr(&buf, note_template);
> +		strbuf_addf(&buf, "\n%c\n", comment_line_char);
> +		write_or_die(fd, buf.buf, buf.len);

Show 20 quoted lines

> diff --git a/builtin/tag.c b/builtin/tag.c
> index 9c3e067..e1b72be 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -246,19 +246,13 @@ static int do_sign(struct strbuf *buffer)
>  }
>  
>  static const char tag_template[] =
> -	N_("\n"
> -	"#\n"
> -	"# Write a tag message\n"
> -	"# Lines starting with '#' will be ignored.\n"
> -	"#\n");
> +	N_("Write a tag message\n"
> +	"Lines starting with '%c' will be ignored.");
> ...
> +			else
> +				strbuf_commented_addf(&buf, _(tag_template_nocleanup), comment_line_char);
> +			strbuf_addf(&buf, "\n%c\n", comment_line_char);
> +			write_or_die(fd, buf.buf, buf.len);

Same here.

Show 12 quoted lines

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 22ec5b6..1b8d95f 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -975,13 +975,19 @@ cmd_summary() {
>  		echo
>  	done |
>  	if test -n "$for_status"; then
> +		comment_char=`git config core.commentchar`
> +		if [ ! -n "$comment_char" ]; then
> +			comment_char='#'
> +		elif [ ${#comment_char} -gt 1 ]; then

Not portable, I think.

> +		echo "$comment_char"
> +		sed -e "s|^|$comment_char |" -e "s|^$comment_char $|$comment_char|"

Can $comment_char be a '|'?

Show 18 quoted lines

> diff --git a/strbuf.c b/strbuf.c
> index 9a373be..8af4b4f 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -204,6 +204,44 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
>  	va_end(ap);
>  }
>  
> +void strbuf_commented_addstr(struct strbuf *sb, const char *s)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	struct strbuf prefix = STRBUF_INIT;
> +
> +	strbuf_addf(&prefix, "%c ", comment_line_char);
> +	strbuf_addstr(&buf, s);
> +	strbuf_add_lines(sb, prefix.buf, buf.buf, buf.len);
> +
> +	// remove additional '\n' added by strbuf_add_lines()

No C++ comments.