# [PATCH 0/7] gitweb: Cleanups, fixes and small improvements 11 messages from 2006-08-26 to 2006-08-27. Participants: Jakub Narebski, Junio C Hamano. Thread: https://gitlist.dev/t/5397 ## Jakub Narebski, 2006-08-26 17:13 Subject: [PATCH 0/7] gitweb: Cleanups, fixes and small improvements Message-ID: <1156612392716-git-send-email-jnareb@gmail.com> URL: https://gitlist.dev/e/1156612392716-git-send-email-jnareb%40gmail.com ``` This series contains fixes, improvements and cleanups. Fixes are patch 2 and 4, with 4 more important. This series is based on next (885a89a077c2c79e1fac81c57711566ec30f9bc1) --- gitweb/gitweb.perl | 75 +++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 54 insertions(+), 21 deletions(-) Shortlog: gitweb: Restore old git_blame using git-annotate under "annotate" gitweb: Remove workaround for git-diff bug fixed in f82cd3c gitweb: Improve comments about gitweb features configuration gitweb: Fix typo in git_patchset_body gitweb: Use 'local $/ = undef;' before 'print <$fd>;' gitweb: blobs defined by non-textual hash ids can be cached gitweb: Always link to plain version of the blob ``` ## Jakub Narebski, 2006-08-26 17:14 Subject: [PATCH 1/7] gitweb: Restore old git_blame using git-annotate under "annotate" Message-ID: <11566124661001-git-send-email-jnareb@gmail.com> URL: https://gitlist.dev/e/11566124661001-git-send-email-jnareb%40gmail.com In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com> ``` Rename git_blame to git_annotate, and git_blame2 to git_blame. Link git_annotate under "annotate" action. Add link to "blame in git_annotate, and to "annotate" in git_blame. Signed-off-by: Jakub Narebski --- gitweb/gitweb.perl | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index e00a6ed..abd0f0d 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -229,7 +229,8 @@ if (defined $searchtext) { # dispatch my %actions = ( - "blame" => \&git_blame2, + "blame" => \&git_blame, + "annotate" => \&git_annotate, "blobdiff" => \&git_blobdiff, "blobdiff_plain" => \&git_blobdiff_plain, "blob" => \&git_blob, @@ -2166,7 +2167,7 @@ sub git_tag { git_footer_html(); } -sub git_blame2 { +sub git_blame { my $fd; my $ftype; @@ -2193,6 +2194,9 @@ sub git_blame2 { $cgi->a({-href => href(action=>"blob", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)}, "blob") . " | " . + $cgi->a({-href => href(action=>"annotate", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)}, + "annotate") . + " | " . $cgi->a({-href => href(action=>"blame", file_name=>$file_name)}, "head"); git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav); @@ -2236,7 +2240,7 @@ HTML git_footer_html(); } -sub git_blame { +sub git_annotate { my $fd; if (!gitweb_check_feature('blame')) { @@ -2258,6 +2262,9 @@ sub git_blame { $cgi->a({-href => href(action=>"blob", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)}, "blob") . " | " . + $cgi->a({-href => href(action=>"blame", hash=>$hash, hash_base=>$hash_base, file_name=>$file_name)}, + "blame") . + " | " . $cgi->a({-href => href(action=>"blame", file_name=>$file_name)}, "head"); git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav); -- 1.4.1.1 ``` ## Jakub Narebski, 2006-08-26 17:14 Subject: [PATCH 2/7] gitweb: Remove workaround for git-diff bug fixed in f82cd3c Message-ID: <11566124672654-git-send-email-jnareb@gmail.com> URL: https://gitlist.dev/e/11566124672654-git-send-email-jnareb%40gmail.com In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com> ``` Remove workaround in git_blobdiff for error in git-diff (showing reversed diff for diff of blobs), corrected in commit f82cd3c Fix "git diff blob1 blob2" showing the diff in reverse. which is post 1.4.2-rc2 commit. Signed-off-by: Jakub Narebski --- gitweb/gitweb.perl | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index abd0f0d..df2f9e5 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -2841,8 +2841,7 @@ sub git_blobdiff { } # open patch output - #open $fd, "-|", $GIT, "diff", '-p', $hash_parent, $hash - open $fd, "-|", $GIT, "diff", '-p', $hash, $hash_parent + open $fd, "-|", $GIT, "diff", '-p', $hash_parent, $hash or die_error(undef, "Open git-diff failed"); } else { die_error('404 Not Found', "Missing one of the blob diff parameters") -- 1.4.1.1 ``` ## Jakub Narebski, 2006-08-26 17:14 Subject: [PATCH 3/7] gitweb: Improve comments about gitweb features configuration Message-ID: <11566124672096-git-send-email-jnareb@gmail.com> URL: https://gitlist.dev/e/11566124672096-git-send-email-jnareb%40gmail.com In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com> ``` Signed-off-by: Jakub Narebski --- gitweb/gitweb.perl | 25 ++++++++++++++++--------- 1 files changed, 16 insertions(+), 9 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index df2f9e5..c29ffa8 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -67,9 +67,16 @@ our $mimetypes_file = undef; # You define site-wide feature defaults here; override them with # $GITWEB_CONFIG as necessary. our %feature = ( - # feature => {'sub' => feature-sub, 'override' => allow-override, 'default' => [ default options...] - # if feature is overridable, feature-sub will be called with default options; - # return value indicates if to enable specified feature + # feature => { + # 'sub' => feature-sub (subroutine), + # 'override' => allow-override (boolean), + # 'default' => [ default options...] (array reference)} + # + # if feature is overridable (it means that allow-override has true value, + # then feature-sub will be called with default options as parameters; + # return value of feature-sub indicates if to enable specified feature + # + # use gitweb_check_feature() to check if is enabled 'blame' => { 'sub' => \&feature_blame, @@ -95,9 +102,9 @@ sub gitweb_check_feature { } # To enable system wide have in $GITWEB_CONFIG -# $feature{'blame'}{'default'} = [1]; -# To have project specific config enable override in $GITWEB_CONFIG -# $feature{'blame'}{'override'} = 1; +# $feature{'blame'}{'default'} = [1]; +# To have project specific config enable override in $GITWEB_CONFIG +# $feature{'blame'}{'override'} = 1; # and in project config gitweb.blame = 0|1; sub feature_blame { @@ -113,9 +120,9 @@ sub feature_blame { } # To disable system wide have in $GITWEB_CONFIG -# $feature{'snapshot'}{'default'} = [undef]; -# To have project specific config enable override in $GITWEB_CONFIG -# $feature{'blame'}{'override'} = 1; +# $feature{'snapshot'}{'default'} = [undef]; +# To have project specific config enable override in $GITWEB_CONFIG +# $feature{'blame'}{'override'} = 1; # and in project config gitweb.snapshot = none|gzip|bzip2 sub feature_snapshot { -- 1.4.1.1 ``` ## Jakub Narebski, 2006-08-26 17:14 Subject: [PATCH 4/7] gitweb: Fix typo in git_patchset_body Message-ID: <11566124672334-git-send-email-jnareb@gmail.com> URL: https://gitlist.dev/e/11566124672334-git-send-email-jnareb%40gmail.com In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com> ``` Signed-off-by: Jakub Narebski --- gitweb/gitweb.perl | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index c29ffa8..093deab 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1657,7 +1657,7 @@ sub git_patchset_body { print "
\n"; LINE: - while (my $patch_line @$fd>) { + while (my $patch_line = <$fd>) { chomp $patch_line; if ($patch_line =~ m/^diff /) { # "git diff" header -- 1.4.1.1 ``` ## Jakub Narebski, 2006-08-26 17:14 Subject: [PATCH 5/7] gitweb: Use 'local $/ = undef;' before 'print <$fd>;' Message-ID: <11566124673653-git-send-email-jnareb@gmail.com> URL: https://gitlist.dev/e/11566124673653-git-send-email-jnareb%40gmail.com In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com> ``` Signed-off-by: Jakub Narebski --- gitweb/gitweb.perl | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 093deab..eae83e6 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -2017,6 +2017,7 @@ sub git_project_list { if (-f $home_text) { print "
\n"; open (my $fd, $home_text); + local $/ = undef; print <$fd>; close $fd; print "
\n"; @@ -2395,11 +2396,10 @@ sub git_blob_plain { print $cgi->header(-type => "$type", -content_disposition => "inline; filename=\"$save_as\""); - undef $/; binmode STDOUT, ':raw'; + local $/ = undef; print <$fd>; binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi - $/ = "\n"; close $fd; } @@ -2585,6 +2585,7 @@ sub git_snapshot { open my $fd, "-|", "$GIT tar-tree $hash \'$project\' | $command" or die_error(undef, "Execute git-tar-tree failed."); binmode STDOUT, ':raw'; + local $/ = undef; print <$fd>; binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi close $fd; -- 1.4.1.1 ``` ## Jakub Narebski, 2006-08-26 17:14 Subject: [PATCH 6/7] gitweb: blobs defined by non-textual hash ids can be cached Message-ID: <11566124672100-git-send-email-jnareb@gmail.com> URL: https://gitlist.dev/e/11566124672100-git-send-email-jnareb%40gmail.com In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com> ``` Signed-off-by: Jakub Narebski --- gitweb/gitweb.perl | 20 +++++++++++++++++--- 1 files changed, 17 insertions(+), 3 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index eae83e6..f6175bb 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -2371,6 +2371,12 @@ sub git_heads { } sub git_blob_plain { + # blobs defined by non-textual hash id's can be cached + my $expires; + if ($hash =~ m/^[0-9a-fA-F]{40}$/) { + $expires = "+1d"; + } + if (!defined $hash) { if (defined $file_name) { my $base = $hash_base || git_get_head_hash($project); @@ -2394,8 +2400,10 @@ sub git_blob_plain { $save_as .= '.txt'; } - print $cgi->header(-type => "$type", - -content_disposition => "inline; filename=\"$save_as\""); + print $cgi->header( + -type => "$type", + -expires=>$expires, + -content_disposition => "inline; filename=\"$save_as\""); binmode STDOUT, ':raw'; local $/ = undef; print <$fd>; @@ -2404,6 +2412,12 @@ sub git_blob_plain { } sub git_blob { + # blobs defined by non-textual hash id's can be cached + my $expires; + if ($hash =~ m/^[0-9a-fA-F]{40}$/) { + $expires = "+1d"; + } + if (!defined $hash) { if (defined $file_name) { my $base = $hash_base || git_get_head_hash($project); @@ -2421,7 +2435,7 @@ sub git_blob { close $fd; return git_blob_plain($mimetype); } - git_header_html(); + git_header_html(undef, $expires); my $formats_nav = ''; if (defined $hash_base && (my %co = parse_commit($hash_base))) { if (defined $file_name) { -- 1.4.1.1 ``` ## Jakub Narebski, 2006-08-26 17:14 Subject: [PATCH 7/7] gitweb: Always link to plain version of the blob in git_blob. Message-ID: <1156612467711-git-send-email-jnareb@gmail.com> URL: https://gitlist.dev/e/1156612467711-git-send-email-jnareb%40gmail.com In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com> ``` Link to plain version of the blob. even if no valid hash_base is given in "blob" view (in git_blob). Signed-off-by: Jakub Narebski --- gitweb/gitweb.perl | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index f6175bb..02d327c 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -2461,8 +2461,13 @@ sub git_blob { git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav); git_print_header_div('commit', esc_html($co{'title'}), $hash_base); } else { + $formats_nav .= + $cgi->a({-href => href(action=>"blob_plain", + hash=>$hash, file_name=>$file_name)}, + "plain") . + print "
\n" . - "

\n" . + "
$formats_nav
\n" . "
$hash
\n"; } git_print_page_path($file_name, "blob", $hash_base); -- 1.4.1.1 ``` ## Junio C Hamano, 2006-08-26 20:58 Subject: Re: [PATCH 0/7] gitweb: Cleanups, fixes and small improvements Message-ID: <7vd5an1afz.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vd5an1afz.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <1156612392716-git-send-email-jnareb@gmail.com> ``` Jakub Narebski writes: * gitweb: Restore old git_blame using git-annotate under "annotate" I actually was hoping to see a patch to remove the git_blame which is not used as far as I can see. Although we carried annotate and blame in git.git tree for quite a while, the intention was always to deprecate one over the other once pros-and-cons of each implementation become clear (and I think people would not miss annotate if we remove annotate and make it an alias for blame -c anymore). What's the reason we would want to have both? * gitweb: Remove workaround for git-diff bug fixed in f82cd3c . gitweb: Fix typo in git_patchset_body I think you had separate patches for these; applied. * gitweb: Use 'local $/ = undef;' before 'print <$fd>;' You changed: $/ = undef; print <$fd>; ... hope that nobody depends on standard value of $/ ... around here, which may still break if you did sub ... calls, the sub did not localize $/ (who would?), ... and depended to have a sane $/. $/ = "\n"; to local $/ = undef; print <$fd>; ... hope that nobody depends on standard value of $/ ... until the end of scope, and whoever changes this ... sub is careful enough in the future which I think is worse. Introducing an extra scope explicitly delimit the part you want to use localized $/ like this { local $/; print <$fd>; } might have been more palatable. Am I guessing the reason of your change wrong? * gitweb: Always link to plain version of the blob Needs a better description to justify the change by describing in what way the current implementation is broken. The ones above I won't apply just yet (I first applied, merged into "next" and then commented on the earlier 19 series, but I'm not going to repeat that mistake again this time). The rest look OK so I'll apply them; [6/7] depended on [5/7] so I manually adjusted the context. ``` ## Jakub Narebski, 2006-08-26 21:18 Subject: Re: [PATCH 0/7] gitweb: Cleanups, fixes and small improvements Message-ID: URL: https://gitlist.dev/e/ecqdqt%24rcl%241%40sea.gmane.org In-Reply-To: <7vd5an1afz.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > Jakub Narebski writes: > > * gitweb: Restore old git_blame using git-annotate under "annotate" > >   I actually was hoping to see a patch to remove the git_blame >   which is not used as far as I can see. > >   Although we carried annotate and blame in git.git tree for >   quite a while, the intention was always to deprecate one over >   the other once pros-and-cons of each implementation become >   clear (and I think people would not miss annotate if we remove >   annotate and make it an alias for blame -c anymore).  What's >   the reason we would want to have both? Well, removing it completely might be not a best idea for now, as git_annotate has for example two more columns ("Age" and "Author") which might be usefull. I guess the intention is to provide patch for those who want to improve git_blame, to have comparison with git_annotate, i.e. patch not applied but available. > * gitweb: Use 'local $/ = undef;' before 'print <$fd>;' > >   You changed: > >         $/ = undef; >         print <$fd>; >         ... hope that nobody depends on standard value of $/ >         ... around here, which may still break if you did sub >         ... calls, the sub did not localize $/ (who would?), >         ... and depended to have a sane $/. >         $/ = "\n"; > >   to > >         local $/ = undef; >         print <$fd>; >         ... hope that nobody depends on standard value of $/ >         ... until the end of scope, and whoever changes this >         ... sub is careful enough in the future > >   which I think is worse.  Introducing an extra scope explicitly >   delimit the part you want to use localized $/ like this > >         { local $/; print <$fd>; } > >   might have been more palatable.  Am I guessing the reason of >   your change wrong? Actually only one change was from '$/ = undef; print <$fd>; $/ = "\n"', namely at the end of git_blob_plain. I think nobody will print anything to the end of the sub. Two other chunks actually _introduced_ 'local $/ = undef', both in the fairly short 'if' body. First to read $home_text (there change is not that important, as $home_text should be fairly short), next more important in git_snapshot. IMVHO 'local $/ = undef' in all commands that get the rest of the output from filehandle (and outputting nothing later), like all 'plain' outputs should be the idiom to use. -- Jakub Narebski Warsaw, Poland ShadeHawk on #git ``` ## Jakub Narebski, 2006-08-27 20:21 Subject: Re: [PATCH 0/7] gitweb: Cleanups, fixes and small improvements Message-ID: URL: https://gitlist.dev/e/ecsur8%24gqh%241%40sea.gmane.org In-Reply-To: <7vd5an1afz.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > * gitweb: Remove workaround for git-diff bug fixed in f82cd3c > . gitweb: Fix typo in git_patchset_body > >   I think you had separate patches for these; applied. I think the second one, the one fixing typo ('@$fd>' instead of '= <$fd>') was not applied. I don't know how this typo got into gitweb, as I have tested gitweb after the 19-patch series, and this error looks was introduced in 'next' branch, 157e43b4b0dd5a08eb7a6838192ac58bca62fa5b commit gitweb: Streamify patch output in git_commitdiff This error is critical! -- Jakub Narebski Warsaw, Poland ShadeHawk on #git ```