* [PATCH 1/3] gitweb: Whitespace cleanup - tabs are for indent, spaces are for align (3)
2007-04-01 20:21 [PATCH 0/3] gitweb: Few cleanup patches Jakub Narebski
@ 2007-04-01 20:21 ` Jakub Narebski
2007-04-01 20:22 ` [PATCH 2/3] gitweb: Quote hash keys, and do not use barewords keys Jakub Narebski
2007-04-01 20:22 ` [PATCH 3/3] gitweb: Few doublequoted strings cleanups Jakub Narebski
2 siblings, 0 replies; 6+ messages in thread
From: Jakub Narebski @ 2007-04-01 20:21 UTC (permalink / raw)
To: git
Code should be look the same way, regardless of tab size.
Use tabs for indent, but spaces for align.
Indent continued part of command spanning multiple lines, but only once.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 48 ++++++++++++++++++++++++------------------------
1 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d1f4aeb..5c49056 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1800,7 +1800,7 @@ EOF
$cgi->hidden(-name => "a") . "\n" .
$cgi->hidden(-name => "h") . "\n" .
$cgi->popup_menu(-name => 'st', -default => 'commit',
- -values => ['commit', 'author', 'committer', 'pickaxe']) .
+ -values => ['commit', 'author', 'committer', 'pickaxe']) .
$cgi->sup($cgi->a({-href => href(action=>"search_help")}, "?")) .
" search:\n",
$cgi->textfield(-name => "s", -value => $searchtext) . "\n" .
@@ -3097,7 +3097,7 @@ sub git_summary {
git_project_list_body(\@forklist, undef, 0, 15,
$#forklist <= 15 ? undef :
$cgi->a({-href => href(action=>"forks")}, "..."),
- 'noheader');
+ 'noheader');
}
git_footer_html();
@@ -3204,7 +3204,7 @@ HTML
my $rev = substr($full_rev, 0, 8);
my $author = $meta->{'author'};
my %date = parse_date($meta->{'author-time'},
- $meta->{'author-tz'});
+ $meta->{'author-tz'});
my $date = $date{'iso-tz'};
if ($group_size) {
$current_color = ++$current_color % $num_colors;
@@ -3216,9 +3216,9 @@ HTML
print " rowspan=\"$group_size\"" if ($group_size > 1);
print ">";
print $cgi->a({-href => href(action=>"commit",
- hash=>$full_rev,
- file_name=>$file_name)},
- esc_html($rev));
+ hash=>$full_rev,
+ file_name=>$file_name)},
+ esc_html($rev));
print "</td>\n";
}
open (my $dd, "-|", git_cmd(), "rev-parse", "$full_rev^")
@@ -3227,13 +3227,13 @@ HTML
close $dd;
chomp($parent_commit);
my $blamed = href(action => 'blame',
- file_name => $meta->{'filename'},
- hash_base => $parent_commit);
+ file_name => $meta->{'filename'},
+ hash_base => $parent_commit);
print "<td class=\"linenr\">";
print $cgi->a({ -href => "$blamed#l$orig_lineno",
- -id => "l$lineno",
- -class => "linenr" },
- esc_html($lineno));
+ -id => "l$lineno",
+ -class => "linenr" },
+ esc_html($lineno));
print "</td>";
print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
print "</tr>\n";
@@ -3623,7 +3623,7 @@ sub git_snapshot {
my $name = $project;
$name =~ s/\047/\047\\\047\047/g;
open my $fd, "-|",
- "$git archive --format=tar --prefix=\'$name\'/ $hash | $command"
+ "$git archive --format=tar --prefix=\'$name\'/ $hash | $command"
or die_error(undef, "Execute git-tar-tree failed");
binmode STDOUT, ':raw';
print <$fd>;
@@ -3736,7 +3736,7 @@ sub git_commit {
# difftree output is not printed for merges
open my $fd, "-|", git_cmd(), "diff-tree", '-r', "--no-commit-id",
@diff_opts, $parent, $hash, "--"
- or die_error(undef, "Open git-diff-tree failed");
+ or die_error(undef, "Open git-diff-tree failed");
@difftree = map { chomp; $_ } <$fd>;
close $fd or die_error(undef, "Reading git-diff-tree failed");
}
@@ -4308,13 +4308,13 @@ sub git_search {
if ($page > 0) {
$paging_nav .=
$cgi->a({-href => href(action=>"search", hash=>$hash,
- searchtext=>$searchtext, searchtype=>$searchtype)},
- "first");
+ searchtext=>$searchtext, searchtype=>$searchtype)},
+ "first");
$paging_nav .= " ⋅ " .
$cgi->a({-href => href(action=>"search", hash=>$hash,
- searchtext=>$searchtext, searchtype=>$searchtype,
- page=>$page-1),
- -accesskey => "p", -title => "Alt-p"}, "prev");
+ searchtext=>$searchtext, searchtype=>$searchtype,
+ page=>$page-1),
+ -accesskey => "p", -title => "Alt-p"}, "prev");
} else {
$paging_nav .= "first";
$paging_nav .= " ⋅ prev";
@@ -4322,9 +4322,9 @@ sub git_search {
if ($#commitlist >= 100) {
$paging_nav .= " ⋅ " .
$cgi->a({-href => href(action=>"search", hash=>$hash,
- searchtext=>$searchtext, searchtype=>$searchtype,
- page=>$page+1),
- -accesskey => "n", -title => "Alt-n"}, "next");
+ searchtext=>$searchtext, searchtype=>$searchtype,
+ page=>$page+1),
+ -accesskey => "n", -title => "Alt-n"}, "next");
} else {
$paging_nav .= " ⋅ next";
}
@@ -4332,9 +4332,9 @@ sub git_search {
if ($#commitlist >= 100) {
$next_link =
$cgi->a({-href => href(action=>"search", hash=>$hash,
- searchtext=>$searchtext, searchtype=>$searchtype,
- page=>$page+1),
- -accesskey => "n", -title => "Alt-n"}, "next");
+ searchtext=>$searchtext, searchtype=>$searchtype,
+ page=>$page+1),
+ -accesskey => "n", -title => "Alt-n"}, "next");
}
git_print_page_nav('','', $hash,$co{'tree'},$hash, $paging_nav);
--
1.5.0.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] gitweb: Quote hash keys, and do not use barewords keys
2007-04-01 20:21 [PATCH 0/3] gitweb: Few cleanup patches Jakub Narebski
2007-04-01 20:21 ` [PATCH 1/3] gitweb: Whitespace cleanup - tabs are for indent, spaces are for align (3) Jakub Narebski
@ 2007-04-01 20:22 ` Jakub Narebski
2007-04-01 20:22 ` [PATCH 3/3] gitweb: Few doublequoted strings cleanups Jakub Narebski
2 siblings, 0 replies; 6+ messages in thread
From: Jakub Narebski @ 2007-04-01 20:22 UTC (permalink / raw)
To: git
Ensure that in all references to an elemento of a hash the value
of key is singlequoted, instead of using bareword: use $hash{'key'}
instead of $hash{key}
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 5c49056..eab8f3f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -19,7 +19,7 @@ use File::Basename qw(basename);
binmode STDOUT, ':utf8';
BEGIN {
- CGI->compile() if $ENV{MOD_PERL};
+ CGI->compile() if $ENV{'MOD_PERL'};
}
our $cgi = new CGI;
@@ -1870,16 +1870,16 @@ sub git_print_page_nav {
my %arg = map { $_ => {action=>$_} } @navs;
if (defined $head) {
for (qw(commit commitdiff)) {
- $arg{$_}{hash} = $head;
+ $arg{$_}{'hash'} = $head;
}
if ($current =~ m/^(tree | log | shortlog | commit | commitdiff | search)$/x) {
for (qw(shortlog log)) {
- $arg{$_}{hash} = $head;
+ $arg{$_}{'hash'} = $head;
}
}
}
- $arg{tree}{hash} = $treehead if defined $treehead;
- $arg{tree}{hash_base} = $treebase if defined $treebase;
+ $arg{'tree'}{'hash'} = $treehead if defined $treehead;
+ $arg{'tree'}{'hash_base'} = $treebase if defined $treebase;
print "<div class=\"page_nav\">\n" .
(join " | ",
@@ -1927,9 +1927,9 @@ sub git_print_header_div {
my ($action, $title, $hash, $hash_base) = @_;
my %args = ();
- $args{action} = $action;
- $args{hash} = $hash if $hash;
- $args{hash_base} = $hash_base if $hash_base;
+ $args{'action'} = $action;
+ $args{'hash'} = $hash if $hash;
+ $args{'hash_base'} = $hash_base if $hash_base;
print "<div class=\"header\">\n" .
$cgi->a({-href => href(%args), -class => "title"},
--
1.5.0.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] gitweb: Few doublequoted strings cleanups
2007-04-01 20:21 [PATCH 0/3] gitweb: Few cleanup patches Jakub Narebski
2007-04-01 20:21 ` [PATCH 1/3] gitweb: Whitespace cleanup - tabs are for indent, spaces are for align (3) Jakub Narebski
2007-04-01 20:22 ` [PATCH 2/3] gitweb: Quote hash keys, and do not use barewords keys Jakub Narebski
@ 2007-04-01 20:22 ` Jakub Narebski
2007-04-03 20:20 ` Junio C Hamano
2 siblings, 1 reply; 6+ messages in thread
From: Jakub Narebski @ 2007-04-01 20:22 UTC (permalink / raw)
To: git
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 "").
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 86 ++++++++++++++++++++++++++--------------------------
1 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index eab8f3f..9f116a2 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -36,7 +36,7 @@ our $GIT = "++GIT_BINDIR++/git";
our $projectroot = "++GITWEB_PROJECTROOT++";
# target of the home link on top of all pages
-our $home_link = $my_uri || "/";
+our $home_link = $my_uri || '/';
# string of the home link on top of all pages
our $home_link_str = "++GITWEB_HOME_LINK_STR++";
@@ -377,7 +377,7 @@ if (defined $searchtype) {
# now read PATH_INFO and use it as alternative to parameters
sub evaluate_path_info {
return if defined $project;
- my $path_info = $ENV{"PATH_INFO"};
+ my $path_info = $ENV{'PATH_INFO'};
return if !$path_info;
$path_info =~ s,^/+,,;
return if !$path_info;
@@ -403,7 +403,7 @@ sub evaluate_path_info {
# we got "project.git/branch:filename" or "project.git/branch:dir/"
# we could use git_get_type(branch:pathname), but it needs $git_dir
$pathname =~ s,^/+,,;
- if (!$pathname || substr($pathname, -1) eq "/") {
+ if (!$pathname || substr($pathname, -1) eq '/') {
$action ||= "tree";
$pathname =~ s,/$,,;
} else {
@@ -515,10 +515,10 @@ sub href(%) {
for (my $i = 0; $i < @mapping; $i += 2) {
my ($name, $symbol) = ($mapping[$i], $mapping[$i+1]);
if (defined $params{$name}) {
- push @result, $symbol . "=" . esc_param($params{$name});
+ push @result, $symbol . '=' . esc_param($params{$name});
}
}
- $href .= "?" . join(';', @result) if scalar @result;
+ $href .= '?' . join(';', @result) if scalar @result;
return $href;
}
@@ -876,7 +876,7 @@ sub format_ref_marker {
if ($markers) {
return ' <span class="refs">'. $markers . '</span>';
} else {
- return "";
+ return '';
}
}
@@ -900,17 +900,17 @@ sub format_diff_line {
my $line = shift;
my ($from, $to) = @_;
my $char = substr($line, 0, 1);
- my $diff_class = "";
+ my $diff_class = '';
chomp $line;
if ($char eq '+') {
$diff_class = " add";
- } elsif ($char eq "-") {
+ } elsif ($char eq '-') {
$diff_class = " rem";
- } elsif ($char eq "@") {
+ } elsif ($char eq '@') {
$diff_class = " chunk_header";
- } elsif ($char eq "\\") {
+ } elsif ($char eq '\\') {
$diff_class = " incomplete";
}
$line = untabify($line);
@@ -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);
my ($check_forks) = gitweb_check_feature('forks');
@@ -1074,7 +1074,7 @@ sub git_get_projects_list {
$File::Find::prune = 1;
}
},
- }, "$dir");
+ }, $dir);
} elsif (-f $projects_list) {
# read from file(url-encoded):
@@ -1166,7 +1166,7 @@ sub git_get_last_activity {
}
sub git_get_references {
- my $type = shift || "";
+ my $type = shift || '';
my %refs;
# 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c refs/tags/v2.6.11
# c39ae07f393806ccf406ef966e9a15afc43cc36a refs/tags/v2.6.11^{}
@@ -1261,7 +1261,7 @@ sub parse_tag {
} elsif ($line =~ m/--BEGIN/) {
push @comment, $line;
last;
- } elsif ($line eq "") {
+ } elsif ($line eq '') {
last;
}
}
@@ -1323,7 +1323,7 @@ sub parse_commit_text {
foreach my $title (@commit_lines) {
$title =~ s/^ //;
- if ($title ne "") {
+ if ($title ne '') {
$co{'title'} = chop_str($title, 80, 5);
# remove leading stuff of merges to make the interesting part visible
if (length($title) > 50) {
@@ -1346,7 +1346,7 @@ sub parse_commit_text {
last;
}
}
- if ($co{'title'} eq "") {
+ if ($co{'title'} eq '') {
$co{'title'} = $co{'title_short'} = '(no commit message)';
}
# remove added spaces
@@ -1781,7 +1781,7 @@ EOF
my ($have_search) = gitweb_check_feature('search');
if ((defined $project) && ($have_search)) {
if (!defined $searchtext) {
- $searchtext = "";
+ $searchtext = '';
}
my $search_hash;
if (defined $hash_base) {
@@ -1803,7 +1803,7 @@ EOF
-values => ['commit', 'author', 'committer', 'pickaxe']) .
$cgi->sup($cgi->a({-href => href(action=>"search_help")}, "?")) .
" search:\n",
- $cgi->textfield(-name => "s", -value => $searchtext) . "\n" .
+ $cgi->textfield(-name => 's', -value => $searchtext) . "\n" .
"</div>" .
$cgi->end_form() . "\n";
}
@@ -2003,7 +2003,7 @@ sub git_print_log ($;%) {
shift @$log;
}
# remove leading empty lines
- while (defined $log->[0] && $log->[0] eq "") {
+ while (defined $log->[0] && $log->[0] eq '') {
shift @$log;
}
@@ -2027,7 +2027,7 @@ sub git_print_log ($;%) {
# print only one empty line
# do not print empty line after signoff
- if ($line eq "") {
+ if ($line eq '') {
next if ($empty || $signoff);
$empty = 1;
} else {
@@ -2220,7 +2220,7 @@ sub git_difftree_body {
$from_file_type = file_type($diff{'from_mode'});
}
- if ($diff{'status'} eq "A") { # created
+ if ($diff{'status'} eq 'A') { # created
my $mode_chng = "<span class=\"file_status new\">[new $to_file_type";
$mode_chng .= " with mode: $to_mode_str" if $to_mode_str;
$mode_chng .= "]</span>";
@@ -2242,7 +2242,7 @@ sub git_difftree_body {
"blob");
print "</td>\n";
- } elsif ($diff{'status'} eq "D") { # deleted
+ } elsif ($diff{'status'} eq 'D') { # deleted
my $mode_chng = "<span class=\"file_status deleted\">[deleted $from_file_type]</span>";
print "<td>";
print $cgi->a({-href => href(action=>"blob", hash=>$diff{'from_id'},
@@ -2270,8 +2270,8 @@ sub git_difftree_body {
"history");
print "</td>\n";
- } elsif ($diff{'status'} eq "M" || $diff{'status'} eq "T") { # modified, or type changed
- my $mode_chnge = "";
+ } elsif ($diff{'status'} eq 'M' || $diff{'status'} eq 'T') { # modified, or type changed
+ my $mode_chnge = '';
if ($diff{'from_mode'} != $diff{'to_mode'}) {
$mode_chnge = "<span class=\"file_status mode_chnge\">[changed";
if ($from_file_type ne $to_file_type) {
@@ -2320,10 +2320,10 @@ sub git_difftree_body {
"history");
print "</td>\n";
- } elsif ($diff{'status'} eq "R" || $diff{'status'} eq "C") { # renamed or copied
+ } elsif ($diff{'status'} eq 'R' || $diff{'status'} eq 'C') { # renamed or copied
my %status_name = ('R' => 'moved', 'C' => 'copied');
my $nstatus = $status_name{$diff{'status'}};
- my $mode_chng = "";
+ my $mode_chng = '';
if ($diff{'from_mode'} != $diff{'to_mode'}) {
# mode also for directories, so we cannot use $to_mode_str
$mode_chng = sprintf(", mode: %04o", $to_mode_oct & 0777);
@@ -2434,14 +2434,14 @@ sub git_patchset_body {
}
$from{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
$to{'file'} = $diffinfo->{'to_file'} || $diffinfo->{'file'};
- if ($diffinfo->{'status'} ne "A") { # not new (added) file
+ if ($diffinfo->{'status'} ne 'A') { # not new (added) file
$from{'href'} = href(action=>"blob", hash_base=>$hash_parent,
hash=>$diffinfo->{'from_id'},
file_name=>$from{'file'});
} else {
delete $from{'href'};
}
- if ($diffinfo->{'status'} ne "D") { # not deleted file
+ if ($diffinfo->{'status'} ne 'D') { # not deleted file
$to{'href'} = href(action=>"blob", hash_base=>$hash,
hash=>$diffinfo->{'to_id'},
file_name=>$to{'file'});
@@ -2577,12 +2577,12 @@ sub git_project_list_body {
}
($pr->{'age'}, $pr->{'age_string'}) = @aa;
if (!defined $pr->{'descr'}) {
- my $descr = git_get_project_description($pr->{'path'}) || "";
+ my $descr = git_get_project_description($pr->{'path'}) || '';
$pr->{'descr_long'} = to_utf8($descr);
$pr->{'descr'} = chop_str($descr, 25, 5);
}
if (!defined $pr->{'owner'}) {
- $pr->{'owner'} = get_file_owner("$projectroot/$pr->{'path'}") || "";
+ $pr->{'owner'} = get_file_owner("$projectroot/$pr->{'path'}") || '';
}
if ($check_forks) {
my $pname = $pr->{'path'};
@@ -2926,10 +2926,10 @@ sub git_search_grep_body {
my $comment = $co{'comment'};
foreach my $line (@$comment) {
if ($line =~ m/^(.*)($searchtext)(.*)$/i) {
- my $lead = esc_html($1) || "";
+ my $lead = esc_html($1) || '';
$lead = chop_str($lead, 30, 10);
- my $match = esc_html($2) || "";
- my $trail = esc_html($3) || "";
+ my $match = esc_html($2) || '';
+ my $trail = esc_html($3) || '';
$trail = chop_str($trail, 30, 10);
my $text = "$lead<span class=\"match\">$match</span>$trail";
print chop_str($text, 80, 5) . "<br/>\n";
@@ -3056,7 +3056,7 @@ sub git_summary {
foreach my $git_url (@url_list) {
next unless $git_url;
print "<tr><td>$url_tag</td><td>$git_url</td></tr>\n";
- $url_tag = "";
+ $url_tag = '';
}
print "</table>\n";
@@ -3381,7 +3381,7 @@ sub git_blob_plain {
}
} elsif ($hash =~ m/^[0-9a-fA-F]{40}$/) {
# blobs defined by non-textual hash id's can be cached
- $expires = "+1d";
+ $expires = '+1d';
}
my $type = shift;
@@ -3391,7 +3391,7 @@ sub git_blob_plain {
$type ||= blob_mimetype($fd, $file_name);
# save as filename, even when no $file_name is given
- my $save_as = "$hash";
+ my $save_as = $hash;
if (defined $file_name) {
$save_as = $file_name;
} elsif ($type =~ m/^text\//) {
@@ -3399,8 +3399,8 @@ sub git_blob_plain {
}
print $cgi->header(
- -type => "$type",
- -expires=>$expires,
+ -type => $type,
+ -expires => $expires,
-content_disposition => 'inline; filename="' . "$save_as" . '"');
undef $/;
binmode STDOUT, ':raw';
@@ -3423,7 +3423,7 @@ sub git_blob {
}
} elsif ($hash =~ m/^[0-9a-fA-F]{40}$/) {
# blobs defined by non-textual hash id's can be cached
- $expires = "+1d";
+ $expires = '+1d';
}
my ($have_blame) = gitweb_check_feature('blame');
@@ -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');
my $git = git_cmd_str();
@@ -3744,7 +3744,7 @@ sub git_commit {
# non-textual hash id's can be cached
my $expires;
if ($hash =~ m/^[0-9a-fA-F]{40}$/) {
- $expires = "+1d";
+ $expires = '+1d';
}
my $refs = git_get_references();
my $ref = format_ref_marker($refs, $co{'id'});
@@ -4129,7 +4129,7 @@ sub git_commitdiff {
# non-textual hash id's can be cached
my $expires;
if ($hash =~ m/^[0-9a-fA-F]{40}$/) {
- $expires = "+1d";
+ $expires = '+1d';
}
# write commit message
@@ -4157,7 +4157,7 @@ sub git_commitdiff {
-type => 'text/plain',
-charset => 'utf-8',
-expires => $expires,
- -content_disposition => 'inline; filename="' . "$filename" . '"');
+ -content_disposition => 'inline; filename="' . $filename . '"');
my %ad = parse_date($co{'author_epoch'}, $co{'author_tz'});
print <<TEXT;
From: $co{'author'}
--
1.5.0.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] gitweb: Few doublequoted strings cleanups
2007-04-01 20:22 ` [PATCH 3/3] gitweb: Few doublequoted strings cleanups Jakub Narebski
@ 2007-04-03 20:20 ` Junio C Hamano
2007-04-04 9:10 ` Jakub Narebski
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2007-04-03 20:20 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
Jakub Narebski <jnareb@gmail.com> 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).
> - } 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.
> @@ -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.
> @@ -1261,7 +1261,7 @@ sub parse_tag {
> } elsif ($line =~ m/--BEGIN/) {
> push @comment, $line;
> last;
One thing I noticed is that we seem to have needless 'm' as in
the above m/--BEGIN/ in some places. My preferences are (this
falls within "personal taste" category):
* Unless the pattern has slashes, drop 'm' for match, always
use '|' as the delimiter for substitution s/A/B;
* If the pattern has slashes, consistently use the same
alternate letter as the delimiter (gitweb seems to use '!').
> @@ -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?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] gitweb: Few doublequoted strings cleanups
2007-04-03 20:20 ` Junio C Hamano
@ 2007-04-04 9:10 ` Jakub Narebski
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Narebski @ 2007-04-04 9:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> 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
^ permalink raw reply [flat|nested] 6+ messages in thread