{"thread":{"id":"6036","subject":"[PATCH 1/3] gitweb: Get rid of git_print_simplified_log","startedAt":"2006-10-24T11:49:54Z","lastAt":"2006-10-25T12:23:21Z","messageCount":8,"participants":["Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"29854","messageId":"200610241349.54685.jnareb@gmail.com","threadId":"6036","inReplyTo":null,"subject":"[PATCH 0/3] gitweb: Improvements to commitdiff oview","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-24T11:49:54Z","receivedAt":"2006-10-24T11:49:54Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This series (patch 2 to be more exact) fixes error seen for example in\n  a=commitdiff;h=161332a521fe10c41979bcd493d95e2ac562b7f\nin git.git repository.\n\nShortlog:\n [PATCH 1/3] gitweb: Get rid of git_print_simplified_log\n [PATCH 2/3] gitweb: Filter out commit ID from @difftree in git_commit and git_commitdiff\n [PATCH 3/3] gitweb: Print commit message without title in commitdiff only if there is any\n\nDiffstat:\n gitweb/gitweb.perl |   17 +++++++----------\n 1 files changed, 7 insertions(+), 10 deletions(-)\n\n-- \nJakub Narebski\nPoland\n"},{"id":"29853","messageId":"200610241352.46422.jnareb@gmail.com","threadId":"6036","inReplyTo":"200610241349.54685.jnareb@gmail.com","subject":"[PATCH 1/3] gitweb: Get rid of git_print_simplified_log","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-24T11:52:46Z","receivedAt":"2006-10-24T11:52:46Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Replace calls to git_print_simplified_log with its expansion,\ni.e. with calling git_print_log with appropriate options.\n\nRemove no longer used git_print_simplified_log subroutine.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\n gitweb/gitweb.perl |   13 ++-----------\n 1 files changed, 2 insertions(+), 11 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0a7d65c..8ac60be 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1697,15 +1697,6 @@ sub git_print_log ($;%) {\n \t}\n }\n \n-sub git_print_simplified_log {\n-\tmy $log = shift;\n-\tmy $remove_title = shift;\n-\n-\tgit_print_log($log,\n-\t\t-final_empty_line=> 1,\n-\t\t-remove_title => $remove_title);\n-}\n-\n # print tree entry (row of git_tree), but without encompassing <tr> element\n sub git_print_tree_entry {\n \tmy ($t, $basedir, $hash_base, $have_blame) = @_;\n@@ -2995,7 +2986,7 @@ sub git_log {\n \t\t      \"</div>\\n\";\n \n \t\tprint \"<div class=\\\"log_body\\\">\\n\";\n-\t\tgit_print_simplified_log($co{'comment'});\n+\t\tgit_print_log($co{'comment'}, -final_empty_line=> 1);\n \t\tprint \"</div>\\n\";\n \t}\n \tgit_footer_html();\n@@ -3365,7 +3356,7 @@ sub git_commitdiff {\n \t\tgit_print_authorship(\\%co);\n \t\tprint \"<div class=\\\"page_body\\\">\\n\";\n \t\tprint \"<div class=\\\"log\\\">\\n\";\n-\t\tgit_print_simplified_log($co{'comment'}, 1); # skip title\n+\t\tgit_print_log($co{'comment'}, -final_empty_line=> 1, -remove_title => 1);\n \t\tprint \"</div>\\n\"; # class=\"log\"\n \n \t} elsif ($format eq 'plain') {\n-- \n1.4.2.1\n"},{"id":"29855","messageId":"200610241354.49396.jnareb@gmail.com","threadId":"6036","inReplyTo":"200610241349.54685.jnareb@gmail.com","subject":"[PATCH 2/3] gitweb: Filter out commit ID from @difftree in git_commit and git_commitdiff","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-24T11:54:49Z","receivedAt":"2006-10-24T11:54:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Filter out commit ID output that git-diff-tree adds when called with\nonly one <tree-ish> (not only for --stdin) in git_commit and\ngit_commitdiff.\n\nThis also works with older git versions, which doesn't have\n--no-commit-id option to git-diff-tree.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\n gitweb/gitweb.perl |    7 ++++++-\n 1 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 8ac60be..a88e0a8 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3009,6 +3009,9 @@ sub git_commit {\n \tmy @difftree = map { chomp; $_ } <$fd>;\n \tclose $fd or die_error(undef, \"Reading git-diff-tree failed\");\n \n+\t# filter out commit ID output\n+\t@difftree = grep(!/^[0-9a-fA-F]{40}$/, @difftree);\n+\n \t# non-textual hash id's can be cached\n \tmy $expires;\n \tif ($hash =~ m/^[0-9a-fA-F]{40}$/) {\n@@ -3327,7 +3330,9 @@ sub git_commitdiff {\n \t\twhile (chomp(my $line = <$fd>)) {\n \t\t\t# empty line ends raw part of diff-tree output\n \t\t\tlast unless $line;\n-\t\t\tpush @difftree, $line;\n+\t\t\t# filter out commit ID output\n+\t\t\tpush @difftree, $line\n+\t\t\t\tunless $line =~ m/^[0-9a-fA-F]{40}$/;\n \t\t}\n \n \t} elsif ($format eq 'plain') {\n-- \n1.4.2.1\n"},{"id":"29856","messageId":"200610241355.33560.jnareb@gmail.com","threadId":"6036","inReplyTo":"200610241349.54685.jnareb@gmail.com","subject":"[PATCH 3/3] gitweb: Print commit message without title in commitdiff only if there is any","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-24T11:55:33Z","receivedAt":"2006-10-24T11:55:33Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Print the rest of commit message (title, i.e. first line of commit\nmessage, is printed separately) only if there is any.\n\nIn repository which uses signoffs this shouldn't happen, because\ncommit message should consist of at least title and signoff.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\n gitweb/gitweb.perl |    8 +++++---\n 1 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex a88e0a8..e35ecb4 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3360,9 +3360,11 @@ sub git_commitdiff {\n \t\tgit_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash);\n \t\tgit_print_authorship(\\%co);\n \t\tprint \"<div class=\\\"page_body\\\">\\n\";\n-\t\tprint \"<div class=\\\"log\\\">\\n\";\n-\t\tgit_print_log($co{'comment'}, -final_empty_line=> 1, -remove_title => 1);\n-\t\tprint \"</div>\\n\"; # class=\"log\"\n+\t\tif (@{$co{'comment'}} > 1) {\n+\t\t\tprint \"<div class=\\\"log\\\">\\n\";\n+\t\t\tgit_print_log($co{'comment'}, -final_empty_line=> 1, -remove_title => 1);\n+\t\t\tprint \"</div>\\n\"; # class=\"log\"\n+\t\t}\n \n \t} elsif ($format eq 'plain') {\n \t\tmy $refs = git_get_references(\"tags\");\n-- \n1.4.2.1\n"},{"id":"298275","messageId":"7v1woxf1v9.fsf@assigned-by-dhcp.cox.net","threadId":"6036","inReplyTo":"200610241354.49396.jnareb@gmail.com","subject":"Re: [PATCH 2/3] gitweb: Filter out commit ID from @difftree in git_commit and git_commitdiff","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-25T04:36:26Z","receivedAt":"2006-10-25T04:36:26Z","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> Filter out commit ID output that git-diff-tree adds when called with\n> only one <tree-ish> (not only for --stdin) in git_commit and\n> git_commitdiff.\n\nI initially wondered why this, or other existing such sripping,\nis necessary in the first place.\n\nThe collected result is given to git_difftree_body() and it\nfeeds parse_difftree_raw_line() with it.  Interestingly enough,\nit _is_ prepared to handle the line with the commit object\nname.\n\nHowever, the very initial part of git_difftree_body assumes that\nthe array it gets does not have the commit object name (i.e. it\ncounts the array members and says \"N files changed\").\n\nSo I think your change is probably a good one, but I suspect you\nprobably are better off to make parse_difftree_raw_line() to\nbarf when it gets the commit object name to make sure that all\ncallers strip it at the same time; better yet, perhaps you can\nhave a single function that invokes git-diff-tree -r (with\ndifferent parameters) and returns the result that is already\nonly the difftree body lines?\n"},{"id":"298061","messageId":"200610251003.22526.jnareb@gmail.com","threadId":"6036","inReplyTo":"7v1woxf1v9.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] gitweb: Filter out commit ID from @difftree in git_commit and git_commitdiff","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-25T08:03:22Z","receivedAt":"2006-10-25T08:03:22Z","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>> Filter out commit ID output that git-diff-tree adds when called with\n>> only one <tree-ish> (not only for --stdin) in git_commit and\n>> git_commitdiff.\n> \n> I initially wondered why this, or other existing such sripping,\n> is necessary in the first place.\n> \n> The collected result is given to git_difftree_body() and it\n> feeds parse_difftree_raw_line() with it.  Interestingly enough,\n> it _is_ prepared to handle the line with the commit object\n> name.\n\nPerhaps I was overeager in adding that. The only place it was used\nwas git_tree_blame in the proof-of-concept/RFC patch which added\n\"tree_blame\" view, and even there it was discarded solution: using\ngit-diff-tree can give only commit which changed the file to current\nversion; it cannot find commit which changed tree/directory (latest\ncommit which changed any of its files).\n\nThis can be useful if at any time we need to parse git-diff-tree --stdin\noutput.\n \n> However, the very initial part of git_difftree_body assumes that\n> the array it gets does not have the commit object name (i.e. it\n> counts the array members and says \"N files changed\").\n\nThe same is true for git_patchset_body. Moreover if I remember correctly\nthey count elements of @difftree array (or do equivalent of counting)\n_before_ parsing.\n\n> So I think your change is probably a good one, but I suspect you\n> probably are better off to make parse_difftree_raw_line() to\n> barf when it gets the commit object name to make sure that all\n> callers strip it at the same time; \n\nI'd rather not do that, and leave parse_difftree_raw_line generic.\n\n>                                    better yet, perhaps you can \n> have a single function that invokes git-diff-tree -r (with\n> different parameters) and returns the result that is already\n> only the difftree body lines?\n\nThat would be good idea... if not for the fact that git_commitdiff\nuses --patch-with-raw format... but that would make it only slightly\nmore complicated (strip commit-id, or rather tree-ish from output,\nand stop on end of raw part i.e. on first empty line).\n\nI'll do that (unless someone else would do this first).\n-- \nJakub Narebski\n"},{"id":"295560","messageId":"200610251417.20076.jnareb@gmail.com","threadId":"6036","inReplyTo":"200610241354.49396.jnareb@gmail.com","subject":"[PATCH 4/3] gitweb: Use --no-commit-id in git_commit and git_commitdiff","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-25T12:17:19Z","receivedAt":"2006-10-25T12:17:19Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Use --no-commit-id option to git diff-tree command in git_commit and\ngit_commitdiff to filter out commit ID output that git-diff-tree adds\nwhen called with only one <tree-ish> (not only for --stdin).\n\nThis option is in git since at least v1.0.0, so make use of it.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nGaaah, --no-commit-id is at least since v1.0.0, and we do require\n--full-history and --git-dir options which are much later additions.\n\nThis is \"correction\" patch.\n\n gitweb/gitweb.perl |   11 ++++-------\n 1 files changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex e35ecb4..345e336 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3004,14 +3004,12 @@ sub git_commit {\n \tif (!defined $parent) {\n \t\t$parent = \"--root\";\n \t}\n-\topen my $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts, $parent, $hash\n+\topen my $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', \"--no-commit-id\",\n+\t\t@diff_opts, $parent, $hash\n \t\tor die_error(undef, \"Open git-diff-tree failed\");\n \tmy @difftree = map { chomp; $_ } <$fd>;\n \tclose $fd or die_error(undef, \"Reading git-diff-tree failed\");\n \n-\t# filter out commit ID output\n-\t@difftree = grep(!/^[0-9a-fA-F]{40}$/, @difftree);\n-\n \t# non-textual hash id's can be cached\n \tmy $expires;\n \tif ($hash =~ m/^[0-9a-fA-F]{40}$/) {\n@@ -3324,15 +3322,14 @@ sub git_commitdiff {\n \tmy @difftree;\n \tif ($format eq 'html') {\n \t\topen $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n+\t\t\t\"--no-commit-id\",\n \t\t\t\"--patch-with-raw\", \"--full-index\", $hash_parent, $hash\n \t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n \n \t\twhile (chomp(my $line = <$fd>)) {\n \t\t\t# empty line ends raw part of diff-tree output\n \t\t\tlast unless $line;\n-\t\t\t# filter out commit ID output\n-\t\t\tpush @difftree, $line\n-\t\t\t\tunless $line =~ m/^[0-9a-fA-F]{40}$/;\n+\t\t\tpush @difftree, $line;\n \t\t}\n \n \t} elsif ($format eq 'plain') {\n-- \n1.4.2.1\n"},{"id":"297907","messageId":"200610251423.21739.jnareb@gmail.com","threadId":"6036","inReplyTo":"200610241354.49396.jnareb@gmail.com","subject":"[PATCH 2/3 (amend)] gitweb: Use --no-commit-id in git_commit and git_commitdiff","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-25T12:23:21Z","receivedAt":"2006-10-25T12:23:21Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Use --no-commit-id option to git diff-tree command in git_commit and\ngit_commitdiff to filter out commit ID output that git-diff-tree adds\nwhen called with only one <tree-ish> (not only for --stdin).\n\nThis option is in git since at least v1.0.0, so make use of it.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis is \"amend\" patch. It adds --no-commit-id option to git-diff-tree\ninvocation instead of removing commit IDs (tree-ish IDs to be more exact)\nfrom git-diff-tree output.\n\nStill, I'm not sure why git-diff-tree outputs tree-ish before diff\nwhen called with only one tree-ish. Bugwards compatibility?\n\n gitweb/gitweb.perl |   4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 8ac60be..345e336 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3004,7 +3004,8 @@ sub git_commit {\n \tif (!defined $parent) {\n \t\t$parent = \"--root\";\n \t}\n-\topen my $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts, $parent, $hash\n+\topen my $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', \"--no-commit-id\",\n+\t\t@diff_opts, $parent, $hash\n \t\tor die_error(undef, \"Open git-diff-tree failed\");\n \tmy @difftree = map { chomp; $_ } <$fd>;\n \tclose $fd or die_error(undef, \"Reading git-diff-tree failed\");\n@@ -3321,6 +3322,7 @@ sub git_commitdiff {\n \tmy @difftree;\n \tif ($format eq 'html') {\n \t\topen $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n+\t\t\t\"--no-commit-id\",\n \t\t\t\"--patch-with-raw\", \"--full-index\", $hash_parent, $hash\n \t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n \n-- \n1.4.2.1\n"}]}