{"thread":{"id":"7394","subject":"[PATCH] gitweb: show no difference message","startedAt":"2007-03-25T20:34:17Z","lastAt":"2007-04-05T10:38:49Z","messageCount":28,"participants":["Martin Koegler","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"37942","messageId":"11748548622888-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"7394","inReplyTo":null,"subject":"[PATCH] gitweb: show no difference message","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-25T20:34:17Z","receivedAt":"2007-03-25T20:34:17Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Currently, gitweb shows only header and footer, if no differences are\nfound. This patch adds a \"No differences are found\" message for the html\noutput.\n\nSigned-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>\n---\n gitweb/gitweb.perl |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 5214050..fbadab4 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2376,6 +2376,7 @@ sub git_patchset_body {\n \n \tmy $patch_idx = 0;\n \tmy $patch_line;\n+\tmy $empty = 0;\n \tmy $diffinfo;\n \tmy (%from, %to);\n \n@@ -2396,6 +2397,7 @@ sub git_patchset_body {\n \t\t# git diff header\n \t\t#assert($patch_line =~ m/^diff /) if DEBUG;\n \t\t#assert($patch_line !~ m!$/$!) if DEBUG; # is chomp-ed\n+\t\t$empty++;\n \t\tpush @diff_header, $patch_line;\n \n \t\t# extended diff header\n@@ -2559,6 +2561,8 @@ sub git_patchset_body {\n \t\tprint \"</div>\\n\"; # class=\"patch\"\n \t}\n \n+\tprint \"<div class=\\\"diff header\\\">No differences found</div>\\n\" if (!$empty);\n+\n \tprint \"</div>\\n\"; # class=\"patchset\"\n }\n \n-- \n1.5.1.rc1.51.gb08b-dirty\n"},{"id":"37943","messageId":"11748548621186-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"7394","inReplyTo":"11748548622888-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH] gitweb: Support comparing blobs with different names","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-25T20:34:18Z","receivedAt":"2007-03-25T20:34:18Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Currently, blobdiff can only compare blobs with diffenrent file\nnames, if no hb/hpb parameters are present.\n\nThis patch adds support for comparing two blobs specified by hb/f\nand hpb/fp.\n\nSigned-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>\n---\n gitweb/gitweb.perl |   27 ++++++++++++++++++++++++++-\n 1 files changed, 26 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex fbadab4..e3c2918 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3882,9 +3882,34 @@ sub git_blobdiff {\n \tmy %diffinfo;\n \tmy $expires;\n \n+\tif (defined $hash_base && defined $hash_parent_base &&\n+\t    defined $file_name && defined $file_parent) {\n+\n+\t\t$diffinfo{'from_mode'} = $diffinfo{'to_mode'} = \"blob\";\n+\t\t$diffinfo{'from_file'} = $file_parent;\n+\t\t$diffinfo{'to_file'}   = $file_name;\n+\t\t$diffinfo{'status'} = '2';\n+\n+\t\tif (defined $hash) {\n+\t\t    $diffinfo{'to_id'}   = $hash;\n+\t\t} else {\n+\t\t    $diffinfo{'to_id'}   = git_get_hash_by_path($hash_base, $file_name);\n+\t\t}\n+\t\tif (defined $hash_parent) {\n+\t\t    $diffinfo{'from_id'}   = $hash_parent;\n+\t\t} else {\n+\t\t    $diffinfo{'from_id'}   = git_get_hash_by_path($hash_parent_base, $file_parent);\n+\t\t}\n+\n+\t\t# open patch output\n+\t\topen $fd, \"-|\", git_cmd(), \"diff\", '-p', @diff_opts,\n+\t\t\t$hash_parent_base.\":\".$file_parent, $hash_base.\":\".$file_name, \"--\"\n+\t\t\tor die_error(undef, \"Open git-diff failed\");\n+\t}\n+\n \t# preparing $fd and %diffinfo for git_patchset_body\n \t# new style URI\n-\tif (defined $hash_base && defined $hash_parent_base) {\n+\tif (defined $hash_base && defined $hash_parent_base && !%diffinfo) {\n \t\tif (defined $file_name) {\n \t\t\t# read raw output\n \t\t\topen $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n-- \n1.5.1.rc1.51.gb08b-dirty\n"},{"id":"37944","messageId":"11748548623841-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"7394","inReplyTo":"11748548621186-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH] gitweb: link base commit (hpb) to blobdiff output","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-25T20:34:19Z","receivedAt":"2007-03-25T20:34:19Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"blobdiff only has links for the \"TO\" commit. This patch\nadds the same links for the \"FROM\" commit.\n\nSigned-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>\n---\n gitweb/gitweb.perl |   15 +++++++++++++++\n 1 files changed, 15 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex e3c2918..4c371b2 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -4014,6 +4014,21 @@ sub git_blobdiff {\n \t\t\t                       hash_base=>$hash_base, hash_parent_base=>$hash_parent_base,\n \t\t\t                       file_name=>$file_name, file_parent=>$file_parent)},\n \t\t\t        \"raw\");\n+\t\tif (defined $hash_parent_base && (my %co = parse_commit($hash_parent_base))) {\n+\t\t\t$formats_nav .= \" | from: \".\n+\t\t\t\t$cgi->a({-href => href(action=>\"commit\",\n+\t\t\t\t                       hash=>$hash_parent_base)},\n+\t\t\t\t        \"commit\")\n+\t\t\t\t.\" | \".\n+\t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n+\t\t\t\t                       hash=>$hash_parent_base)},\n+\t\t\t\t        \"commitdiff\")\n+\t\t\t\t.\" | \".\n+\t\t\t\t$cgi->a({-href => href(action=>\"tree\",\n+\t\t\t\t\t\t       hash=>$co{'tree'},\n+\t\t\t\t                       hash_base=>$hash_parent_base)},\n+\t\t\t\t        \"tree\");\n+\t\t}\n \t\tgit_header_html(undef, $expires);\n \t\tif (defined $hash_base && (my %co = parse_commit($hash_base))) {\n \t\t\tgit_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);\n-- \n1.5.1.rc1.51.gb08b-dirty\n"},{"id":"37945","messageId":"11748548622060-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"7394","inReplyTo":"11748548623841-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH] gitweb: support filename prefix in git_patchset_body","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-25T20:34:20Z","receivedAt":"2007-03-25T20:34:20Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"git_treediff supports comparing subdirectories. As the output of\ngit-difftree is missing the path to the compared directories,\nthe links in the output would be wrong.\n\nThe patch adds two new parameters to add the missing path prefix.\n\nSigned-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>\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 4c371b2..4195b1a 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2372,7 +2372,7 @@ sub git_difftree_body {\n }\n \n sub git_patchset_body {\n-\tmy ($fd, $difftree, $hash, $hash_parent) = @_;\n+\tmy ($fd, $difftree, $hash, $hash_parent, $file_name, $file_parent) = @_;\n \n \tmy $patch_idx = 0;\n \tmy $patch_line;\n@@ -2380,6 +2380,9 @@ sub git_patchset_body {\n \tmy $diffinfo;\n \tmy (%from, %to);\n \n+\t$file_name = (!defined $file_name)?\"\":($file_name.\"/\");\n+\t$file_parent = (!defined $file_parent)?\"\":($file_parent.\"/\");\n+\n \tprint \"<div class=\\\"patchset\\\">\\n\";\n \n \t# skip to first patch\n@@ -2439,14 +2442,14 @@ sub git_patchset_body {\n \t\t\tif ($diffinfo->{'status'} ne \"A\") { # not new (added) file\n \t\t\t\t$from{'href'} = href(action=>\"blob\", hash_base=>$hash_parent,\n \t\t\t\t                     hash=>$diffinfo->{'from_id'},\n-\t\t\t\t                     file_name=>$from{'file'});\n+\t\t\t\t                     file_name=>$file_parent.$from{'file'});\n \t\t\t} else {\n \t\t\t\tdelete $from{'href'};\n \t\t\t}\n \t\t\tif ($diffinfo->{'status'} ne \"D\") { # not deleted file\n \t\t\t\t$to{'href'} = href(action=>\"blob\", hash_base=>$hash,\n \t\t\t\t                   hash=>$diffinfo->{'to_id'},\n-\t\t\t\t                   file_name=>$to{'file'});\n+\t\t\t\t                   file_name=>$file_name.$to{'file'});\n \t\t\t} else {\n \t\t\t\tdelete $to{'href'};\n \t\t\t}\n-- \n1.5.1.rc1.51.gb08b-dirty\n"},{"id":"37946","messageId":"11748548621755-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"7394","inReplyTo":"11748548622060-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH] gitweb: support filename prefix in git_difftree_body","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-25T20:34:21Z","receivedAt":"2007-03-25T20:34:21Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"git_treediff supports comparing subdirectories. As the output of\ngit-difftree is missing the path to the compared directories,\nthe links in the output would be wrong.\n\nThe patch adds two new parameters to add the missing path prefix.\n\nSigned-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>\n---\n gitweb/gitweb.perl |   40 ++++++++++++++++++++++------------------\n 1 files changed, 22 insertions(+), 18 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4195b1a..95723c7 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2182,8 +2182,12 @@ sub git_print_tree_entry {\n ## functions printing large fragments of HTML\n \n sub git_difftree_body {\n-\tmy ($difftree, $hash, $parent) = @_;\n+\tmy ($difftree, $hash, $parent, $file_name, $file_parent) = @_;\n \tmy ($have_blame) = gitweb_check_feature('blame');\n+\n+\t$file_name = (!defined $file_name)?\"\":($file_name.\"/\");\n+\t$file_parent = (!defined $file_parent)?\"\":($file_parent.\"/\");\n+\n \tprint \"<div class=\\\"list_head\\\">\\n\";\n \tif ($#{$difftree} > 10) {\n \t\tprint(($#{$difftree} + 1) . \" files changed:\\n\");\n@@ -2226,7 +2230,7 @@ sub git_difftree_body {\n \t\t\t$mode_chng   .= \"]</span>\";\n \t\t\tprint \"<td>\";\n \t\t\tprint $cgi->a({-href => href(action=>\"blob\", hash=>$diff{'to_id'},\n-\t\t\t                             hash_base=>$hash, file_name=>$diff{'file'}),\n+\t\t\t                             hash_base=>$hash, file_name=>$file_name.$diff{'file'}),\n \t\t\t              -class => \"list\"}, esc_path($diff{'file'}));\n \t\t\tprint \"</td>\\n\";\n \t\t\tprint \"<td>$mode_chng</td>\\n\";\n@@ -2238,7 +2242,7 @@ sub git_difftree_body {\n \t\t\t\tprint \" | \";\n \t\t\t}\n \t\t\tprint $cgi->a({-href => href(action=>\"blob\", hash=>$diff{'to_id'},\n-\t\t\t                             hash_base=>$hash, file_name=>$diff{'file'})},\n+\t\t\t                             hash_base=>$hash, file_name=>$file_name.$diff{'file'})},\n \t\t\t              \"blob\");\n \t\t\tprint \"</td>\\n\";\n \n@@ -2246,7 +2250,7 @@ sub git_difftree_body {\n \t\t\tmy $mode_chng = \"<span class=\\\"file_status deleted\\\">[deleted $from_file_type]</span>\";\n \t\t\tprint \"<td>\";\n \t\t\tprint $cgi->a({-href => href(action=>\"blob\", hash=>$diff{'from_id'},\n-\t\t\t                             hash_base=>$parent, file_name=>$diff{'file'}),\n+\t\t\t                             hash_base=>$parent, file_name=>$file_parent.$diff{'file'}),\n \t\t\t               -class => \"list\"}, esc_path($diff{'file'}));\n \t\t\tprint \"</td>\\n\";\n \t\t\tprint \"<td>$mode_chng</td>\\n\";\n@@ -2258,15 +2262,15 @@ sub git_difftree_body {\n \t\t\t\tprint \" | \";\n \t\t\t}\n \t\t\tprint $cgi->a({-href => href(action=>\"blob\", hash=>$diff{'from_id'},\n-\t\t\t                             hash_base=>$parent, file_name=>$diff{'file'})},\n+\t\t\t                             hash_base=>$parent, file_name=>$file_parent.$diff{'file'})},\n \t\t\t              \"blob\") . \" | \";\n \t\t\tif ($have_blame) {\n \t\t\t\tprint $cgi->a({-href => href(action=>\"blame\", hash_base=>$parent,\n-\t\t\t\t                             file_name=>$diff{'file'})},\n+\t\t\t\t                             file_name=>$file_parent.$diff{'file'})},\n \t\t\t\t              \"blame\") . \" | \";\n \t\t\t}\n \t\t\tprint $cgi->a({-href => href(action=>\"history\", hash_base=>$parent,\n-\t\t\t                             file_name=>$diff{'file'})},\n+\t\t\t                             file_name=>$file_parent.$diff{'file'})},\n \t\t\t              \"history\");\n \t\t\tprint \"</td>\\n\";\n \n@@ -2288,7 +2292,7 @@ sub git_difftree_body {\n \t\t\t}\n \t\t\tprint \"<td>\";\n \t\t\tprint $cgi->a({-href => href(action=>\"blob\", hash=>$diff{'to_id'},\n-\t\t\t                             hash_base=>$hash, file_name=>$diff{'file'}),\n+\t\t\t                             hash_base=>$hash, file_name=>$file_name.$diff{'file'}),\n \t\t\t              -class => \"list\"}, esc_path($diff{'file'}));\n \t\t\tprint \"</td>\\n\";\n \t\t\tprint \"<td>$mode_chnge</td>\\n\";\n@@ -2303,20 +2307,20 @@ sub git_difftree_body {\n \t\t\t\tprint $cgi->a({-href => href(action=>\"blobdiff\",\n \t\t\t\t                             hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},\n \t\t\t\t                             hash_base=>$hash, hash_parent_base=>$parent,\n-\t\t\t\t                             file_name=>$diff{'file'})},\n+\t\t\t\t                             file_name=>$file_name.$diff{'file'})},\n \t\t\t\t              \"diff\") .\n \t\t\t\t      \" | \";\n \t\t\t}\n \t\t\tprint $cgi->a({-href => href(action=>\"blob\", hash=>$diff{'to_id'},\n-\t\t\t                             hash_base=>$hash, file_name=>$diff{'file'})},\n+\t\t\t                             hash_base=>$hash, file_name=>$file_name.$diff{'file'})},\n \t\t\t               \"blob\") . \" | \";\n \t\t\tif ($have_blame) {\n \t\t\t\tprint $cgi->a({-href => href(action=>\"blame\", hash_base=>$hash,\n-\t\t\t\t                             file_name=>$diff{'file'})},\n+\t\t\t\t                             file_name=>$file_name.$diff{'file'})},\n \t\t\t\t              \"blame\") . \" | \";\n \t\t\t}\n \t\t\tprint $cgi->a({-href => href(action=>\"history\", hash_base=>$hash,\n-\t\t\t                             file_name=>$diff{'file'})},\n+\t\t\t                             file_name=>$file_name.$diff{'file'})},\n \t\t\t              \"history\");\n \t\t\tprint \"</td>\\n\";\n \n@@ -2330,11 +2334,11 @@ sub git_difftree_body {\n \t\t\t}\n \t\t\tprint \"<td>\" .\n \t\t\t      $cgi->a({-href => href(action=>\"blob\", hash_base=>$hash,\n-\t\t\t                             hash=>$diff{'to_id'}, file_name=>$diff{'to_file'}),\n+\t\t\t                             hash=>$diff{'to_id'}, file_name=>$file_name.$diff{'to_file'}),\n \t\t\t              -class => \"list\"}, esc_path($diff{'to_file'})) . \"</td>\\n\" .\n \t\t\t      \"<td><span class=\\\"file_status $nstatus\\\">[$nstatus from \" .\n \t\t\t      $cgi->a({-href => href(action=>\"blob\", hash_base=>$parent,\n-\t\t\t                             hash=>$diff{'from_id'}, file_name=>$diff{'from_file'}),\n+\t\t\t                             hash=>$diff{'from_id'}, file_name=>$file_parent.$diff{'from_file'}),\n \t\t\t              -class => \"list\"}, esc_path($diff{'from_file'})) .\n \t\t\t      \" with \" . (int $diff{'similarity'}) . \"% similarity$mode_chng]</span></td>\\n\" .\n \t\t\t      \"<td class=\\\"link\\\">\";\n@@ -2348,20 +2352,20 @@ sub git_difftree_body {\n \t\t\t\tprint $cgi->a({-href => href(action=>\"blobdiff\",\n \t\t\t\t                             hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},\n \t\t\t\t                             hash_base=>$hash, hash_parent_base=>$parent,\n-\t\t\t\t                             file_name=>$diff{'to_file'}, file_parent=>$diff{'from_file'})},\n+\t\t\t\t                             file_name=>$file_name.$diff{'to_file'}, file_parent=>$file_parent.$diff{'from_file'})},\n \t\t\t\t              \"diff\") .\n \t\t\t\t      \" | \";\n \t\t\t}\n \t\t\tprint $cgi->a({-href => href(action=>\"blob\", hash=>$diff{'to_id'},\n-\t\t\t                             hash_base=>$parent, file_name=>$diff{'to_file'})},\n+\t\t\t                             hash_base=>$parent, file_name=>$file_name.$diff{'to_file'})},\n \t\t\t              \"blob\") . \" | \";\n \t\t\tif ($have_blame) {\n \t\t\t\tprint $cgi->a({-href => href(action=>\"blame\", hash_base=>$hash,\n-\t\t\t\t                             file_name=>$diff{'to_file'})},\n+\t\t\t\t                             file_name=>$file_name.$diff{'to_file'})},\n \t\t\t\t              \"blame\") . \" | \";\n \t\t\t}\n \t\t\tprint $cgi->a({-href => href(action=>\"history\", hash_base=>$hash,\n-\t\t\t                            file_name=>$diff{'to_file'})},\n+\t\t\t                            file_name=>$file_name.$diff{'to_file'})},\n \t\t\t              \"history\");\n \t\t\tprint \"</td>\\n\";\n \n-- \n1.5.1.rc1.51.gb08b-dirty\n"},{"id":"37947","messageId":"11748548623081-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"7394","inReplyTo":"11748548621755-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH] gitweb: Add treediff","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-25T20:34:22Z","receivedAt":"2007-03-25T20:34:22Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"treediff supports comparing different trees. A tree can be specified\neither as hash or as base hash and filename.\n\nSigned-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>\n---\n gitweb/gitweb.perl |  131 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 131 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 95723c7..52c0c13 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -446,6 +446,8 @@ my %actions = (\n \t\"tag\" => \\&git_tag,\n \t\"tags\" => \\&git_tags,\n \t\"tree\" => \\&git_tree,\n+\t\"treediff\" => \\&git_treediff,\n+\t\"treediff_plain\" => \\&git_treediff_plain,\n \t\"snapshot\" => \\&git_snapshot,\n \t\"object\" => \\&git_object,\n \t# those below don't need $project\n@@ -4242,6 +4244,135 @@ sub git_commitdiff_plain {\n \tgit_commitdiff('plain');\n }\n \n+\n+sub git_treediff {\n+\tmy $format = shift || 'html';\n+\tmy $from = $file_parent || \"\";\n+\tmy $to = $file_name || \"\";\n+\n+\tif (!defined $hash) {\n+\t\tif (!defined $hash_base) {\n+\t\t\tdie_error('','tree parameter missing');\n+\t\t}\n+\t\t$hash = $hash_base;\n+\t\t$hash .= \":\".$file_name if (defined $file_name);\n+\t}\n+\n+\tif (!defined $hash_parent) {\n+\t\tif (!defined $hash_parent_base) {\n+\t\t\tdie_error('','tree parameter missing');\n+\t\t}\n+\t\t$hash_parent = $hash_parent_base;\n+\t\t$hash_parent .= \":\".$file_parent if (defined $file_parent);\n+\t}\n+\n+\t# we need to prepare $formats_nav before any parameter munging\n+\tmy $formats_nav;\n+\tif ($format eq 'html') {\n+\t\t$formats_nav =\n+\t\t\t$cgi->a({-href => href(action=>\"treediff_plain\",\n+\t\t\t                       hash=>$hash, hash_parent=>$hash_parent,\n+\t\t\t                       hash_base=>$hash_base, hash_parent_base=>$hash_parent_base,\n+\t\t\t                       file_name=>$file_name, file_parent=>$file_parent)},\n+\t\t\t        \"raw\");\n+\n+\t\tif (defined $hash_parent_base && (my %co = parse_commit($hash_parent_base))) {\n+\t \t\t$formats_nav .= \" | from: \".\n+\t\t\t\t$cgi->a({-href => href(action=>\"commit\",\n+\t\t\t        \t               hash=>$hash_parent_base)},\n+\t\t\t\t        \"commit\")\n+\t\t\t\t.\" | \".\n+\t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n+\t\t\t\t                       hash=>$hash_parent_base)},\n+\t\t\t\t        \"commitdiff\")\n+\t\t\t\t.\" | \".\n+\t\t\t\t$cgi->a({-href => href(action=>\"tree\",\n+\t\t\t\t\t\t       hash=>$co{'tree'},\n+\t\t\t\t                       hash_base=>$hash_parent_base)},\n+\t\t\t\t        \"tree\");\n+\t\t}\n+\t}\n+\n+\t# read treediff\n+\tmy $fd;\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\", \"--patch-with-raw\", \"--full-index\",\n+\t\t\t$hash_parent, $hash, \"--\"\n+\t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n+\n+\t\twhile (my $line = <$fd>) {\n+\t\t\tchomp $line;\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}\n+\n+\t} elsif ($format eq 'plain') {\n+\t\topen $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n+\t\t\t'-p', $hash_parent, $hash, \"--\"\n+\t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n+\n+\t} else {\n+\t\tdie_error(undef, \"Unknown treediff format\");\n+\t}\n+\n+\t# non-textual hash id's can be cached\n+\tmy $expires;\n+\tif ($hash =~ m/^[0-9a-fA-F]{40}$/) {\n+\t\t$expires = \"+1d\";\n+\t}\n+\n+\t# write header\n+\tif ($format eq 'html') {\n+\n+\t\tgit_header_html(undef, $expires);\n+\t\tif (defined $hash_base && (my %co = parse_commit($hash_base))) {\n+\t\t\tgit_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);\n+\t\t\tgit_print_header_div('commit', esc_html($co{'title'}), $hash_base);\n+\t\t\tprint \"<div class=\\\"title\\\">$hash_base:$from vs $hash_parent_base:$to</div>\\n\";\n+\t\t} else {\n+\t\t\tprint \"<div class=\\\"page_nav\\\"><br/>$formats_nav<br/></div>\\n\";\n+\t\t\tprint \"<div class=\\\"title\\\">$hash vs $hash_parent</div>\\n\";\n+\t\t}\n+\t\tprint \"<div class=\\\"page_body\\\">\\n\";\n+\n+\t} elsif ($format eq 'plain') {\n+\t\tmy $filename = basename($project) . \"-diff.patch\";\n+\n+\t\tprint $cgi->header(\n+\t\t\t-type => 'text/plain',\n+\t\t\t-charset => 'utf-8',\n+\t\t\t-expires => $expires,\n+\t\t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"');\n+\n+\t\tprint \"X-Git-Url: \" . $cgi->self_url() . \"\\n\\n\";\n+\t\tprint \"---\\n\\n\";\n+\t}\n+\n+\t# write patch\n+\tif ($format eq 'html') {\n+\t\tgit_difftree_body(\\@difftree, $hash_base, $hash_parent_base, $file_name, $file_parent);\n+\t\tprint \"<br/>\\n\";\n+\n+\t\tgit_patchset_body($fd, \\@difftree, $hash_base, $hash_parent_base, $file_name, $file_parent);\n+\t\tclose $fd;\n+\t\tprint \"</div>\\n\"; # class=\"page_body\"\n+\t\tgit_footer_html();\n+\n+\t} elsif ($format eq 'plain') {\n+\t\tlocal $/ = undef;\n+\t\tprint <$fd>;\n+\t\tclose $fd\n+\t\t\tor print \"Reading git-diff-tree failed\\n\";\n+\t}\n+}\n+\n+sub git_treediff_plain {\n+\tgit_treediff('plain');\n+}\n+\n sub git_history {\n \tif (!defined $hash_base) {\n \t\t$hash_base = git_get_head_hash($project);\n-- \n1.5.1.rc1.51.gb08b-dirty\n"},{"id":"38044","messageId":"200703261911.54013.jnareb@gmail.com","threadId":"7394","inReplyTo":"11748548622888-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"Re: [PATCH] gitweb: show no difference message","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-26T17:11:53Z","receivedAt":"2007-03-26T17:11:53Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, Mar 25, 2007, Martin Koegler wrote:\n\n> Currently, gitweb shows only header and footer, if no differences are\n> found. This patch adds a \"No differences are found\" message for the html\n> output.\n\nThis is a good idea, as it reduces confusion for first-time gitweb user,\nwho might not know what it means to have an empty diff page.\n\nCurrently we have only one place (I think) where gitweb can generate\nlink to \"blobdiff\", namely \"diff to parent\" link in \"history\" view\nfor plain file, when e.g. some change was (explicitely or accidentally)\nreverted.\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 5214050..fbadab4 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -2376,6 +2376,7 @@ sub git_patchset_body {\n>  \n>  \tmy $patch_idx = 0;\n>  \tmy $patch_line;\n> +\tmy $empty = 0;\n>  \tmy $diffinfo;\n>  \tmy (%from, %to);\n>  \n> @@ -2396,6 +2397,7 @@ sub git_patchset_body {\n>  \t\t# git diff header\n>  \t\t#assert($patch_line =~ m/^diff /) if DEBUG;\n>  \t\t#assert($patch_line !~ m!$/$!) if DEBUG; # is chomp-ed\n> +\t\t$empty++;\n>  \t\tpush @diff_header, $patch_line;\n>  \n>  \t\t# extended diff header\n> @@ -2559,6 +2561,8 @@ sub git_patchset_body {\n>  \t\tprint \"</div>\\n\"; # class=\"patch\"\n>  \t}\n>  \n> +\tprint \"<div class=\\\"diff header\\\">No differences found</div>\\n\" if (!$empty);\n> +\n>  \tprint \"</div>\\n\"; # class=\"patchset\"\n>  }\n>  \n\nA few nits.\n\nFirst, programming style. You named the variable $empty, when it\nevaluates to true when the patch is _not_ empty, i.e. when some\ndifferences are found.\n\nSecond, HTML. I'm not so sure if this info belongs to _patchset_,\nbut if git-diff or git-diff-tree returns empty patch output,\nthen we do not generate 'patch' div. So I'm a bit for this\nsolution.\n\nThird, CSS (style). I'm reluctant about using \"diff header\" for\nstyling \"no differences found\" div; it is used to style\n\n\tdiff --git a/filename b/filename\n\nheader. I think that it would be better to add a separate CSS class,\ne.g. \"diff notfound\", or \"diff nochanges\", or \"diff nodifferences\"\nclass for this message.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"38040","messageId":"200703261912.09445.jnareb@gmail.com","threadId":"7394","inReplyTo":"11748548621186-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"[PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-26T17:12:09Z","receivedAt":"2007-03-26T17:12:09Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Fix the bug that caused \"blobdiff\" view called with new style URI\n(it means with $hash_base ('hb') and $hash_parent_base ('hpb') denoting\ntree-ish, usually commit, which have blobs being compared) for\nrenamed files (it means with both $file_name ('f') and $file_parent ('fp')\nparameters set) to be show as new (added) file diff.\n\nIt is done by adding $file_parent ('fp') to the path limiter, meaning\nthat diff command becomes:\n\n\tgit diff-tree [options] hpb hb -- fp f\n\ninstead of finding hash of a blob using git_get_hash_by_path subroutine\nor using extended SHA-1 syntax:\n\n\tgit diff [options] hpb:fp hp:f\n\nCurrently code for \"blobdiff\" does not support well mixed style URI,\nfor example asking for diff between blob given by its hash only, or\nby hash and filename (without hash of tree-ish / commit), and blob\ngiven by hash base (hash of tree-ish / commit) and filename but without\nhash of blob itself, and probably would never will.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nMartin, sorry for the confusing suggestion about using tree-ish:path\nsyntax to compare (generate diff of) two file with different name.\n\nThis patch is less invasive and I think better solution.\n\n gitweb/gitweb.perl |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 5214050..c79bfeb 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3885,7 +3885,7 @@ sub git_blobdiff {\n \t\t\t# read raw output\n \t\t\topen $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n \t\t\t\t$hash_parent_base, $hash_base,\n-\t\t\t\t\"--\", $file_name\n+\t\t\t\t\"--\", (defined $file_parent ? $file_parent : ()), $file_name\n \t\t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n \t\t\t@difftree = map { chomp; $_ } <$fd>;\n \t\t\tclose $fd\n@@ -3935,7 +3935,7 @@ sub git_blobdiff {\n \t\t# open patch output\n \t\topen $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n \t\t\t'-p', $hash_parent_base, $hash_base,\n-\t\t\t\"--\", $file_name\n+\t\t\t\"--\", (defined $file_parent ? $file_parent : ()), $file_name\n \t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n \t}\n \n-- \n1.5.0.5\n"},{"id":"38042","messageId":"200703261912.18549.jnareb@gmail.com","threadId":"7394","inReplyTo":"11748548622060-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"Re: [PATCH] gitweb: support filename prefix in git_patchset_body","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-26T17:12:18Z","receivedAt":"2007-03-26T17:12:18Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, Mar 25, 2007, Martin Koegler wrote:\n\n> git_treediff supports comparing subdirectories. As the output of\n> git-difftree is missing the path to the compared directories,\n> the links in the output would be wrong.\n> \n> The patch adds two new parameters to add the missing path prefix.\n\nWouldn't it be better to concatenate the two \"path prefix\" patches\ntogether? They are about the same thing.\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 4c371b2..4195b1a 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -2372,7 +2372,7 @@ sub git_difftree_body {\n>  }\n>  \n>  sub git_patchset_body {\n> -\tmy ($fd, $difftree, $hash, $hash_parent) = @_;\n> +\tmy ($fd, $difftree, $hash, $hash_parent, $file_name, $file_parent) = @_;\n>  \n>  \tmy $patch_idx = 0;\n>  \tmy $patch_line;\n\nI'd rather use $from_prefix, $to_prefix here, or $basedif_name,\n$basedir_parent, or $dir_name, $dir_parent (my preference is to\n$from_prefix, $to_prefix variables).\n\n> @@ -2380,6 +2380,9 @@ sub git_patchset_body {\n>  \tmy $diffinfo;\n>  \tmy (%from, %to);\n>  \n> +\t$file_name = (!defined $file_name)?\"\":($file_name.\"/\");\n> +\t$file_parent = (!defined $file_parent)?\"\":($file_parent.\"/\");\n> +\n>  \tprint \"<div class=\\\"patchset\\\">\\n\";\n>  \n>  \t# skip to first patch\n\nMinor nit: I'd rather write\n\n+\t$from_prefix = !defined $from_prefix ? '' : $from_prefix.'/';\n+\t$to_prefix   = !defined $to_prefix   ? '' : $to_prefix . '/';\n+\t$to_prefix ||= $from_prefix;  # to allow to pass common prefix once\n+\n\nor something like that, or just modify $from{'file'} and $to{'file'}\n\n\t$from{'file'} = (!defined $from_prefix ? '' : $from_prefix.'/') . $from{'file'};\n\t$to{'file'}   = (!defined $to_prefix   ? '' : $to_prefix . '/') . $to{'file'};\n\njust after setting $from{'file'} and $to{'file'}, although the second\nsolution would additionally add prefix to the shown patch body itself.\n\n> @@ -2439,14 +2442,14 @@ sub git_patchset_body {\n>  \t\t\tif ($diffinfo->{'status'} ne \"A\") { # not new (added) file\n>  \t\t\t\t$from{'href'} = href(action=>\"blob\", hash_base=>$hash_parent,\n>  \t\t\t\t                     hash=>$diffinfo->{'from_id'},\n> -\t\t\t\t                     file_name=>$from{'file'});\n> +\t\t\t\t                     file_name=>$file_parent.$from{'file'});\n>  \t\t\t} else {\n>  \t\t\t\tdelete $from{'href'};\n>  \t\t\t}\n>  \t\t\tif ($diffinfo->{'status'} ne \"D\") { # not deleted file\n>  \t\t\t\t$to{'href'} = href(action=>\"blob\", hash_base=>$hash,\n>  \t\t\t\t                   hash=>$diffinfo->{'to_id'},\n> -\t\t\t\t                   file_name=>$to{'file'});\n> +\t\t\t\t                   file_name=>$file_name.$to{'file'});\n>  \t\t\t} else {\n>  \t\t\t\tdelete $to{'href'};\n>  \t\t\t}\n\nAnother solution would be to not add additional parameters to\ngit_difftree_body and git_patchset_body subroutines (although it is nice\ntouch towards completeness), but modify %diffinfo in the caller, but this\nwould change also patch contents (in from-file / to-file diff header, etc.)\nwhich might not be a good thing.\n\nI'm not sure if we should not add information somewhere that paths are\nprefixed/shortened, but this might be left for later patch.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"38043","messageId":"200703261912.27612.jnareb@gmail.com","threadId":"7394","inReplyTo":"11748548623081-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"Re: [PATCH] gitweb: Add treediff","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-26T17:12:27Z","receivedAt":"2007-03-26T17:12:27Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, Mar 25, 2007, Martin Koegler wrote:\n\n> treediff supports comparing different trees. A tree can be specified\n> either as hash or as base hash and filename.\n\nI'd use \"treediff\" view, or git_treediff subroutine. Just a minor nit.\n \n> +sub git_treediff {\n> +\tmy $format = shift || 'html';\n> +\tmy $from = $file_parent || \"\";\n> +\tmy $to = $file_name || \"\";\n\nI'd use  $file_name || '';  here, and of course\n\n+\tmy $from = $file_parent || $file_name || '';\n\nThe unwritten rule is that we use 'fp' parameter (thet $file_parent\nvariable is set) only \n\n> +\n> +\tif (!defined $hash) {\n> +\t\tif (!defined $hash_base) {\n> +\t\t\tdie_error('','tree parameter missing');\n\nThis conflicts with the coding style used elsewhere in the gitweb\n(the informal coding convention / guideline for gitweb).\n\nFirst, we either use  undef  and not  ''  to say: use default value\nof the first parameter (HTTP status) of die_error, or provide our\nown value in single quotes.\n\nSecond, the second parameter, error message, is a sentence (without\nfinal fullstop) describing error; it means starting it with capital\nletter. And we use double quotes for this parameter.\n\nExamples:\n\n\tdie_error(undef, \"Couldn't find base commit\");\n\tdie_error(undef, \"Unknown commit object\");\n\tdie_error(undef, \"No file name defined\");\n\tdie_error(undef, \"Open git-diff-tree failed\");\n\n\tdie_error('403 Permission denied', \"Permission denied\");\n\tdie_error('404 Not Found', \"File name not defined\");\n\tdie_error('404 Not Found', \"Not enough information to find object\");\n\tdie_error('400 Bad Request', \"Object is not a blob\");\n\n> +\t\t}\n> +\t\t$hash = $hash_base;\n> +\t\t$hash .= \":\".$file_name if (defined $file_name);\n> +\t}\n> +\n> +\tif (!defined $hash_parent) {\n> +\t\tif (!defined $hash_parent_base) {\n> +\t\t\tdie_error('','tree parameter missing');\n> +\t\t}\n> +\t\t$hash_parent = $hash_parent_base;\n> +\t\t$hash_parent .= \":\".$file_parent if (defined $file_parent);\n\nThe same problem as above: we do not set 'fp' parameter (and $file_parent\nvariable) if name does not change. So you should use instead:\n\n+\t\t$hash_parent .= \":\".($file_parent || $file_name)\n+\t\t\tif (defined $file_parent || defined $file_name);\n\n\nHere I think (contrary to blobdiff case) it is better to use extended\nSHA-1 syntax (taken from git-rev-parse(1)):\n\n * A suffix `:' followed by a path; this names the blob or tree at the given\n   path in the tree-ish object named by the part before the colon.\n\n> +\t}\n> +\n> +\t# we need to prepare $formats_nav before any parameter munging\n> +\tmy $formats_nav;\n> +\tif ($format eq 'html') {\n> +\t\t$formats_nav =\n> +\t\t\t$cgi->a({-href => href(action=>\"treediff_plain\",\n> +\t\t\t                       hash=>$hash, hash_parent=>$hash_parent,\n> +\t\t\t                       hash_base=>$hash_base, hash_parent_base=>$hash_parent_base,\n> +\t\t\t                       file_name=>$file_name, file_parent=>$file_parent)},\n> +\t\t\t        \"raw\");\n\n\nI certainly agree to that.\n\n> +\t\tif (defined $hash_parent_base && (my %co = parse_commit($hash_parent_base))) {\n> +\t \t\t$formats_nav .= \" | from: \".\n> +\t\t\t\t$cgi->a({-href => href(action=>\"commit\",\n> +\t\t\t        \t               hash=>$hash_parent_base)},\n> +\t\t\t\t        \"commit\")\n> +\t\t\t\t.\" | \".\n> +\t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n> +\t\t\t\t                       hash=>$hash_parent_base)},\n> +\t\t\t\t        \"commitdiff\")\n> +\t\t\t\t.\" | \".\n> +\t\t\t\t$cgi->a({-href => href(action=>\"tree\",\n> +\t\t\t\t\t\t       hash=>$co{'tree'},\n> +\t\t\t\t                       hash_base=>$hash_parent_base)},\n> +\t\t\t\t        \"tree\");\n> +\t\t}\n> +\t}\n\nThis depends if the similar change (feature) for \"blobdiff\" view\n(in git_blobdiff subroutine) is accepted. Perhaps this could be\nseparated into separate patch?\n\n> +\n> +\t# read treediff\n> +\tmy $fd;\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\", \"--patch-with-raw\", \"--full-index\",\n> +\t\t\t$hash_parent, $hash, \"--\"\n> +\t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n> +\n> +\t\twhile (my $line = <$fd>) {\n> +\t\t\tchomp $line;\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}\n\nThis is also common with git_commitdiff. Should be it put into separate\nsubroutine? Or do this refactoring in another patch?\n\n> +\t} elsif ($format eq 'plain') {\n> +\t\topen $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n> +\t\t\t'-p', $hash_parent, $hash, \"--\"\n> +\t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n\nFor \"commitdiff_plain\" view the email-like format, with commit message\nand the patch is I think enough. The commit message serves as summary\nfor this view.\n\nI think that it would be better for \"treediff_plain\" to have\nwhatchanged-like (with shothened sha1 for example) plain difftree\nbefore the patchset.\n\n> +\t} else {\n> +\t\tdie_error(undef, \"Unknown treediff format\");\n\nAnd here you use standard convention to call die_error.\n\n> +\t}\n> +\n> +\t# non-textual hash id's can be cached\n> +\tmy $expires;\n> +\tif ($hash =~ m/^[0-9a-fA-F]{40}$/) {\n> +\t\t$expires = \"+1d\";\n> +\t}\n> +\n> +\t# write header\n> +\tif ($format eq 'html') {\n> +\n> +\t\tgit_header_html(undef, $expires);\n> +\t\tif (defined $hash_base && (my %co = parse_commit($hash_base))) {\n> +\t\t\tgit_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);\n> +\t\t\tgit_print_header_div('commit', esc_html($co{'title'}), $hash_base);\n> +\t\t\tprint \"<div class=\\\"title\\\">$hash_base:$from vs $hash_parent_base:$to</div>\\n\";\n> +\t\t} else {\n> +\t\t\tprint \"<div class=\\\"page_nav\\\"><br/>$formats_nav<br/></div>\\n\";\n> +\t\t\tprint \"<div class=\\\"title\\\">$hash vs $hash_parent</div>\\n\";\n> +\t\t}\n> +\t\tprint \"<div class=\\\"page_body\\\">\\n\";\n\nUsually we use title of the $hash_base ('hb') commit as a page header,\nand I think we should do the same for \"treediff\" view. Only if it is not\npossible we use \"$hash vs $hash_parent\" or equivalent. Do not cause\ninconsistency, please. You can propose patch changing this, but make\nit separate issue.\n\nAdditionally, \"blob\" view has page_path, and so has \"blobdiff\" view (it is\nfor $hash / $filename / $hash_base). And \"tree\" view has page_path, so\nI think should \"treediff\" have; of course if $hash_base is defined.\n\n> +\n> +\t} elsif ($format eq 'plain') {\n> +\t\tmy $filename = basename($project) . \"-diff.patch\";\n> +\n\nIn \"commitdiff_plain\" view we use\n\n\t\tmy $filename = basename($project) . \"-$hash.patch\";\n\nPerhaps we should use the same: \"-diff.patch\" does not make much sense.\nIs it a typo?\n\n[...]\n\nSome of further code could also be shared between git_treediff and\ngit_commitdiff, but this could wait I guess for another separate patch.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"38062","messageId":"20070326204116.GB1128@auto.tuwien.ac.at","threadId":"7394","inReplyTo":"200703261912.09445.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-26T20:41:16Z","receivedAt":"2007-03-26T20:41:16Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"On Mon, Mar 26, 2007 at 06:12:09PM +0100, Jakub Narebski wrote:\n> It is done by adding $file_parent ('fp') to the path limiter, meaning\n> that diff command becomes:\n> \n> \tgit diff-tree [options] hpb hb -- fp f\n> \n> instead of finding hash of a blob using git_get_hash_by_path subroutine\n> or using extended SHA-1 syntax:\n> \n> \tgit diff [options] hpb:fp hp:f\n> \n\nAs far as I tested, this only works for renames, not\nfor comparing different objects, eg:\n\ngit-diff -r -p 08727ea8bba8c81678e5cf15124ada23ad097bc3:grep.h bb95e19c5f1e470d2efe1c0e4e04c291019e4b25:refs.h\nshows differences\n\ngit-diff-tree -r 08727ea8bba8c81678e5cf15124ada23ad097bc3 bb95e19c5f1e470d2efe1c0e4e04c291019e4b25 -- grep.h refs.h\nshows no difference\n\nAs I want gitweb to be able to even do such compares (not just renames),\nI'll still need a solution for this.\n\nMy idea is, that if I got hb:f and hpb:fp, the user exactly specified\nthe blobs to be compared. Then I don't want any guessing logic.\n\nmfg Martin Kögler\n"},{"id":"38065","messageId":"20070326205508.GC1128@auto.tuwien.ac.at","threadId":"7394","inReplyTo":"200703261912.18549.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: support filename prefix in git_patchset_body","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-26T20:55:08Z","receivedAt":"2007-03-26T20:55:08Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"On Mon, Mar 26, 2007 at 06:12:18PM +0100, Jakub Narebski wrote:\n> On Sun, Mar 25, 2007, Martin Koegler wrote:\n> > git_treediff supports comparing subdirectories. As the output of\n> > git-difftree is missing the path to the compared directories,\n> > the links in the output would be wrong.\n> > \n> > The patch adds two new parameters to add the missing path prefix.\n> \n> Wouldn't it be better to concatenate the two \"path prefix\" patches\n> together? They are about the same thing.\n\nI thought, each patch would be more readable, I split them in logical\nseperate units. Anyway, I'll combine them.\n\n> >  sub git_patchset_body {\n> > -\tmy ($fd, $difftree, $hash, $hash_parent) = @_;\n> > +\tmy ($fd, $difftree, $hash, $hash_parent, $file_name, $file_parent) = @_;\n> >  \n> >  \tmy $patch_idx = 0;\n> >  \tmy $patch_line;\n> \n> I'd rather use $from_prefix, $to_prefix here, or $basedif_name,\n> $basedir_parent, or $dir_name, $dir_parent (my preference is to\n> $from_prefix, $to_prefix variables).\n\nI'll switch to $to_prefix and $from_prefix.\n\n> +\t$from_prefix = !defined $from_prefix ? '' : $from_prefix.'/';\n> +\t$to_prefix   = !defined $to_prefix   ? '' : $to_prefix . '/';\n> +\t$to_prefix ||= $from_prefix;  # to allow to pass common prefix once\n\nOK. But is not the 3 line useless, as $to_prefix is alway defined\nafter the second line and you probable want $from_prefix ||=\n$to_prefix. This will cause problems, as I currently pass the root\ntree (=tree in commit object) as an missing file name parameter, as\ngitweb does not allow an empty file name.\n\nWith an propagation logic, comparing a root tree with an sub tree will only\nwork in one direction.\n\nSo I prefer to do not implement any automatic propagation between the two prefixes.\n\n> or something like that, or just modify $from{'file'} and $to{'file'}\n> \n> \t$from{'file'} = (!defined $from_prefix ? '' : $from_prefix.'/') . $from{'file'};\n> \t$to{'file'}   = (!defined $to_prefix   ? '' : $to_prefix . '/') . $to{'file'};\n> \n> just after setting $from{'file'} and $to{'file'}, although the second\n> solution would additionally add prefix to the shown patch body itself.\n\nModifing the paths before generating the links is a good idea. I'll\nlook, where its useful.\n\nmfg Martin Kögler\n"},{"id":"38090","messageId":"200703262301.38858.jnareb@gmail.com","threadId":"7394","inReplyTo":"200703261911.54013.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: show no difference message","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-26T21:01:38Z","receivedAt":"2007-03-26T21:01:38Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, Mar 26, 2007, Jakub Narebski wrote:\n> On Sun, Mar 25, 2007, Martin Koegler wrote:\n\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index 5214050..fbadab4 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -2376,6 +2376,7 @@ sub git_patchset_body {\n>>  \n>>  \tmy $patch_idx = 0;\n>>  \tmy $patch_line;\n>> +\tmy $empty = 0;\n>>  \tmy $diffinfo;\n>>  \tmy (%from, %to);\n>>  \n>> @@ -2396,6 +2397,7 @@ sub git_patchset_body {\n>>  \t\t# git diff header\n>>  \t\t#assert($patch_line =~ m/^diff /) if DEBUG;\n>>  \t\t#assert($patch_line !~ m!$/$!) if DEBUG; # is chomp-ed\n>> +\t\t$empty++;\n>>  \t\tpush @diff_header, $patch_line;\n>>  \n>>  \t\t# extended diff header\n[...]\n> First, programming style. You named the variable $empty, when it\n> evaluates to true when the patch is _not_ empty, i.e. when some\n> differences are found.\n\nBy the way, I _think_ that you can use $patch_idx instead of creating\nnew variable for that.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"38066","messageId":"20070326210548.GD1128@auto.tuwien.ac.at","threadId":"7394","inReplyTo":"200703261912.27612.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Add treediff","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-26T21:05:48Z","receivedAt":"2007-03-26T21:05:48Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"On Mon, Mar 26, 2007 at 06:12:27PM +0100, Jakub Narebski wrote:\n> On Sun, Mar 25, 2007, Martin Koegler wrote:\n> > treediff supports comparing different trees. A tree can be specified\n> > either as hash or as base hash and filename.\n> \n> I'd use \"treediff\" view, or git_treediff subroutine. Just a minor nit.\n>  \n> > +sub git_treediff {\n> > +\tmy $format = shift || 'html';\n> > +\tmy $from = $file_parent || \"\";\n> > +\tmy $to = $file_name || \"\";\n> \n> I'd use  $file_name || '';  here, and of course\n> \n> +\tmy $from = $file_parent || $file_name || '';\n> \n> The unwritten rule is that we use 'fp' parameter (thet $file_parent\n> variable is set) only \n\nHow do I specifiy the root tree (=tree in commit) with hpb/fp, as fp\ncan not be empty, but only undefined?\n\n> > +\n> > +\tif (!defined $hash) {\n> > +\t\tif (!defined $hash_base) {\n> > +\t\t\tdie_error('','tree parameter missing');\n> \n> This conflicts with the coding style used elsewhere in the gitweb\n> (the informal coding convention / guideline for gitweb).\n> \n> First, we either use  undef  and not  ''  to say: use default value\n> of the first parameter (HTTP status) of die_error, or provide our\n> own value in single quotes.\n> \n> Second, the second parameter, error message, is a sentence (without\n> final fullstop) describing error; it means starting it with capital\n> letter. And we use double quotes for this parameter.\n> \n> Examples:\n> \tdie_error(undef, \"Couldn't find base commit\");\n> \tdie_error(undef, \"Unknown commit object\");\n> \tdie_error(undef, \"No file name defined\");\n> \tdie_error(undef, \"Open git-diff-tree failed\");\n> \n> \tdie_error('403 Permission denied', \"Permission denied\");\n> \tdie_error('404 Not Found', \"File name not defined\");\n> \tdie_error('404 Not Found', \"Not enough information to find object\");\n> \tdie_error('400 Bad Request', \"Object is not a blob\");\n\nI didn't know this. I'll change this.\n\n> > +\t\t}\n> > +\t\t$hash = $hash_base;\n> > +\t\t$hash .= \":\".$file_name if (defined $file_name);\n> > +\t}\n> > +\n> > +\tif (!defined $hash_parent) {\n> > +\t\tif (!defined $hash_parent_base) {\n> > +\t\t\tdie_error('','tree parameter missing');\n> > +\t\t}\n> > +\t\t$hash_parent = $hash_parent_base;\n> > +\t\t$hash_parent .= \":\".$file_parent if (defined $file_parent);\n> \n> The same problem as above: we do not set 'fp' parameter (and $file_parent\n> variable) if name does not change. So you should use instead:\n> \n> +\t\t$hash_parent .= \":\".($file_parent || $file_name)\n> +\t\t\tif (defined $file_parent || defined $file_name);\n> \n\nSame as above: How to specifiy the root tree with hpb/fp?\n\n> > +\t\tif (defined $hash_parent_base && (my %co = parse_commit($hash_parent_base))) {\n> > +\t \t\t$formats_nav .= \" | from: \".\n> > +\t\t\t\t$cgi->a({-href => href(action=>\"commit\",\n> > +\t\t\t        \t               hash=>$hash_parent_base)},\n> > +\t\t\t\t        \"commit\")\n> > +\t\t\t\t.\" | \".\n> > +\t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n> > +\t\t\t\t                       hash=>$hash_parent_base)},\n> > +\t\t\t\t        \"commitdiff\")\n> > +\t\t\t\t.\" | \".\n> > +\t\t\t\t$cgi->a({-href => href(action=>\"tree\",\n> > +\t\t\t\t\t\t       hash=>$co{'tree'},\n> > +\t\t\t\t                       hash_base=>$hash_parent_base)},\n> > +\t\t\t\t        \"tree\");\n> > +\t\t}\n> > +\t}\n> \n> This depends if the similar change (feature) for \"blobdiff\" view\n> (in git_blobdiff subroutine) is accepted. Perhaps this could be\n> separated into separate patch?\n\nI'll do that.\n\n> > +\n> > +\t# read treediff\n> > +\tmy $fd;\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\", \"--patch-with-raw\", \"--full-index\",\n> > +\t\t\t$hash_parent, $hash, \"--\"\n> > +\t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n> > +\n> > +\t\twhile (my $line = <$fd>) {\n> > +\t\t\tchomp $line;\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}\n> \n> This is also common with git_commitdiff. Should be it put into separate\n> subroutine? Or do this refactoring in another patch?\n\nI would like to save the refactoring for later. Maybe we will need a\nchange, which will not work for git_commitdiff.\n\n> > +\n> > +\t} elsif ($format eq 'plain') {\n> > +\t\tmy $filename = basename($project) . \"-diff.patch\";\n> > +\n> \n> In \"commitdiff_plain\" view we use\n> \n> \t\tmy $filename = basename($project) . \"-$hash.patch\";\n> \n> Perhaps we should use the same: \"-diff.patch\" does not make much sense.\n> Is it a typo?\n\nNo. What unique name do you propose? It needs to include $hash and $hase_parent.\nIn this case, $hash could be $hash_base:$file_name, I'm not sure, how to escape \n$file_name.\n\nmfg Martin Kögler\n"},{"id":"38099","messageId":"200703270256.24295.jnareb@gmail.com","threadId":"7394","inReplyTo":"20070326204116.GB1128@auto.tuwien.ac.at","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-27T00:56:24Z","receivedAt":"2007-03-27T00:56:24Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Martin Koegler wrote:\n> On Mon, Mar 26, 2007 at 06:12:09PM +0100, Jakub Narebski wrote:\n\n>> It is done by adding $file_parent ('fp') to the path limiter, meaning\n>> that diff command becomes:\n>> \n>> \tgit diff-tree [options] hpb hb -- fp f\n>> \n>> instead of finding hash of a blob using git_get_hash_by_path subroutine\n>> or using extended SHA-1 syntax:\n>> \n>> \tgit diff [options] hpb:fp hp:f\n>> \n> \n> As far as I tested, this only works for renames, not\n> for comparing different objects, eg:\n> \n> git-diff -r -p 08727ea8bba8c81678e5cf15124ada23ad097bc3:grep.h bb95e19c5f1e470d2efe1c0e4e04c291019e4b25:refs.h\n> shows differences\n> \n> git-diff-tree -r 08727ea8bba8c81678e5cf15124ada23ad097bc3 bb95e19c5f1e470d2efe1c0e4e04c291019e4b25 -- grep.h refs.h\n> shows no difference\n> \n> As I want gitweb to be able to even do such compares (not just renames),\n> I'll still need a solution for this.\n\nWell, I haven't thought about it. Still, it is two lines changed only,\nand it fixes a bug in \"blobdiff\" view for rename diffs.\n \nPerhaps mine patch should go to 'maint', and yours to 'master' or 'next'\nbranch.\n\n> My idea is, that if I got hb:f and hpb:fp, the user exactly specified\n> the blobs to be compared. Then I don't want any guessing logic.\n\nI'd rather you reuse the \"no hash_parent\" code, which also hand-crafts\ndiffinfo. Perhaps removing \"git-diff-tree hpb hb -- f\" code entirely.\nBesides, code dealing with \"blobdiff\" coming from \"commit\", \"commitdiff\"\nand \"history\" views are tested to work as expected, not so with\narbitrary diffs.\n\nBy the way, if you call git_get_hash_by_path (which is expensive, as it\ncalls git command), you can use resulting hash in place of\nhash_base:filename as an argument to git-diff.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"38100","messageId":"200703270307.26589.jnareb@gmail.com","threadId":"7394","inReplyTo":"20070326205508.GC1128@auto.tuwien.ac.at","subject":"Re: [PATCH] gitweb: support filename prefix in git_patchset_body","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-27T01:07:26Z","receivedAt":"2007-03-27T01:07:26Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Martin Koegler wrote:\n> On Mon, Mar 26, 2007 at 06:12:18PM +0100, Jakub Narebski wrote:\n>> On Sun, Mar 25, 2007, Martin Koegler wrote:\n>>\n>>> git_treediff supports comparing subdirectories. As the output of\n>>> git-difftree is missing the path to the compared directories,\n>>> the links in the output would be wrong.\n>>> \n>>> The patch adds two new parameters to add the missing path prefix.\n>> \n>> Wouldn't it be better to concatenate the two \"path prefix\" patches\n>> together? They are about the same thing.\n> \n> I thought, each patch would be more readable, I split them in logical\n> separate units. Anyway, I'll combine them.\n\nThat was just a thought. If you think that separate patches would be\nmore readable, by all meens keep them splitted.\n\n>>>  sub git_patchset_body {\n>>> -\tmy ($fd, $difftree, $hash, $hash_parent) = @_;\n>>> +\tmy ($fd, $difftree, $hash, $hash_parent, $file_name, $file_parent) = @_;\n>>>  \n>>>  \tmy $patch_idx = 0;\n>>>  \tmy $patch_line;\n>> \n>> I'd rather use $from_prefix, $to_prefix here, or $basedif_name,\n>> $basedir_parent, or $dir_name, $dir_parent (my preference is to\n>> $from_prefix, $to_prefix variables).\n> \n> I'll switch to $to_prefix and $from_prefix.\n> \n>> +\t$from_prefix = !defined $from_prefix ? '' : $from_prefix.'/';\n>> +\t$to_prefix   = !defined $to_prefix   ? '' : $to_prefix . '/';\n>> +\t$to_prefix ||= $from_prefix;  # to allow to pass common prefix once\n> \n> OK. But is not the 3 line useless, as $to_prefix is alway defined\n> after the second line and you probable want $from_prefix ||=\n> $to_prefix. This will cause problems, as I currently pass the root\n> tree (=tree in commit object) as an missing file name parameter, as\n> gitweb does not allow an empty file name.\n\nThird line is not important, as it is you who control the calling\nconvention. Perhaps it should read:\n\n+\t$to_prefix = $from_prefix  if (!defined $to_prefix);\n\nAnd it would be fairly easy to change gitweb to allow empty file name\nparameters; simply change\n\n\t!validate_pathname($file_name)\n\nto\n\n\t!defined validate_pathname($file_name)\n\n(and similarly for $file_parent).\n\nBut I'd rather not change _CGI parameter_ (URI) convention that we set\n'fp' (file parent) parameter *only* if it is different from 'f' (file\nname). Otherwise we would introduce backwards incompatibile change,\nwith respect to bookmarks and old URI-s. Cool URI-s don't change...\n\n\nBTW. \"git rev-parse <tree-ish>:\" == \"git rev-parse <tree-ish>^{tree}\"\n\n> With an propagation logic, comparing a root tree with an sub tree will only\n> work in one direction.\n> \n> So I prefer to do not implement any automatic propagation between the\n> two prefixes. \n\nFine by me. It is just _internal_ call convention.\n \n>> or something like that, or just modify $from{'file'} and $to{'file'}\n>> \n>> \t$from{'file'} = (!defined $from_prefix ? '' : $from_prefix.'/') . $from{'file'};\n>> \t$to{'file'}   = (!defined $to_prefix   ? '' : $to_prefix . '/') . $to{'file'};\n>> \n>> just after setting $from{'file'} and $to{'file'}, although the second\n>> solution would additionally add prefix to the shown patch body itself.\n> \n> Modifing the paths before generating the links is a good idea. I'll\n> look, where its useful.\n\nPlease examine consequences of this, and changes in the output if you\ndecide to go this way (IMHO bit simpler).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"38101","messageId":"200703270315.54901.jnareb@gmail.com","threadId":"7394","inReplyTo":"20070326210548.GD1128@auto.tuwien.ac.at","subject":"Re: [PATCH] gitweb: Add treediff","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-27T01:15:54Z","receivedAt":"2007-03-27T01:15:54Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Martin Koegler wrote:\n> On Mon, Mar 26, 2007 at 06:12:27PM +0100, Jakub Narebski wrote:\n>> On Sun, Mar 25, 2007, Martin Koegler wrote:\n\n>>> +sub git_treediff {\n>>> +\tmy $format = shift || 'html';\n>>> +\tmy $from = $file_parent || \"\";\n>>> +\tmy $to = $file_name || \"\";\n>> \n>> I'd use  $file_name || '';  here, and of course\n>> \n>> +\tmy $from = $file_parent || $file_name || '';\n>> \n>> The unwritten rule is that we use 'fp' parameter (thet $file_parent\n>> variable is set) only \n> \n> How do I specifiy the root tree (=tree in commit) with hpb/fp, as fp\n> can not be empty, but only undefined?\n\nAs I said in previous message, we can simply relax check for 'f' and 'fp'\nparameters, by changing\n\n\t!validate_pathname($file_name)\n\nto\n\n\t!defined validate_pathname($file_name)\n \nStill, there are some places where we assume that 'f' and 'fp' cannot be\nempty, like in above proposal. It would be:\n\n+\tmy $from = (defined $file_parent ? $file_parent : $file_name) || '';\n\n\nAgain, I don't want to loose the assumption that we generate 'fp' _only_\nif it is different from 'f'. Otherwise old links would cease working,\nwhich breaks backwards-compatibility, and is not cool. \"Cool UR_is don't\nchange.\"\n\n>>> +\n>>> +\tif (!defined $hash) {\n>>> +\t\tif (!defined $hash_base) {\n>>> +\t\t\tdie_error('','tree parameter missing');\n>> \n>> This conflicts with the coding style used elsewhere in the gitweb\n[...]\n>> Examples:\n>> \tdie_error(undef, \"Couldn't find base commit\");\n[...]\n> I didn't know this. I'll change this.\n\nWhat's strange in other place you used die_error accoring to coding\nguidelines.\n\n>>> +\n>>> +\t} elsif ($format eq 'plain') {\n>>> +\t\tmy $filename = basename($project) . \"-diff.patch\";\n>>> +\n>> \n>> In \"commitdiff_plain\" view we use\n>> \n>> \t\tmy $filename = basename($project) . \"-$hash.patch\";\n>> \n>> Perhaps we should use the same: \"-diff.patch\" does not make much sense.\n>> Is it a typo?\n> \n> No. What unique name do you propose? It needs to include $hash and $hase_parent.\n> In this case, $hash could be $hash_base:$file_name, I'm not sure, how to escape \n> $file_name.\n\nFor \"commitdiff\" case sole $hash is also not unique. But if not -hash,\nthen perhaps -treediff instead of simply -diff?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"38180","messageId":"20070327195627.GA23205@auto.tuwien.ac.at","threadId":"7394","inReplyTo":"200703270256.24295.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-27T19:56:27Z","receivedAt":"2007-03-27T19:56:27Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"On Tue, Mar 27, 2007 at 01:56:24AM +0100, Jakub Narebski wrote:\n> Martin Koegler wrote:\n> > My idea is, that if I got hb:f and hpb:fp, the user exactly specified\n> > the blobs to be compared. Then I don't want any guessing logic.\n> \n> I'd rather you reuse the \"no hash_parent\" code, which also hand-crafts\n> diffinfo. Perhaps removing \"git-diff-tree hpb hb -- f\" code entirely.\n> Besides, code dealing with \"blobdiff\" coming from \"commit\", \"commitdiff\"\n> and \"history\" views are tested to work as expected, not so with\n> arbitrary diffs.\n\nI don't like the whole rename detection code, so I offer to simplify\ngit_blobdiff. For all calls to git_blobdiff (except those from git_history),\nI'm sure, that I can assume $file_parent ||= $file_name.\n\nIf you think, its safe, I can simplify git_blobdiff. I propose\ndoing the following way (pseudo-code):\n\nsub git_blobdiff {\n$file_parent ||= $file_name;\n\nif (defined $hash_base && defined $file_name && !defined $hash)\n$hash=git_get_hash_by_path($hash_base,$file_name);\n\nif (defined $hash_parent_base && defined $file_parent && !defined $hash_parent)\n$hash_parent=git_get_hash_by_path($hash_parent_base,$file_parent);\n\nif (!defined $hash || ! defined $hash_parent )\ndie_error(....);\n\n$diffinfo{'from_mode'} = $diffinfo{'to_mode'} = \"blob\";\n$diffinfo{'from_file'} = $file_parent || $hash_parent;\n$diffinfo{'to_file'}   = $file_name || $hash;\n$diffinfo{'status'} = '2';\n$diffinfo{'to_id'}   = $hash;\n$diffinfo{'from_id'}   = $hash_parent;\n\n open $fd, \"-|\", git_cmd(), \"diff\", '-p', @diff_opts,\n    $hash_parent, $hash\n    or die_error(undef, \"Open git-diff failed\");\n\n/* print output */\n\n}\n\nElse I will keep a reworked version of my patch.\n\n> By the way, if you call git_get_hash_by_path (which is expensive, as it\n> calls git command), you can use resulting hash in place of\n> hash_base:filename as an argument to git-diff.\n \nI must check, if we need to resolve $hash ($hash_parent) by\ngit_get_hash_by_path, if we construct it out of $hash_base and\n$file_name. Maybe we can avoid this call.\n\nmfg Martin Kögler\n \n"},{"id":"38206","messageId":"200703280158.54929.jnareb@gmail.com","threadId":"7394","inReplyTo":"20070327195627.GA23205@auto.tuwien.ac.at","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-27T23:58:54Z","receivedAt":"2007-03-27T23:58:54Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, Mar 27, 2007, Martin Koegler wrote:\n> On Tue, Mar 27, 2007 at 01:56:24AM +0100, Jakub Narebski wrote:\n>> Martin Koegler wrote:\n>>> My idea is, that if I got hb:f and hpb:fp, the user exactly specified\n>>> the blobs to be compared. Then I don't want any guessing logic.\n>> \n>> I'd rather you reuse the \"no hash_parent\" code, which also hand-crafts\n>> diffinfo. Perhaps removing \"git-diff-tree hpb hb -- f\" code entirely.\n>> Besides, code dealing with \"blobdiff\" coming from \"commit\", \"commitdiff\"\n>> and \"history\" views are tested to work as expected, not so with\n>> arbitrary diffs.\n> \n> I don't like the whole rename detection code, so I offer to simplify\n> git_blobdiff. For all calls to git_blobdiff (except those from git_history),\n> I'm sure, that I can assume $file_parent ||= $file_name.\n\nThat was the idea. Perhaps I haven't said it clearly, but I wanted to\nsuggest to remove the whole git-diff-tree code, and use git-diff to\ngenerate diff between blobs.\n\n> If you think, its safe, I can simplify git_blobdiff. I propose\n> doing the following way (pseudo-code):\n\n> $file_parent ||= $file_name;\n[...]\n> $hash=git_get_hash_by_path($hash_base,$file_name);\n[...]\n> $hash_parent=git_get_hash_by_path($hash_parent_base,$file_parent);\n[...]\n>  open $fd, \"-|\", git_cmd(), \"diff\", '-p', @diff_opts,\n>     $hash_parent, $hash\n>     or die_error(undef, \"Open git-diff failed\");\n[...]\n> Else I will keep a reworked version of my patch.\n\nThe trouble with this is that we may lose mode change (symlink to\nordinary file etc.) because we hand-generate %diffinfo.\n\n>> By the way, if you call git_get_hash_by_path (which is expensive, as it\n>> calls git command), you can use resulting hash in place of\n>> hash_base:filename as an argument to git-diff.\n>  \n> I must check, if we need to resolve $hash ($hash_parent) by\n> git_get_hash_by_path, if we construct it out of $hash_base and\n> $file_name. Maybe we can avoid this call.\n\nWe can use \"$hash_base:$file_name\" as second parameter to git-diff etc.,\nbut I don't think we want to create links with \"$hash_base:$file_name\"\ninstead of sha-1 id of a blob as 'h' parameter.\n\nIt can be first implementation, thought, and later we can try to use\n\"index <hash>..<hash> <mode>\" lines from extended header to get $hash\nand $hash_parent (with exception of pure rename, but then we need only\none invocation of git_get_hash_by_path subroutine).\n\nBut I think it is better left for later patch.\n-- \nJakub Narebski\nPoland\n"},{"id":"38265","messageId":"20070328210301.GA57@auto.tuwien.ac.at","threadId":"7394","inReplyTo":"200703280158.54929.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-28T21:03:01Z","receivedAt":"2007-03-28T21:03:01Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"On Wed, Mar 28, 2007 at 12:58:54AM +0100, Jakub Narebski wrote:\n> On Thu, Mar 27, 2007, Martin Koegler wrote:\n> > On Tue, Mar 27, 2007 at 01:56:24AM +0100, Jakub Narebski wrote:\n> > If you think, its safe, I can simplify git_blobdiff. I propose\n> > doing the following way (pseudo-code):\n> \n> > $file_parent ||= $file_name;\n> [...]\n> > $hash=git_get_hash_by_path($hash_base,$file_name);\n> [...]\n> > $hash_parent=git_get_hash_by_path($hash_parent_base,$file_parent);\n> [...]\n> >  open $fd, \"-|\", git_cmd(), \"diff\", '-p', @diff_opts,\n> >     $hash_parent, $hash\n> >     or die_error(undef, \"Open git-diff failed\");\n> [...]\n> > Else I will keep a reworked version of my patch.\n> \n> The trouble with this is that we may lose mode change (symlink to\n> ordinary file etc.) because we hand-generate %diffinfo.\n\nIf we need the correct mode in %diffinfo, this is not difficult:\n\n$from_mode=\"blob\";\nif (defined $hash_base && defined $file_name)\n($to_mode,$hash)=git_get_hash_by_path($hash_base,$file_name);\n\n$to_mode=\"blob\";\nif (defined $hash_parent_base && defined $file_parent)\n($from_mode,$hash_parent)=git_get_hash_by_path_mode($hash_parent_base,$file_parent);\n\nThen we set to_mode and from_mode in %diffinfo to $to_mode and $from_mode.\n\ngit_get_hash_by_path_mode is like git_get_hash_by_path, only that it\nreturns an array with the mode and the hash.\n\nBlobdiff (html output) in its current version can not handle symlinks:\n> diff --git a/x b/x\n> deleted file mode 100644 (file)\n> index 190a180..873fb8d\n> --- a/x\n> +++ /dev/null\n> @@ -1 +0,0 @@\n> -123\n> diff --git a/ b/\n> new file mode 120000 (symlink)\n> index 190a180..873fb8d\n> --- /dev/null\n> +++ b/\n> @@ -0,0 +1 @@\n> +file3\n> \\ No newline at end of file\n\nThis was generated by \"diff to current\" in the history view of a file,\nwhich was changed between symlink and normal file.\n \nAdditionally, to_mode and from_mode of %diffinfo seem to be ignored by\ngit_patchset_body.\n\n> >> By the way, if you call git_get_hash_by_path (which is expensive, as it\n> >> calls git command), you can use resulting hash in place of\n> >> hash_base:filename as an argument to git-diff.\n> >  \n> > I must check, if we need to resolve $hash ($hash_parent) by\n> > git_get_hash_by_path, if we construct it out of $hash_base and\n> > $file_name. Maybe we can avoid this call.\n> \n> We can use \"$hash_base:$file_name\" as second parameter to git-diff etc.,\n> but I don't think we want to create links with \"$hash_base:$file_name\"\n> instead of sha-1 id of a blob as 'h' parameter.\n> \n> It can be first implementation, thought, and later we can try to use\n> \"index <hash>..<hash> <mode>\" lines from extended header to get $hash\n> and $hash_parent (with exception of pure rename, but then we need only\n> one invocation of git_get_hash_by_path subroutine).\n\nThe <mode> must be discarded, as it is wrong for anything which as not\na mode of 100644, as we specify a two blob as parameter to git-diff.\n\n> But I think it is better left for later patch.\n\nAs git_patchset_body requires an information about the compared file\nas parameter, a new formating function will be needed. I'm not sure,\nif the overhead of git_get_hash_by_path is this worth. Additionally,\nif we have the hash passed by parameter in most cases, there is no\nneed to call it in these cases.\n\nmfg Martin Kögler\nPS:\nI created a blob with a \"strange\" filename: &()=!\"§$%[<>]*#+_;.\nIn the result of the blob view, the \" is not escaped in the filename in the header\nand a strage content type is returned:\n\n$ telnet localhost 80\nTrying 127.0.0.1...\nConnected to localhost.\nEscape character is '^]'.\nGET /gitweb/gitweb.cgi?p=t/.git;a=blob;f=%26()%3D%21%22%A7%24%25%5B%3C%3E%5D%2A%23%2B_%3B.;hb=7bfed2588bee66b33db544830606fa6606478fd9 HTTP/1.0\n\nHTTP/1.1 200 OK\nDate: Wed, 28 Mar 2007 19:55:36 GMT\nServer: Apache\nContent-disposition: inline; filename=\"&()=!\"§$%[<>]*#+_;.\"\nExpires: Thu, 29 Mar 2007 19:55:39 GMT\nConnection: close\nContent-Type: application/vnd.mif\n\nxx\n"},{"id":"38379","messageId":"200703301048.27636.jnareb@gmail.com","threadId":"7394","inReplyTo":"20070328210301.GA57@auto.tuwien.ac.at","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-30T08:48:26Z","receivedAt":"2007-03-30T08:48:26Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, Mar 28, 2007 at 23:03 +0200, Martin Koegler wrote:\n> On Wed, Mar 28, 2007 at 12:58:54AM +0100, Jakub Narebski wrote:\n>> On Thu, Mar 27, 2007, Martin Koegler wrote:\n>>> On Tue, Mar 27, 2007 at 01:56:24AM +0100, Jakub Narebski wrote:\n\n>>> If you think, its safe, I can simplify git_blobdiff. I propose\n>>> doing the following way (pseudo-code):\n>> \n>>> $file_parent ||= $file_name;\n>> [...]\n>>> $hash=git_get_hash_by_path($hash_base,$file_name);\n>> [...]\n>>> $hash_parent=git_get_hash_by_path($hash_parent_base,$file_parent);\n>> [...]\n>>>  open $fd, \"-|\", git_cmd(), \"diff\", '-p', @diff_opts,\n>>>     $hash_parent, $hash\n>>>     or die_error(undef, \"Open git-diff failed\");\n>> [...]\n>>> Else I will keep a reworked version of my patch.\n>> \n>> The trouble with this is that we may lose mode change (symlink to\n>> ordinary file etc.) because we hand-generate %diffinfo.\n> \n> If we need the correct mode in %diffinfo, this is not difficult:\n> \n> $from_mode=\"blob\";\n> if (defined $hash_base && defined $file_name)\n> ($to_mode,$hash)=git_get_hash_by_path($hash_base,$file_name);\n[...]\n> Then we set to_mode and from_mode in %diffinfo to $to_mode and $from_mode.\n> \n> git_get_hash_by_path_mode is like git_get_hash_by_path, only that it\n> returns an array with the mode and the hash.\n\nI'd rather either have git_get_ls_tree_line_by_path, which we would then\nparse using parse_ls_tree_line, or have git_get_info_by_path which would\nreturn already parsed information (as hash reference, or as a list).\n\n>>>> By the way, if you call git_get_hash_by_path (which is expensive, as it\n>>>> calls git command), you can use resulting hash in place of\n>>>> hash_base:filename as an argument to git-diff.\n>>>  \n>>> I must check, if we need to resolve $hash ($hash_parent) by\n>>> git_get_hash_by_path, if we construct it out of $hash_base and\n>>> $file_name. Maybe we can avoid this call.\n>> \n>> We can use \"$hash_base:$file_name\" as second parameter to git-diff etc.,\n>> but I don't think we want to create links with \"$hash_base:$file_name\"\n>> instead of SHA-1 id (hash) of a blob as 'h' parameter.\n>> \n>> It can be first implementation, thought, and later we can try to use\n>> \"index <hash>..<hash> <mode>\" lines from extended header to get $hash\n>> and $hash_parent (with exception of pure rename, but then we need only\n>> one invocation of git_get_hash_by_path subroutine).\n> \n> The <mode> must be discarded, as it is wrong for anything which as not\n> a mode of 100644, as we specify a two blob as parameter to git-diff.\n\nThe <mode> information might be discarded, or might be saved. \"blob\"\ncan have 100644 mode, can have 100755 mode (executable file), or can\nhave 120000 mode (symbolic link).\n \n>> But I think it is better left for later patch.\n> \n> As git_patchset_body requires an information about the compared file\n> as parameter, a new formating function will be needed. I'm not sure,\n> if the overhead of git_get_hash_by_path is this worth. Additionally,\n> if we have the hash passed by parameter in most cases, there is no\n> need to call it in these cases.\n\ngit_patchset_body currently buffers (caches) diff header (up to \nfrom-file / to-file header) to catch situation where one \"raw\" format\nline (one difftree line) correspond to two patches (like e.g. type change\nsituation below, from ordinary file to symlink or vice versa, and which\n_should_ be handled by git_patchset_body). So I think it would not be\nhard to parse extended diff header and fill values which are not present\nin %diffinfo from extended diff header. This includes filling $from_hash\nand $to_hash ($hash_parent and $hash) information from the \n\"index <hash>..<hash> <mode>\" or \"index <hash>..<hash>\" extended diff\nheader line.\n\nThe (only?) rare exception is when files (blobs) does _not_ differ, as\npatch in this situation is empty, and we would have to get hash of blob\n(which would be the same for from and to, for $hash_parent and $hash)\nusing git_get_hash_by_name or new git_get_info_by_name.\n\n> Blobdiff (html output) in its current version can not handle symlinks:\n\nI'd investigate that. Is \"commitdiff\" view correct in this situation?\n\n>> diff --git a/x b/x\n>> deleted file mode 100644 (file)\n>> index 190a180..873fb8d\n>> --- a/x\n>> +++ /dev/null\n>> @@ -1 +0,0 @@\n>> -123\n>> diff --git a/ b/\n>> new file mode 120000 (symlink)\n>> index 190a180..873fb8d\n>> --- /dev/null\n>> +++ b/\n>> @@ -0,0 +1 @@\n>> +file3\n>> \\ No newline at end of file\n> \n> This was generated by \"diff to current\" in the history view of a file,\n> which was changed between symlink and normal file.\n\n\n> Additionally, to_mode and from_mode of %diffinfo seem to be ignored by\n> git_patchset_body.\n\nAs it should, I think.\n\n> mfg Martin Kögler\n> PS:\n> I created a blob with a \"strange\" filename: &()=!\"§$%[<>]*#+_;.\n> In the result of the blob view, the \" is not escaped in the filename in the header\n> and a strange content type is returned:\n> \n> $ telnet localhost 80\n> Trying 127.0.0.1...\n> Connected to localhost.\n> Escape character is '^]'.\n> GET /gitweb/gitweb.cgi?p=t/.git;a=blob;f=%26()%3D%21%22%A7%24%25%5B%3C%3E%5D%2A%23%2B_%3B.;hb=7bfed2588bee66b33db544830606fa6606478fd9 HTTP/1.0\n> \n> HTTP/1.1 200 OK\n> Date: Wed, 28 Mar 2007 19:55:36 GMT\n> Server: Apache\n> Content-disposition: inline; filename=\"&()=!\"§$%[<>]*#+_;.\"\n> Expires: Thu, 29 Mar 2007 19:55:39 GMT\n> Connection: close\n> Content-Type: application/vnd.mif\n> \n> xx\n\nI'd try to check that\n\n-- \nJakub Narebski\nPoland\n"},{"id":"38389","messageId":"200703310155.14452.jnareb@gmail.com","threadId":"7394","inReplyTo":"20070328210301.GA57@auto.tuwien.ac.at","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-30T23:55:14Z","receivedAt":"2007-03-30T23:55:14Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, Mar 28, 2007, Martin Koegler wrote:\n\n> I created a blob with a \"strange\" filename: &()=!\"§$%[<>]*#+_;.\n> In the result of the blob view, the \" is not escaped in the filename in the header\n> and a strage content type is returned:\n> \n> $ telnet localhost 80\n> Trying 127.0.0.1...\n> Connected to localhost.\n> Escape character is '^]'.\n> GET /gitweb/gitweb.cgi?p=t/.git;a=blob;f=%26()%3D%21%22%A7%24%25%5B%3C%3E%5D%2A%23%2B_%3B.;hb=7bfed2588bee66b33db544830606fa6606478fd9 HTTP/1.0\n> \n> HTTP/1.1 200 OK\n> Date: Wed, 28 Mar 2007 19:55:36 GMT\n> Server: Apache\n> Content-disposition: inline; filename=\"&()=!\"§$%[<>]*#+_;.\"\n> Expires: Thu, 29 Mar 2007 19:55:39 GMT\n> Connection: close\n> Content-Type: application/vnd.mif\n> \n> xx\n\nThere are two separate things. \n\nFirst is not escaped filename in HTTP header. There was some discussion\nabout this, and even patch by Luben Tuikov which added to_qtext \nsubroutine to deal with escaping in HTTP (which has diferent rules than\nescaping in HTML, or in HTML attributes)\n * gitweb: using quotemeta\n   http://thread.gmane.org/gmane.comp.version-control.git/28050/\n * [PATCH] gitweb: Convert Content-Disposition filenames into qtext\n   http://thread.gmane.org/gmane.comp.version-control.git/28437\nBut the patch was newer accepted; either lost in the noise, or in lack\nof summary to the discussion.\n\nSecond is detecting file as binary (for text files we always return\nHTML), and content type set. Gitweb checks mimetype of a file (based\non extension, mainly) by default using /etc/mime.magic, and if it is\nneither text/* nor HTML images (png, jpg, gif) it uses \"blobdiff_plain\"\nview. Perhaps instead of just calling git_blob_plain($mimetype) we\nshould do a redirect (but then how to pass detected mimetype?). It looks\nlike there is an error with mimetype detection: could you tell me the\nline with \"application/vnd.mif\" in your /etc/mime.types?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"38419","messageId":"20070331091806.GA12442@auto.tuwien.ac.at","threadId":"7394","inReplyTo":"200703310155.14452.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2007-03-31T09:18:06Z","receivedAt":"2007-03-31T09:18:06Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"On Sat, Mar 31, 2007 at 01:55:14AM +0200, Jakub Narebski wrote:\n> Second is detecting file as binary (for text files we always return\n> HTML), and content type set. Gitweb checks mimetype of a file (based\n> on extension, mainly) by default using /etc/mime.magic, and if it is\n> neither text/* nor HTML images (png, jpg, gif) it uses \"blobdiff_plain\"\n> view. Perhaps instead of just calling git_blob_plain($mimetype) we\n> should do a redirect (but then how to pass detected mimetype?). It looks\n> like there is an error with mimetype detection: could you tell me the\n> line with \"application/vnd.mif\" in your /etc/mime.types?\n\nThe system is a Debian system. /etc/mime.types is part of mime-support:\n$dpkg -l mime-support\nDesired=Unknown/Install/Remove/Purge/Hold\n| Status=Not/Installed/Config-files/Unpacked/Failed-config/Half-installed\n|/ Err?=(none)/Hold/Reinst-required/X=both-problems (Status,Err: uppercase=bad)\n||/ Name                         Version                      Description\n+++-============================-============================-========================================================================\nii  mime-support                 3.28-1                       MIME files 'mime.types' & 'mailcap', and support programs\n\n$ grep vnd.mif /etc/mime.types\napplication/vnd.mif           mif\n\nAs it's a config file, this line could be added by an other package.\n\nThe context of this entry is:\napplication/vnd.koan\napplication/vnd.lotus-1-2-3\napplication/vnd.lotus-approach\napplication/vnd.lotus-freelance\napplication/vnd.lotus-notes\napplication/vnd.lotus-organizer\napplication/vnd.lotus-screencam\napplication/vnd.lotus-wordpro\napplication/vnd.mcd\napplication/vnd.mediastation.cdkey\napplication/vnd.meridian-slingshot\napplication/vnd.mif           mif\napplication/vnd.minisoft-hp3000-save\napplication/vnd.mitsubishi.misty-guard.trustweb\napplication/vnd.mobius.daf\napplication/vnd.mobius.dis\napplication/vnd.mobius.msl\napplication/vnd.mobius.plc\napplication/vnd.mobius.txf\napplication/vnd.motorola.flexsuite\napplication/vnd.motorola.flexsuite.adsi\napplication/vnd.motorola.flexsuite.fis\napplication/vnd.motorola.flexsuite.gotap\n\nHope, it helps.\n\nmfg Martin Kögler\n"},{"id":"38452","messageId":"200703311652.33481.jnareb@gmail.com","threadId":"7394","inReplyTo":"20070328210301.GA57@auto.tuwien.ac.at","subject":"[PATCH] gitweb: Fix bug in \"blobdiff\" view for split (e.g. file to symlink) patches","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-31T14:52:33Z","receivedAt":"2007-03-31T14:52:33Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"git_patchset_body needs patch generated with --full-index option to\ndetect split patches, meaning two patches which corresponds to single\ndifftree (raw diff) entry.  An example of such situation is changing\ntype (mode) of a file, e.g. from plain file to symbolic link.\n\nAdd, in git_blobdiff, --full-index option to patch generating git diff\ninvocation, for the 'html' format output (\"blobdiff\" view).\n\n\"blobdiff_plain\" still uses shortened sha1 in the extended git diff\nheader \"index <hash>..<hash>[ <mode>]\" line.\n\ngit_patchset_body and git_commitdiff (but not git_blobdiff) were adjusted\nfor the split patches in commit 6d55f05576851aedc7c53f7438c1d505c22ee78f\n\nNoticed-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nOn Wed, Mar 28, 2007 at 23:03 +0200, Martin Koegler wrote:\n\n> Blobdiff (html output) in its current version can not handle symlinks:\n>\n> > diff --git a/x b/x\n> > deleted file mode 100644 (file)\n> > index 190a180..873fb8d\n> > --- a/x\n> > +++ /dev/null\n> > @@ -1 +0,0 @@\n> > -123\n> > diff --git a/ b/\n> > new file mode 120000 (symlink)\n> > index 190a180..873fb8d\n> > --- /dev/null\n> > +++ b/\n> > @@ -0,0 +1 @@\n> > +file3\n> > \\ No newline at end of file\n> \n> This was generated by \"diff to current\" in the history view of a file,\n> which was changed between symlink and normal file.\n\nThis patch fixes it.\n\n gitweb/gitweb.perl |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4f34c29..d1f4aeb 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3936,7 +3936,8 @@ sub git_blobdiff {\n \n \t\t# open patch output\n \t\topen $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n-\t\t\t'-p', $hash_parent_base, $hash_base,\n+\t\t\t'-p', ($format eq 'html' ? \"--full-index\" : ()),\n+\t\t\t$hash_parent_base, $hash_base,\n \t\t\t\"--\", (defined $file_parent ? $file_parent : ()), $file_name\n \t\t\tor die_error(undef, \"Open git-diff-tree failed\");\n \t}\n@@ -3971,7 +3972,8 @@ sub git_blobdiff {\n \t\t}\n \n \t\t# open patch output\n-\t\topen $fd, \"-|\", git_cmd(), \"diff\", '-p', @diff_opts,\n+\t\topen $fd, \"-|\", git_cmd(), \"diff\", @diff_opts,\n+\t\t\t'-p', ($format eq 'html' ? \"--full-index\" : ()),\n \t\t\t$hash_parent, $hash, \"--\"\n \t\t\tor die_error(undef, \"Open git-diff failed\");\n \t} else  {\n-- \n1.5.0.5\n"},{"id":"38451","messageId":"200703311816.05283.jnareb@gmail.com","threadId":"7394","inReplyTo":"200703310155.14452.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-03-31T16:16:04Z","receivedAt":"2007-03-31T16:16:04Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, Mar 31, 2007 at 01:55 +0200, Jakub Narebski wrote:\n> On Wed, Mar 28, 2007, Martin Koegler wrote:\n> \n>> I created a blob with a \"strange\" filename: &()=!\"§$%[<>]*#+_;.\n>> In the result of the blob view, the \" is not escaped in the filename in the header\n>> and a strange content type is returned:\n>> \n>> $ telnet localhost 80\n>> Trying 127.0.0.1...\n>> Connected to localhost.\n>> Escape character is '^]'.\n>> GET /gitweb/gitweb.cgi?p=t/.git;a=blob;f=%26()%3D%21%22%A7%24%25%5B%3C%3E%5D%2A%23%2B_%3B.;hb=7bfed2588bee66b33db544830606fa6606478fd9 HTTP/1.0\n>> \n>> HTTP/1.1 200 OK\n>> Date: Wed, 28 Mar 2007 19:55:36 GMT\n>> Server: Apache\n>> Content-disposition: inline; filename=\"&()=!\"§$%[<>]*#+_;.\"\n>> Expires: Thu, 29 Mar 2007 19:55:39 GMT\n>> Connection: close\n>> Content-Type: application/vnd.mif\n>> \n>> xx\n> \n> There are two separate things. \n> \n> First is not escaped filename in HTTP header. There was some discussion\n> about this, and even patch by Luben Tuikov which added to_qtext \n> subroutine to deal with escaping in HTTP (which has diferent rules than\n> escaping in HTML, or in HTML attributes)\n>  * gitweb: using quotemeta\n>    http://thread.gmane.org/gmane.comp.version-control.git/28050/\n>  * [PATCH] gitweb: Convert Content-Disposition filenames into qtext\n>    http://thread.gmane.org/gmane.comp.version-control.git/28437\n> But the patch was newer accepted; either lost in the noise, or in lack\n> of summary to the discussion.\n\nJunio, do you remember by chance why this patch was dropped?\n\n> Second is detecting file as binary (for text files we always return\n> HTML), and content type set. Gitweb checks mimetype of a file (based\n> on extension, mainly) by default using /etc/mime.magic, and if it is\n> neither text/* nor HTML images (png, jpg, gif) it uses \"blobdiff_plain\"\n> view. Perhaps instead of just calling git_blob_plain($mimetype) we\n> should do a redirect (but then how to pass detected mimetype?). It looks\n> like there is an error with mimetype detection: could you tell me the\n> line with \"application/vnd.mif\" in your /etc/mime.types?\n\n\nUnfortunately trying to find the source of this error failed (perhaps\ndue to name of file slightly changed by accident), and I get no errors.\nWORKSFORME aka SAA#1 (Standard Answer of Admin #1: Works for me).\n\nI use the following script to debug errors in gitweb:\n\n-- >8 ------------------------------------\n#!/bin/sh\n\nexport GATEWAY_INTERFACE=\"CGI/1.1\"\nexport HTTP_ACCEPT=\"*/*\"\nexport REQUEST_METHOD=\"GET\"\nexport QUERY_STRING=\"\"$1\"\"\nexport PATH_INFO=\"\"$2\"\"\n\nddd \"/var/www/cgi-bin/gitweb/gitweb.cgi\"\n-- >8 ------------------------------------\n\n-- \nJakub Narebski\nPoland\n"},{"id":"38505","messageId":"200704031657.25698.jnareb@gmail.com","threadId":"7394","inReplyTo":"7vmz1t6oe2.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-04-03T14:57:24Z","receivedAt":"2007-04-03T14:57:24Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, Apr 1, 2007, Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n>>> First is not escaped filename in HTTP header. There was some discussion\n>>> about this, and even patch by Luben Tuikov which added to_qtext \n>>> subroutine to deal with escaping in HTTP (which has diferent rules than\n>>> escaping in HTML, or in HTML attributes)\n>>>  * gitweb: using quotemeta\n>>>    http://thread.gmane.org/gmane.comp.version-control.git/28050/\n>>>  * [PATCH] gitweb: Convert Content-Disposition filenames into qtext\n>>>    http://thread.gmane.org/gmane.comp.version-control.git/28437\n>>> But the patch was newer accepted; either lost in the noise, or in lack\n>>> of summary to the discussion.\n>>\n>> Junio, do you remember by chance why this patch was dropped?\n> \n> No, but I suspect that was because the noisiness of the thread\n> around them suggested they were not ready to be applied.  I do\n> not remember if people submitted the patch and commented on\n> reached a consensus.\n\nProbably not. Here is alternative proposal. It does not implement\n  RFC2184: MIME Parameter Value and Encoded Word Extensions\nbut I'm not sure if 1) it is needed for _HTTP_ Content-Disposition\nheader filename, 2) all browsers implement it.\n\nBy the way, $str =~ s/[\\n\\r]/_/g; line (as per Junio Hamano and Petr\nBaudis suggestion) is needed not only for buggy browsers, but also for\nbuggy CGI implementation:\n\n  $ perl -wle \\\n  'use CGI; \\\n   our $cgi = new CGI; \\\n   print $cgi->header(-content_disposition => \"inline; filename=\\\"file\\nname\\\"\");'\n\ngenerates (for CGI version 3.10)\n\n  Content-disposition=\"inline; filename=&quot;file\n  name&quot;\"\n\nwhich is a bit strange. Single LF (not CRLF pair) does not need to be\nquoted in the header, as per RFC822.\n\n-- >8 --\n# Generate value of Content-Disposition header field, with \"inline\"\n# disposition type, for a given filename parameter\n# Usage: $cgi->header( [...],\n#          -content_disposition => content_disposition($filename))\n# References: RFC 2183, RFC 822 and RFC 2045\nsub content_disposition {\n\tmy $filename = shift;\n\n\t#RFC2183: The Content-Disposition Header Field\n\t# parameter value containing only non-`tspecials' characters [RFC 2045]\n\t# SHOULD be represented as a single `token'.\n\t#RFC2045: MIME Part One: Format of Internet Message Bodies\n\t# token := 1*<any (US-ASCII) CHAR except SPACE, CTLs,\n\t#             or tspecials>\n\tif ($filename =~ m/[[:space:][:cntrl:]()<>@,;:\\\\\"\\/\\[\\]?=]/) {\n\t\t#RFC2183: The Content-Disposition Header Field\n\t\t# parameter value containing only ASCII characters, but including\n\t\t# `tspecials' characters, SHOULD be represented as `quoted-string'.\n\n\t\t# It not worth potential problems to try to carry newlines (and such)\n\t\t# in the header; it is just _suggested_ filename\n\t\t$filename =~ s/[[:cntrl:]\\n\\r]/_/g;\n\n\t\t#RFC822: Standard for the Format of ARPA Internet Text Messages\n\t\t# quoting is REQUIRED for CR and \"\\\" and for the character(s) that\n\t\t# delimit the token (e.g., \"(\" and \")\" for a comment).  However,\n\t\t# quoting is PERMITTED for any character.\n\t\t$filename =~ s/([\\\\\"\\r])/\\\\$1/g;\n\t\t$filename = '\"' . $filename . '\"';\n\t}\n\treturn \"inline; filename=$filename\";\n}\n-- >8 --\nP.S. We could probably always quote filename parameter, even if it\nis not needed (\"SHOULD be represented as a single `token'\" part).\n\nP.P.S. Here is an example of RFC2184 encoded header:\n\n   Content-Type: application/x-stuff\n    title*1*=us-ascii'en'This%20is%20even%20more%20\n    title*2*=%2A%2A%2Afun%2A%2A%2A%20\n    title*3=\"isn't it!\"\n\n-- \nJakub Narebski\nPoland\n"},{"id":"38667","messageId":"200704042327.49632.jnareb@gmail.com","threadId":"7394","inReplyTo":"200704031657.25698.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-04-04T21:27:49Z","receivedAt":"2007-04-04T21:27:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, Apr 03, 2007 at 16:57 +0200, Jakub Narebski wrote:\n> On Sun, Apr 1, 2007, Junio C Hamano wrote:\n>> Jakub Narebski <jnareb@gmail.com> writes:\n>> \n>>>> First is not escaped filename in HTTP header. There was some discussion\n>>>> about this, and even patch by Luben Tuikov which added to_qtext \n>>>> subroutine to deal with escaping in HTTP [...]\n>>>\n>>> Junio, do you remember by chance why this patch was dropped?\n>> \n>> No, but I suspect that was because the noisiness of the thread\n>> around them suggested they were not ready to be applied.  I do\n>> not remember if people submitted the patch and commented on\n>> reached a consensus.\n> \n> Probably not. Here is alternative proposal. It does not implement\n>   RFC2184: MIME Parameter Value and Encoded Word Extensions\n> but I'm not sure if 1) it is needed for _HTTP_ Content-Disposition\n> header filename, 2) all browsers implement it.\n\n[...]\n> P.P.S. Here is an example of RFC2184 encoded header:\n> \n>    Content-Type: application/x-stuff\n>     title*1*=us-ascii'en'This%20is%20even%20more%20\n>     title*2*=%2A%2A%2Afun%2A%2A%2A%20\n>     title*3=\"isn't it!\"\n\nAnother example:\n\n  Content-Type: text/plain; charset=utf-8\\r\n  Content-Disposition: inline; filename*=utf-8'en-US'This%20is%0A%20%2A%2A%2Afun%2A%2A%2A\n\n\nAlthough \"RFC 2183: The Content-Disposition Header Field\" says:\n\n  Parameter values longer than 78 characters, or which contain non-ASCII\n  characters, MUST be encoded as specified in [RFC 2184].\n\nthe limit of 78 characters is because it is was created for mail, and\nsome old MUA had limit on line length. It is not the case of HTTP\nprotocol: lines can be, and are, quite long. Besides for example\nApache 2.0.54 does not understand MUA-style continued HTTP headers\nif in 'parse headers' mode: it returns server error.\n\nAs to browsers: Mozilla 1.7.12 implements RFC2183 correctly, although for\nexample shows %0A / \\n as a strange symbol in \"save as\" dialog, created\nfile has embedded newline in filename, as it should. But both Lynx 2.8.5,\nand ELinks 0.10.3 do not implement it fully and without errors.\n\nSo that is why we have:\n\n\t# It not worth potential problems to try to carry newlines\n\t# in the header; it is just _suggested_ filename\n\t$filename =~ s/[[:cntrl:]\\n\\r]/_/g;\n\n\nP.S. If there were no objections (no discussion), I'd resend\ncontent_disposition subroutine as patch to gitweb in about a week.\n\n-- \nJakub Narebski\nShadeHawk on #git\nPoland\n"},{"id":"38693","messageId":"7vircbqf5y.fsf@assigned-by-dhcp.cox.net","threadId":"7394","inReplyTo":"200704042327.49632.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Support comparing blobs (files) with different names","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-05T10:38:49Z","receivedAt":"2007-04-05T10:38: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> As to browsers: Mozilla 1.7.12 implements RFC2183 correctly, although for\n> example shows %0A / \\n as a strange symbol in \"save as\" dialog, created\n> file has embedded newline in filename, as it should. But both Lynx 2.8.5,\n> and ELinks 0.10.3 do not implement it fully and without errors.\n>\n> So that is why we have:\n>\n> \t# It not worth potential problems to try to carry newlines\n> \t# in the header; it is just _suggested_ filename\n> \t$filename =~ s/[[:cntrl:]\\n\\r]/_/g;\n\nI think the logic in the comment is very sane, although I am not\nsure if saying \\n \\r when you say :cntrl: is necessary.\n"}]}