{"thread":{"id":"43394","subject":"Re: [PATCH] gitweb: Show '...' links in \"summary\" view only if there are more items","startedAt":"2006-12-18T22:43:27Z","lastAt":"2006-12-19T18:05:47Z","messageCount":11,"participants":["Jakub Narebski","Robert Fitzsimons","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"298653","messageId":"20061218224327.GG16029@localhost","threadId":"43394","inReplyTo":null,"subject":"[PATCH] Small optimizations to gitweb","fromName":"Robert Fitzsimons","fromEmail":"robfitz@273k.net","sentAt":"2006-12-18T22:43:27Z","receivedAt":"2006-12-18T22:43:27Z","isPatch":true,"sender":{"key":"robfitz@273k.net","avatar":null},"body":"Limit some of the git_cmd's so they only return the number of lines\nthat will be processed.  Don't recompute head hash or have_snapshot\nvalues.\n\nSigned-off-by: Robert Fitzsimons <robfitz@273k.net>\n---\n gitweb/gitweb.perl |    9 ++++++---\n 1 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 5ea3fda..1990f15 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1141,6 +1141,7 @@ sub git_get_last_activity {\n \topen($fd, \"-|\", git_cmd(), 'for-each-ref',\n \t     '--format=%(refname) %(committer)',\n \t     '--sort=-committerdate',\n+\t     '--count=1',\n \t     'refs/heads') or return;\n \tmy $most_recent = <$fd>;\n \tclose $fd or return;\n@@ -2559,6 +2560,8 @@ sub git_shortlog_body {\n \t# uses global variable $project\n \tmy ($revlist, $from, $to, $refs, $extra) = @_;\n \n+\tmy $have_snapshot = gitweb_have_snapshot();\n+\n \t$from = 0 unless defined $from;\n \t$to = $#{$revlist} if (!defined $to || $#{$revlist} < $to);\n \n@@ -2586,7 +2589,7 @@ sub git_shortlog_body {\n \t\t      $cgi->a({-href => href(action=>\"commit\", hash=>$commit)}, \"commit\") . \" | \" .\n \t\t      $cgi->a({-href => href(action=>\"commitdiff\", hash=>$commit)}, \"commitdiff\") . \" | \" .\n \t\t      $cgi->a({-href => href(action=>\"tree\", hash=>$commit, hash_base=>$commit)}, \"tree\");\n-\t\tif (gitweb_have_snapshot()) {\n+\t\tif ($have_snapshot) {\n \t\t\tprint \" | \" . $cgi->a({-href => href(action=>\"snapshot\", hash=>$commit)}, \"snapshot\");\n \t\t}\n \t\tprint \"</td>\\n\" .\n@@ -2876,8 +2879,8 @@ sub git_summary {\n \t\t}\n \t}\n \n-\topen my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=17\",\n-\t\tgit_get_head_hash($project), \"--\"\n+\topen my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=16\",\n+\t\t$head, \"--\"\n \t\tor die_error(undef, \"Open git-rev-list failed\");\n \tmy @revlist = map { chomp; $_ } <$fd>;\n \tclose $fd;\n-- \n1.4.4.2.gee60-dirty\n"},{"id":"295353","messageId":"em77cg$obn$1@sea.gmane.org","threadId":"43394","inReplyTo":"20061218224327.GG16029@localhost","subject":"Re: [PATCH] Small optimizations to gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-18T23:17:03Z","receivedAt":"2006-12-18T23:17:03Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Robert Fitzsimons wrote:\n\n> Limit some of the git_cmd's so they only return the number of lines\n> that will be processed. \n[...]\n> @@ -2876,8 +2879,8 @@ sub git_summary {\n>                 }\n>         }\n>  \n> -       open my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=17\",\n> -               git_get_head_hash($project), \"--\"\n> +       open my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=16\",\n> +               $head, \"--\"\n>                 or die_error(undef, \"Open git-rev-list failed\");\n>         my @revlist = map { chomp; $_ } <$fd>;\n>         close $fd;\n\nActually, that is needed to implement checking if we have more than\nthe number of commits to show to add '...' at the end only if there\nare some commits which we don't show. The same for heads and tags.\n(On my short TODO list, but feel free to do it yourself, if you want).\n\nSo ack without the last chunk.\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n\n"},{"id":"297887","messageId":"7vbqm0vkd6.fsf@assigned-by-dhcp.cox.net","threadId":"43394","inReplyTo":"em77cg$obn$1@sea.gmane.org","subject":"Re: [PATCH] Small optimizations to gitweb","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-18T23:45:41Z","receivedAt":"2006-12-18T23:45:41Z","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> Actually, that is needed to implement checking if we have more than\n> the number of commits to show to add '...' at the end only if there\n> are some commits which we don't show.\n\nThe counting code in git_*_body is seriously unusual to tempt\nanybody who reviews the code to reduce that 17 to 16.\n\nThe caller says:\n\n\tgit_shortlog_body(\\@revlist, 0, 15, $refs,\n\t                  $cgi->a({-href => href(action=>\"shortlog\")}, \"...\"));\n\nIf it counts up, especially if it counts from zero, the loop\nwould usually say:\n\n\tfor (i = bottom; i < end; i++)\n\nand anybody who reads that caller would expect it to show 15\nlines of output.\n\nBut the actual code does this instead:\n\n    sub git_shortlog_body {\n            # uses global variable $project\n            my ($revlist, $from, $to, $refs, $extra) = @_;\n\n            $from = 0 unless defined $from;\n            $to = $#{$revlist} if (!defined $to || $#{$revlist} < $to);\n            ...\n            for (my $i = $from; $i <= $to; $i++) {\n                    ... draw each item ...\n            }\n            if (defined $extra) {\n                    print \"<tr>\\n\" .\n                          \"<td colspan=\\\"4\\\">$extra</td>\\n\" .\n                          \"</tr>\\n\";\n            }\n    }\n\nBy the way, I wonder how that $extra is omitted when $revlist is\nlonger than $to; it should be a trivial fix but it seems to me\nthat it is always spitted out with the current code.\n\n\n"},{"id":"297554","messageId":"200612190159.24173.jnareb@gmail.com","threadId":"43394","inReplyTo":"7vbqm0vkd6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Small optimizations to gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-19T00:59:23Z","receivedAt":"2006-12-19T00:59:23Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n>> Actually, that is needed to implement checking if we have more than\n>> the number of commits to show to add '...' at the end only if there\n>> are some commits which we don't show.\n> \n> The counting code in git_*_body is seriously unusual to tempt\n> anybody who reviews the code to reduce that 17 to 16.\n> \n> The caller says:\n> \n> \tgit_shortlog_body(\\@revlist, 0, 15, $refs,\n> \t                  $cgi->a({-href => href(action=>\"shortlog\")}, \"...\"));\n> \n> If it counts up, especially if it counts from zero, the loop\n> would usually say:\n> \n> \tfor (i = bottom; i < end; i++)\n> \n> and anybody who reads that caller would expect it to show 15\n> lines of output.\n> \n> But the actual code does this instead:\n> \n>     sub git_shortlog_body {\n>             # uses global variable $project\n>             my ($revlist, $from, $to, $refs, $extra) = @_;\n> \n>             $from = 0 unless defined $from;\n>             $to = $#{$revlist} if (!defined $to || $#{$revlist} < $to);\n>             ...\n>             for (my $i = $from; $i <= $to; $i++) {\n>                     ... draw each item ...\n>             }\n\nWell, this should be then corrected perhaps to\n\n            my ($revlist, $begin, $end, $refs, $extra) = @_;\n\n            $begin = 0 unless defined $from;\n            $end = scalar(@$revlist) if (!defined $end || @$revlist <= $end);\n            ...\n            for (my $i = $begin; $i < $end; $i++) {\n                    ... draw each item ...\n            }\n\nI thought that $from..$to ($from <= i <= $to) is more natural\nand easier to understand than $begin..$end ($begin <= i < $end)...\nguess I guessed wrong.\n\n>             if (defined $extra) {\n>                     print \"<tr>\\n\" .\n>                           \"<td colspan=\\\"4\\\">$extra</td>\\n\" .\n>                           \"</tr>\\n\";\n>             }\n>     }\n> \n> By the way, I wonder how that $extra is omitted when $revlist is\n> longer than $to; it should be a trivial fix but it seems to me\n> that it is always spitted out with the current code.\n\nWe should check if we want to omit $extra, either in caller or\nin callee, the *_body subroutine itself.\n\n-- \nJakub Narebski\n"},{"id":"295301","messageId":"7vtzzstozi.fsf@assigned-by-dhcp.cox.net","threadId":"43394","inReplyTo":"200612190159.24173.jnareb@gmail.com","subject":"Re: [PATCH] Small optimizations to gitweb","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-19T05:48:49Z","receivedAt":"2006-12-19T05:48:49Z","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> Junio C Hamano wrote:\n>> ...\n>> The counting code in git_*_body is seriously unusual to tempt\n>> anybody who reviews the code to reduce that 17 to 16.\n>\n> Well, this should be then corrected perhaps to\n\nWell, I did not say it was _wrong_, just unusual, so there is\nnothing to fix.\n\n>> By the way, I wonder how that $extra is omitted when $revlist is\n>> longer than $to; it should be a trivial fix but it seems to me\n>> that it is always spitted out with the current code.\n>\n> We should check if we want to omit $extra, either in caller or\n> in callee, the *_body subroutine itself.\n\nCheck?\n"},{"id":"296885","messageId":"200612191214.58474.jnareb@gmail.com","threadId":"43394","inReplyTo":"200612190159.24173.jnareb@gmail.com","subject":"[PATCH] gitweb: Show '...' links in \"summary\" view only if there are more items","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-19T11:14:57Z","receivedAt":"2006-12-19T11:14:57Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Show \"...\" links in \"summary\" view to shortlog, heads (if there are\nany), and tags (if there are any) only if there are more items to show\nthan shown already.\n\nThis means that \"...\" link is shown below shortened shortlog if there\nare more than 16 commits, \"...\" link below shortened heads list if\nthere are more than 16 heads refs (16 branches), \"...\" link below\nshortened tags list if there are more than 16 tags.\n\nAdded some comments.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\n\nJakub Narebski wrote:\n> Junio C Hamano wrote:\n>> Jakub Narebski <jnareb@gmail.com> writes:\n>> \n>>> Actually, that is needed to implement checking if we have more than\n>>> the number of commits to show to add '...' at the end only if there\n>>> are some commits which we don't show.\n[...]\n>> By the way, I wonder how that $extra is omitted when $revlist is\n>> longer than $to; it should be a trivial fix but it seems to me\n>> that it is always spitted out with the current code.\n> \n> We should check if we want to omit $extra, either in caller or\n> in callee, the *_body subroutine itself.\n\nAnd now it is done.\n\nSlightly tested: on my clone (copy) of git repository, which more than\n16 commits, more than 16 heads (most temporary, and no longer worked on,\nfew tracking branches) and more than 16 heads show all \"...\" as it should.\nTest of freshly created repository shown no \"...\" for commits (only one\ncommit), no \"...\" for heads (only one default head 'master'), and no\ntags list (no tags at all).\n\nBy the way, I have _NOT_ applied Robert Fitzsimons patch, but they\n(this patch and Robert patch) should be not in conflict if we remove\nlast chunk of Robert's patch (this changing --count=17 to --count=15\nin git_summary).\n\n gitweb/gitweb.perl |    9 +++++++--\n 1 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4059894..73877f2 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2915,8 +2915,9 @@ sub git_summary {\n \tmy $owner = git_get_project_owner($project);\n \n \tmy $refs = git_get_references();\n-\tmy @taglist  = git_get_tags_list(15);\n-\tmy @headlist = git_get_heads_list(15);\n+\t# we need to request one more than 16 (0..15) to check if those 16 are all\n+\tmy @taglist  = git_get_tags_list(17);\n+\tmy @headlist = git_get_heads_list(17);\n \tmy @forklist;\n \tmy ($check_forks) = gitweb_check_feature('forks');\n \n@@ -2952,6 +2953,7 @@ sub git_summary {\n \t\t}\n \t}\n \n+\t# we need to request one more than 16 (0..15) to check if those 16 are all\n \topen my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=17\",\n \t\tgit_get_head_hash($project), \"--\"\n \t\tor die_error(undef, \"Open git-rev-list failed\");\n@@ -2959,17 +2961,20 @@ sub git_summary {\n \tclose $fd;\n \tgit_print_header_div('shortlog');\n \tgit_shortlog_body(\\@revlist, 0, 15, $refs,\n+\t                  $#revlist <=  15 ? undef :\n \t                  $cgi->a({-href => href(action=>\"shortlog\")}, \"...\"));\n \n \tif (@taglist) {\n \t\tgit_print_header_div('tags');\n \t\tgit_tags_body(\\@taglist, 0, 15,\n+\t\t              $#taglist <=  15 ? undef :\n \t\t              $cgi->a({-href => href(action=>\"tags\")}, \"...\"));\n \t}\n \n \tif (@headlist) {\n \t\tgit_print_header_div('heads');\n \t\tgit_heads_body(\\@headlist, $head, 0, 15,\n+\t\t               $#headlist <= 15 ? undef :\n \t\t               $cgi->a({-href => href(action=>\"heads\")}, \"...\"));\n \t}\n \n-- \n"},{"id":"294751","messageId":"20061219120854.GA16429@localhost","threadId":"43394","inReplyTo":"200612191214.58474.jnareb@gmail.com","subject":"[PATCH] gitweb: Show '...' links in \"summary\" view only if there are more items","fromName":"Robert Fitzsimons","fromEmail":"robfitz@273k.net","sentAt":"2006-12-19T12:08:54Z","receivedAt":"2006-12-19T12:08:54Z","isPatch":true,"sender":{"key":"robfitz@273k.net","avatar":null},"body":"Show \"...\" links in \"summary\" view to shortlog, heads (if there are\nany), and tags (if there are any) only if there are more items to show\nthan shown already.\n\nThis means that \"...\" link is shown below shortened shortlog if there\nare more than 16 commits, \"...\" link below shortened heads list if\nthere are more than 16 heads refs (16 branches), \"...\" link below\nshortened tags list if there are more than 16 tags.\n\nModified patch from Jakub to to apply cleanly to master, also preform\nthe same \"...\" link logic to the forks list.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\nSigned-off-by: Robert Fitzsimons <robfitz@273k.net>\n---\n\n> By the way, I have _NOT_ applied Robert Fitzsimons patch, but they\n> (this patch and Robert patch) should be not in conflict if we remove\n> last chunk of Robert's patch (this changing --count=17 to --count=15\n> in git_summary).\n\nJust removing the last chunk isn't correct, there are two slightly\ndifferent changes in that chuck.  The reduction in the max-count value\nand a removal of a call to git_get_head_hash.\n\nHere is an updated version of the patch which should apply against\nmaster.\n\nRobert\n\n\n\n gitweb/gitweb.perl |   12 +++++++++---\n 1 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3bee34c..8d409c7 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2918,8 +2918,9 @@ sub git_summary {\n \tmy $owner = git_get_project_owner($project);\n \n \tmy $refs = git_get_references();\n-\tmy @taglist  = git_get_tags_list(15);\n-\tmy @headlist = git_get_heads_list(15);\n+\t# we need to request one more than 16 (0..15) to check if those 16 are all\n+\tmy @taglist  = git_get_tags_list(16);\n+\tmy @headlist = git_get_heads_list(16);\n \tmy @forklist;\n \tmy ($check_forks) = gitweb_check_feature('forks');\n \n@@ -2955,30 +2956,35 @@ sub git_summary {\n \t\t}\n \t}\n \n-\topen my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=16\",\n+\t# we need to request one more than 16 (0..15) to check if those 16 are all\n+\topen my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=17\",\n \t\t$head, \"--\"\n \t\tor die_error(undef, \"Open git-rev-list failed\");\n \tmy @revlist = map { chomp; $_ } <$fd>;\n \tclose $fd;\n \tgit_print_header_div('shortlog');\n \tgit_shortlog_body(\\@revlist, 0, 15, $refs,\n+\t                  $#revlist <=  15 ? undef :\n \t                  $cgi->a({-href => href(action=>\"shortlog\")}, \"...\"));\n \n \tif (@taglist) {\n \t\tgit_print_header_div('tags');\n \t\tgit_tags_body(\\@taglist, 0, 15,\n+\t\t              $#taglist <=  15 ? undef :\n \t\t              $cgi->a({-href => href(action=>\"tags\")}, \"...\"));\n \t}\n \n \tif (@headlist) {\n \t\tgit_print_header_div('heads');\n \t\tgit_heads_body(\\@headlist, $head, 0, 15,\n+\t\t               $#headlist <= 15 ? undef :\n \t\t               $cgi->a({-href => href(action=>\"heads\")}, \"...\"));\n \t}\n \n \tif (@forklist) {\n \t\tgit_print_header_div('forks');\n \t\tgit_project_list_body(\\@forklist, undef, 0, 15,\n+\t\t                      $#forklist <= 15 ? undef :\n \t\t                      $cgi->a({-href => href(action=>\"forks\")}, \"...\"),\n \t\t\t\t      'noheader');\n \t}\n-- \n"},{"id":"296772","messageId":"200612191328.08928.jnareb@gmail.com","threadId":"43394","inReplyTo":"20061219120854.GA16429@localhost","subject":"Re: [PATCH] gitweb: Show '...' links in \"summary\" view only if there are more items","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-19T12:28:08Z","receivedAt":"2006-12-19T12:28:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Robert Fitzsimons wrote:\n> Show \"...\" links in \"summary\" view to shortlog, heads (if there are\n> any), and tags (if there are any) only if there are more items to show\n> than shown already.\n> \n> This means that \"...\" link is shown below shortened shortlog if there\n> are more than 16 commits, \"...\" link below shortened heads list if\n> there are more than 16 heads refs (16 branches), \"...\" link below\n> shortened tags list if there are more than 16 tags.\n> \n> Modified patch from Jakub to to apply cleanly to master, also preform\n> the same \"...\" link logic to the forks list.\n\nJunio usually puts such comments in brackets (I don't know if it is\nalways used, i.e. if it is some 'convention'), e.g.:\n\n  Also perform the same \"...\" link logic to the forks list.\n\n  [rf: Modified patch from Jakub to to apply cleanly to master]\n\nor something like that. Just a nitpick.\n\n\nBy the way, it looks like git_get_projects_list($project) used to get\nlist of forks does not have any count limit option.\n\n[...]\n> ---\n>\n> > By the way, I have _NOT_ applied Robert Fitzsimons patch, but they\n> > (this patch and Robert patch) should be not in conflict if we\n> > remove last chunk of Robert's patch (this changing --count=17 to\n> > --count=15 in git_summary).\n>\n> Just removing the last chunk isn't correct, there are two slightly\n> different changes in that chuck.  The reduction in the max-count\n> value and a removal of a call to git_get_head_hash.\n\nThe last chunk I meant to be removed was:\n\n> @@ -2876,8 +2879,8 @@ sub git_summary {\n>                 }\n>         }\n>  \n> -       open my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=17\",\n> -               git_get_head_hash($project), \"--\"\n> +       open my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=16\",\n> +               $head, \"--\"\n>                 or die_error(undef, \"Open git-rev-list failed\");\n>         my @revlist = map { chomp; $_ } <$fd>;\n>         close $fd;\n\nand if we remove that chunk, then your earlier patch would not\ntouch git_summary at all, so mine would cleanly apply (I think).\n\n[...]\n> -\tmy @taglist  = git_get_tags_list(15);\n> -\tmy @headlist = git_get_heads_list(15);\n> +\t# we need to request one more than 16 (0..15) to check if those 16 are all\n> +\tmy @taglist  = git_get_tags_list(16);\n> +\tmy @headlist = git_get_heads_list(16);\n\nIt needs to be 17, not 16, otherwise we never would get \"...\". By default\nwe show _16_ items, from 0 to 15 inclusive, so we must get _17_ items\nto check if there are more than 16.\n\n>  \tmy @forklist;\n>  \tmy ($check_forks) = gitweb_check_feature('forks');\n>  \n> @@ -2955,30 +2956,35 @@ sub git_summary {\n>  \t\t}\n>  \t}\n>  \n> -\topen my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=16\",\n> +\t# we need to request one more than 16 (0..15) to check if those 16 are all\n> +\topen my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=17\",\n\nHere you have 17.\n\n\n>  \tif (@forklist) {\n>  \t\tgit_print_header_div('forks');\n>  \t\tgit_project_list_body(\\@forklist, undef, 0, 15,\n> +\t\t                      $#forklist <= 15 ? undef :\n>  \t\t                      $cgi->a({-href => href(action=>\"forks\")}, \"...\"),\n>  \t\t\t\t      'noheader');\n>  \t}\n\nNice catch. I forgot about this one.\n\n-- \nJakub Narebski\n"},{"id":"297889","messageId":"20061219124133.GB16429@localhost","threadId":"43394","inReplyTo":"200612191328.08928.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Show '...' links in \"summary\" view only if there are more items","fromName":"Robert Fitzsimons","fromEmail":"robfitz@273k.net","sentAt":"2006-12-19T12:41:33Z","receivedAt":"2006-12-19T12:41:33Z","isPatch":true,"sender":{"key":"robfitz@273k.net","avatar":null},"body":"> Junio usually puts such comments in brackets (I don't know if it is\n> always used, i.e. if it is some 'convention'), e.g.:\n> \n>   Also perform the same \"...\" link logic to the forks list.\n> \n>   [rf: Modified patch from Jakub to to apply cleanly to master]\n> \n> or something like that. Just a nitpick.\n\nNo problem, I tried to find the approreati convention.\n\n> > -\tmy @taglist  = git_get_tags_list(15);\n> > -\tmy @headlist = git_get_heads_list(15);\n> > +\t# we need to request one more than 16 (0..15) to check if those 16 are all\n> > +\tmy @taglist  = git_get_tags_list(16);\n> > +\tmy @headlist = git_get_heads_list(16);\n> \n> It needs to be 17, not 16, otherwise we never would get \"...\". By default\n> we show _16_ items, from 0 to 15 inclusive, so we must get _17_ items\n> to check if there are more than 16.\n\nThat was a copy error on my part.  Though looking at the code\ngit_get_tags_list and git_get_heads_list already adds one to the limit\nvalue, so if you pass in 17 they will return 18 items.\n\nRobert\n"},{"id":"294605","messageId":"200612191342.46220.jnareb@gmail.com","threadId":"43394","inReplyTo":"200612191328.08928.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Show '...' links in \"summary\" view only if there are more items","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-19T12:42:44Z","receivedAt":"2006-12-19T12:42:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"\n>> @@ -2876,8 +2879,8 @@ sub git_summary {\n>>                 }\n>>         }\n>>  \n>> -       open my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=17\",\n>> -               git_get_head_hash($project), \"--\"\n>> +       open my $fd, \"-|\", git_cmd(), \"rev-list\", \"--max-count=16\",\n>> +               $head, \"--\"\n>>                 or die_error(undef, \"Open git-rev-list failed\");\n>>         my @revlist = map { chomp; $_ } <$fd>;\n>>         close $fd;\n> \n> and if we remove that chunk, then your earlier patch would not\n> touch git_summary at all, so mine would cleanly apply (I think).\n\nSorry, I haven't noticed git_get_head_hash($project) -> $head\nSorry for the noise.\n-- \nJakub Narebski\n"},{"id":"298184","messageId":"7vr6uvpxqc.fsf@assigned-by-dhcp.cox.net","threadId":"43394","inReplyTo":"200612191328.08928.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Show '...' links in \"summary\" view only if there are more items","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-19T18:05:47Z","receivedAt":"2006-12-19T18:05:47Z","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> Robert Fitzsimons wrote:\n>> Show \"...\" links in \"summary\" view to shortlog, heads (if there are\n>> any), and tags (if there are any) only if there are more items to show\n>> than shown already.\n>> \n>> This means that \"...\" link is shown below shortened shortlog if there\n>> are more than 16 commits, \"...\" link below shortened heads list if\n>> there are more than 16 heads refs (16 branches), \"...\" link below\n>> shortened tags list if there are more than 16 tags.\n>> \n>> Modified patch from Jakub to to apply cleanly to master, also preform\n>> the same \"...\" link logic to the forks list.\n>\n> Junio usually puts such comments in brackets (I don't know if it is\n> always used, i.e. if it is some 'convention'), e.g.:\n>\n>   Also perform the same \"...\" link logic to the forks list.\n>\n>   [rf: Modified patch from Jakub to to apply cleanly to master]\n\nI would actually discourage this on messages that are still on\nthe list (i.e. not in commits).  I do [jc:] comment when I take\nthe proposed commit log message from the incoming e-mail\nverbatim but I made modification to the patch text, because\notherwise the original submitter cannot tell from the log\nmessage if that is meant to be identical to what was submitted.\n\nI've only took a cursory look of the actual patch text, but what\nRobert sent looked good to me.  Thanks, both.\n"}]}