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

Vasco Almeida [off-list ref] writes:

A Sex, 11-11-2016 às 11:45 -0100, Vasco Almeida escreveu:

quoted

+=item comment_lines ( STRING [, STRING... ])
+
+Comments lines following core.commentchar configuration.
+
+=cut
+
+sub comment_lines {
+       my $comment_line_char = config("core.commentchar") || '#';
+       return prefix_lines("$comment_line_char ", @_);
+}
+

In light of the recent "Fix problems with rebase -i when
core.commentchar is defined" [1], I realized that this patch does not
handle the 'auto' value of core.commentchat configuration variable.

I propose to do the patch below in the next re-roll.

[1] http://www.mail-archive.com/git@vger.kernel.org/msg107818.html

The incremental update below looks sensible.  We'd also want to
protect this codepath from a misconfigured two-or-more byte sequence
in core.commentchar, I would suspect, to be consistent.

A Ter, 22-11-2016 às 09:42 -0800, Junio C Hamano escreveu:

The incremental update below looks sensible.  We'd also want to
protect this codepath from a misconfigured two-or-more byte sequence
in core.commentchar, I would suspect, to be consistent.

Are the below changes alright for what you propose? It just checks if
the length of core.commentchar's value is 1, otherwise use '#' as the
comment_line_char.
As a note, when I set core.commentchar with "git config
core.commentChar 'batata'", I get the following error message when I
issue "git add -i":

error: core.commentChar should only be one character
fatal: bad config variable 'core.commentchar' in file '.git/config' at line 6

-- >8 --

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 3a6d846..4e0ab5a 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl

@@ -1072,7 +1072,7 @@ sub edit_hunk_manually {

 	print $fh @$oldtext;
 	my $is_reverse = $patch_mode_flavour{IS_REVERSE};
 	my ($remove_plus, $remove_minus) = $is_reverse ? ('-', '+') : ('+', '-');
-	my $comment_line_char = Git::config("core.commentchar") || '#';
+	my $comment_line_char = Git::get_comment_line_char;
 	print $fh Git::comment_lines sprintf(__ <<EOF, $remove_minus, $remove_plus, $comment_line_char),
 ---
 To remove '%s' lines, make them ' ' lines (context).

diff --git a/perl/Git.pm b/perl/Git.pm
index 69cd1dd..e4da913 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm

@@ -1451,6 +1451,20 @@ sub prefix_lines {

 	return $string;
 }
 
+=item get_comment_line_char ( )
+
+Gets the core.commentchar configuration value.
+The value fallbacks to # if core.commentchar is set to 'auto'.
+
+=cut
+
+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;
+}
+
 =item comment_lines ( STRING [, STRING... ])
 
 Comments lines following core.commentchar configuration.

@@ -1458,7 +1472,7 @@ Comments lines following core.commentchar configuration.

 =cut
 
 sub comment_lines {
-	my $comment_line_char = config("core.commentchar") || '#';
+	my $comment_line_char = get_comment_line_char;
 	return prefix_lines("$comment_line_char ", @_);
 }
 

Hi Vasco,

On Fri, 9 Dec 2016, Vasco Almeida wrote:

A Ter, 22-11-2016 às 09:42 -0800, Junio C Hamano escreveu:

quoted

The incremental update below looks sensible.  We'd also want to
protect this codepath from a misconfigured two-or-more byte sequence
in core.commentchar, I would suspect, to be consistent.

Are the below changes alright for what you propose? It just checks if
the length of core.commentchar's value is 1, otherwise use '#' as the
comment_line_char.
As a note, when I set core.commentchar with "git config
core.commentChar 'batata'", I get the following error message when I
issue "git add -i":

error: core.commentChar should only be one character
fatal: bad config variable 'core.commentchar' in file '.git/config' at line 6

This is exactly the same issue I fixed for rebase -i recently.

Good eyes,
Dscho