Re: [PATCH v6 01/16] Git.pm: add subroutines for commenting lines

2 messages, 2 authors, 2016-12-13 · open the first message on its own page

Re: [PATCH v6 01/16] Git.pm: add subroutines for commenting lines

From: Junio C Hamano <hidden>
Date: 2016-12-10 22:09:36

Vasco Almeida [off-list ref] writes:
I wonder why this is important when Git errors out when
core.commentChar is set to more than 1 characters or 0 characters.
I think it should be consistent with the way core.commentchar is
treated in the rest of the system, namely this bit from config.c:

	if (!strcmp(var, "core.commentchar")) {
		if (!value)
			return config_error_nonbool(var);
		else if (!strcasecmp(value, "auto"))
			auto_comment_line_char = 1;
		else if (value[0] && !value[1]) {
			comment_line_char = value[0];
			auto_comment_line_char = 0;
		} else
			return error("core.commentChar should only be one character");
		return 0;
	}

And I think I misread this piece of code.  

We only update comment_line_char from the default "#" when the
configured value is a single-byte character and we ignore incorrect
values in the configuration file.  So I think the patch you sent is
correct after all.

Re: [PATCH v6 01/16] Git.pm: add subroutines for commenting lines

From: Vasco Almeida <hidden>
Date: 2016-12-13 11:17:09

A Sáb, 10-12-2016 às 14:09 -0800, Junio C Hamano escreveu:
We only update comment_line_char from the default "#" when the
configured value is a single-byte character and we ignore incorrect
values in the configuration file.  So I think the patch you sent is
correct after all.
I am still not sure what version do we prefer.

Check whether core.commentchar is a single character. If not, use '#'
as the $comment_line_char.

+sub get_comment_line_char {
+       my $comment_line_char = config("core.commentchar") || '#';
+       $comment_line_char = '#' if ($comment_line_char eq 'auto');
+       $comment_line_char = '#' if (length($comment_line_char) != 1);
+       return $comment_line_char;
+}

Check whether core.commentchar is a single character. If not, use the
first character of the core.commentchar value, mirroring the "rebase
-i" behavior introduced recently.

+sub get_comment_line_char {
+       my $comment_line_char = config("core.commentchar") || '#';
+       $comment_line_char = '#' if ($comment_line_char eq 'auto');
+       if (length($comment_line_char) != 1) {
+               # use first character
+               $comment_line_char = substr($comment_line_char, 0, 1);
+       }
+       return $comment_line_char;
+}

Or akin to what I had in the first patch related to handling 'auto'
value of core.commentchar configuration variable:

+sub get_comment_line_char {
+       my $comment_line_char = config("core.commentchar") || '#';
+       $comment_line_char = '#' if ($comment_line_char eq 'auto');
+       return $comment_line_char;
+}

Which assumes that the value of core.commentchar configuration variable
is either 'auto' or one single character, or the variable is not
defined.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help