{"thread":{"id":"5349","subject":"[PATCH] gitweb: Paginate history output","startedAt":"2006-08-21T22:53:10Z","lastAt":"2006-08-22T08:08:47Z","messageCount":3,"participants":["Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"25718","messageId":"200608220053.10741.jnareb@gmail.com","threadId":"5349","inReplyTo":null,"subject":"[PATCH] gitweb: Paginate history output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-08-21T22:53:10Z","receivedAt":"2006-08-21T22:53:10Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"git_history output is now divided into pages, like git_shortlog,\ngit_tags and git_heads output. As whole git-rev-list output is now\nread into array before writing anything, it allows for better\nsignaling of errors.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nBased on 'next' branch, but should apply to 'master as well.\n\n gitweb/gitweb.perl |   59 ++++++++++++++++++++++++++++++++++++++++++++++------\n 1 files changed, 52 insertions(+), 7 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 31a1824..90157d5 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1544,12 +1544,15 @@ sub git_shortlog_body {\n \n sub git_history_body {\n \t# Warning: assumes constant type (blob or tree) during history\n-\tmy ($fd, $refs, $hash_base, $ftype, $extra) = @_;\n+\tmy ($revlist, $from, $to, $refs, $hash_base, $ftype, $extra) = @_;\n+\n+\t$from = 0 unless defined $from;\n+\t$to = $#{$revlist} if (!defined $to || $#{$revlist} < $to);\n \n \tprint \"<table class=\\\"history\\\" cellspacing=\\\"0\\\">\\n\";\n \tmy $alternate = 0;\n-\twhile (my $line = <$fd>) {\n-\t\tif ($line !~ m/^([0-9a-fA-F]{40})/) {\n+\tfor (my $i = $from; $i <= $to; $i++) {\n+\t\tif ($revlist->[$i] !~ m/^([0-9a-fA-F]{40})/) {\n \t\t\tnext;\n \t\t}\n \n@@ -1559,6 +1562,7 @@ sub git_history_body {\n \t\t\tnext;\n \t\t}\n \n+\t\t#my $ref = defined $refs ? format_ref_marker($refs, $commit) : '';\n \t\tmy $ref = format_ref_marker($refs, $commit);\n \n \t\tif ($alternate) {\n@@ -2651,21 +2655,62 @@ sub git_history {\n \tif (!%co) {\n \t\tdie_error(undef, \"Unknown commit object\");\n \t}\n+\n \tmy $refs = git_get_references();\n-\tgit_header_html();\n-\tgit_print_page_nav('','', $hash_base,$co{'tree'},$hash_base);\n-\tgit_print_header_div('commit', esc_html($co{'title'}), $hash_base);\n+\tmy $limit = sprintf(\"--max-count=%i\", (100 * ($page+1)));\n+\n \tif (!defined $hash && defined $file_name) {\n \t\t$hash = git_get_hash_by_path($hash_base, $file_name);\n \t}\n \tif (defined $hash) {\n \t\t$ftype = git_get_type($hash);\n \t}\n+\n+\topen my $fd, \"-|\",\n+\t\t$GIT, \"rev-list\", $limit, \"--full-history\", $hash_base, \"--\", $file_name\n+\t\t\tor die_error(undef, \"Open git-rev-list-failed\");\n+\tmy @revlist = map { chomp; $_ } <$fd>;\n+\tclose $fd;\n+\n+\tmy $paging_nav = '';\n+\tif ($page > 0) {\n+\t\t$paging_nav .=\n+\t\t\t$cgi->a({-href => href(action=>\"history\", hash=>$hash, hash_base=>$hash_base,\n+\t\t\t                       file_name=>$file_name)},\n+\t\t\t        \"first\");\n+\t\t$paging_nav .= \" &sdot; \" .\n+\t\t\t$cgi->a({-href => href(action=>\"history\", hash=>$hash, hash_base=>$hash_base,\n+\t\t\t                       file_name=>$file_name, page=>$page-1),\n+\t\t\t         -accesskey => \"p\", -title => \"Alt-p\"}, \"prev\");\n+\t} else {\n+\t\t$paging_nav .= \"first\";\n+\t\t$paging_nav .= \" &sdot; prev\";\n+\t}\n+\tif ($#revlist >= (100 * ($page+1)-1)) {\n+\t\t$paging_nav .= \" &sdot; \" .\n+\t\t\t$cgi->a({-href => href(action=>\"history\", hash=>$hash, hash_base=>$hash_base,\n+\t\t\t                       file_name=>$file_name, page=>$page+1),\n+\t\t\t         -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n+\t} else {\n+\t\t$paging_nav .= \" &sdot; next\";\n+\t}\n+\tmy $next_link = '';\n+\tif ($#revlist >= (100 * ($page+1)-1)) {\n+\t\t$next_link =\n+\t\t\t$cgi->a({-href => href(action=>\"history\", hash=>$hash, hash_base=>$hash_base,\n+\t\t\t                       file_name=>$file_name, page=>$page+1),\n+\t\t\t         -title => \"Alt-n\"}, \"next\");\n+\t}\n+\n+\tgit_header_html();\n+\tgit_print_page_nav('history','', $hash_base,$co{'tree'},$hash_base, $paging_nav);\n+\tgit_print_header_div('commit', esc_html($co{'title'}), $hash_base);\n \tgit_print_page_path($file_name, $ftype, $hash_base);\n \n \topen my $fd, \"-|\",\n \t\t$GIT, \"rev-list\", \"--full-history\", $hash_base, \"--\", $file_name;\n-\tgit_history_body($fd, $refs, $hash_base, $ftype);\n+\tgit_history_body(\\@revlist, ($page * 100), $#revlist,\n+\t                 $refs, $hash_base, $ftype, $next_link);\n \n \tclose $fd;\n \tgit_footer_html();\n-- \n1.4.1.1\n"},{"id":"25723","messageId":"7vveols3c7.fsf@assigned-by-dhcp.cox.net","threadId":"5349","inReplyTo":"200608220053.10741.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Paginate history output","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-22T06:15:52Z","receivedAt":"2006-08-22T06:15:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> @@ -1559,6 +1562,7 @@ sub git_history_body {\n>  \t\t\tnext;\n>  \t\t}\n>  \n> +\t\t#my $ref = defined $refs ? format_ref_marker($refs, $commit) : '';\n>  \t\tmy $ref = format_ref_marker($refs, $commit);\n>  \n>  \t\tif ($alternate) {\n\nDo you want to change this or not?  Make up your mind.\n\n> +\t\t$paging_nav .= \" &sdot; \" .\n> +\t\t\t$cgi->a({-href => href(action=>\"history\", hash=>$hash, hash_base=>$hash_base,\n> +\t\t\t                       file_name=>$file_name, page=>$page-1),\n> +\t\t\t         -accesskey => \"p\", -title => \"Alt-p\"}, \"prev\");\n\nThis is something you inherited from the original and not your\nfault, but I am not sure if these -title entries are of good\ntaste (we have corresponding \"Alt-n\" for \"next\").  Something\nmore descriptive like \"<Alt-p> for 100 newer changes\", perhaps?\n\nAlso, \"first\", \"prev\" and \"next\" have always confused me.  Maybe\n\"latest\", \"older\" and \"newer\" are better labels for them?\n\nIs 100 a good default?  It feels a bit unbalanced compared to\nthe height of the default summary page with descriptions, 16\nshortlog entries, 16 tags and 12 heads.\n"},{"id":"25729","messageId":"ecee2g$b0l$1@sea.gmane.org","threadId":"5349","inReplyTo":"7vveols3c7.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: Paginate history output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-08-22T08:08:47Z","receivedAt":"2006-08-22T08:08:47Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n>> @@ -1559,6 +1562,7 @@ sub git_history_body {\n>>                      next;\n>>              }\n>>  \n>> +            #my $ref = defined $refs ? format_ref_marker($refs, $commit) : '';\n>>              my $ref = format_ref_marker($refs, $commit);\n>>  \n>>              if ($alternate) {\n> \n> Do you want to change this or not?  Make up your mind.\n\nI'm not sure. This is an internal function, so $refs should \nbe defined. I'm not sure checking here is worth it.\n\nThis whole hunk could be removed.\n\n>> +            $paging_nav .= \" &sdot; \" .\n>> +                    $cgi->a({-href => href(action=>\"history\", hash=>$hash, hash_base=>$hash_base,\n>> +                                           file_name=>$file_name, page=>$page-1),\n>> +                             -accesskey => \"p\", -title => \"Alt-p\"}, \"prev\");\n> \n> This is something you inherited from the original and not your\n> fault, but I am not sure if these -title entries are of good\n> taste (we have corresponding \"Alt-n\" for \"next\").  Something\n> more descriptive like \"<Alt-p> for 100 newer changes\", perhaps?\n\nThat would be better.\n\n> Also, \"first\", \"prev\" and \"next\" have always confused me.  Maybe\n> \"latest\", \"older\" and \"newer\" are better labels for them?\n\n\"first\" is only for pagination of \"history\" action, \"shortlog\",\n\"heads\" and \"tags\" have \"HEAD\" there instead.\n\n> Is 100 a good default?  It feels a bit unbalanced compared to\n> the height of the default summary page with descriptions, 16\n> shortlog entries, 16 tags and 12 heads.\n\nIt's 16 heads (if you have that many), from what I remember \nfrom source.\n\nThe purpose of summary page is to have everything at glance (it means\nthat it should be contained in one, two pages. The pagination of\noutput is to not send too much at once.\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"}]}