Junio C Hamano wrote:
> Jakub Narebski <jnareb@xxxxxxxxx> writes:
>
>> Replace a few doublequoted strings by theirs singlequoted equivalent,
>> lose doublequotes around variable in string containing only
>> of a variable name, use '' consistently as an empty string (and not
>> sometimes as "").
>
> Why? I do not in particular like a micro-cleanup like this (it
> seems more of personal taste than cleanup).
IMVHO consistency (always use '' to denote empty string... or always
use "") is a cleanup.
But I'm not that attached to this patch. It could stay or it could
go... One of the reasons for this patch was (most probably wrong)
idea from dealing with PHP that using single quotes makes program
a tiny bit faster.
>> - } elsif ($char eq "\\") {
>> + } elsif ($char eq '\\') {
>> $diff_class = " incomplete";
>> }
>
> Especially this makes a shell scripter think twice before
> realizing that this is Perl and a backslash expands inside
> single quotes. In other words, I find that the former is easier
> to read.
O.K. Although you can always escape singlequote inside single
quotes, so escape character also has to be escaped.
>> @@ -1052,7 +1052,7 @@ sub git_get_projects_list {
>> my $dir = $projects_list . ($filter ? "/$filter" : '');
>> # remove the trailing "/"
>> $dir =~ s!/+$!!;
>> - my $pfxlen = length("$dir");
>> + my $pfxlen = length($dir);
>
> On the other hand, I think this makes the code easier to read.
There are only two such chunks. Feel free to pick them...
>> @@ -3616,7 +3616,7 @@ sub git_snapshot {
>>
>> print $cgi->header(
>> -type => "application/$ctype",
>> - -content_disposition => 'inline; filename="' . "$filename" . '"',
>> + -content_disposition => 'inline; filename="' . $filename . '"',
>> -status => '200 OK');
>>
>
> Wouldn't it be easier to read if we did
>
> "inline; filename=\"$filename\""
>
> instead?
It would. Right.
--
Jakub Narebski
Poland
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html