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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 9, 2016, 01:06 UTC
Message-ID: <xmqqshr19oxp.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161108120823.11204-2-vascomalmeida@sapo.pt>

Vasco Almeida <vascomalmeida@sapo.pt> writes:

Show 38 quoted lines

> Add subroutines prefix_lines and comment_lines.
>
> Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
> ---
>  perl/Git.pm | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/perl/Git.pm b/perl/Git.pm
> index b2732822a..17be59fb7 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -1438,6 +1438,29 @@ sub END {
>  
>  } # %TEMP_* Lexical Context
>  
> +=item prefix_lines ( PREFIX, STRING )
> +
> +Prefixes lines in C<STRING> with C<PREFIX>.
> +
> +=cut
> +
> +sub prefix_lines {
> +	my ($prefix, $string) = @_;
> +	$string =~ s/^/$prefix/mg;
> +	return $string;
> +}
> +
> +=item comment_lines ( STRING )
> +
> +Comments lines following core.commentchar configuration.
> +
> +=cut
> +
> +sub comment_lines {
> +	my $comment_line_char = config("core.commentchar") || '#';
> +	return prefix_lines("$comment_line_char ", @_);
> +}
> +

This makes it appear as if comment_lines can take arbitrary number of strings as its arguments (because the outer caller just passes @_ thru), but in fact because prefix_lines ignores anything other than $_[0] and $_[1], only the first parameter given to comment_lineS sub is inspected for lines in it and the prefix-char prefixed at the beginning of each of them.

Which is not a great interface, as it is quite misleading.

Perhaps

	prefix_lines("#", join("\n", @_));

or something like that may make it less confusing.