{"thread":{"id":"6005","subject":"[PATCH] gitweb: Add \"next\" link to commitdiff view","startedAt":"2006-10-22T22:37:56Z","lastAt":"2006-10-24T18:37:02Z","messageCount":16,"participants":["Jakub Narebski","Junio C Hamano","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"29682","messageId":"200610230037.57183.jnareb@gmail.com","threadId":"6005","inReplyTo":null,"subject":"[PATCH] gitweb: Add \"next\" link to commitdiff view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-22T22:37:56Z","receivedAt":"2006-10-22T22:37:56Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add a kind of \"next\" view in the bottom part of navigation bar for\n\"commitdiff\" view.\n\nFor commitdiff between two commits:\n (from: _commit_)\nFor commitdiff for one single parent commit:\n (parent: _commit_)\nFor commitdiff for one merge commit\n (merge: _commit_ _commit_ ...)\nFor commitdiff for root (parentless) commit\n (initial)\nwhere _link_ denotes hyperlink. SHA1 is shortened to 7 characters on\ndisplay, everything is perhaps unnecessary esc_html on display.\n\nSigned-off-by: Jakub Narebski \n---\n\nJunio (and others), is it what you wanted? The idea was to change\n\"commitdiff\" view only in minimal way, and preserve similarity\nto \"commit\" format. \n\nSHA1 ids are printed in monospace (fixed width) font. BTW. should\nwe put '...' after shortened SHA1? Should those '...' be included\nin link?\n\n\n gitweb/gitweb.perl | 49 +++++++++++++++++++++++++++++++++++++++++++++----\n 1 files changed, 45 insertions(+), 4 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex c9e57f0..4241d5c 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3280,6 +3280,51 @@ sub git_commitdiff {\n \tif (!%co) {\n \t\tdie_error(undef, \"Unknown commit object\");\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=>\"commitdiff_plain\",\n+\t\t\t hash=>$hash, hash_parent=>$hash_parent)},\n+\t\t\t \"raw\");\n+\n+\t\tif (defined $hash_parent) {\n+\t\t\t# commitdiff with two commits given\n+\t\t\tmy $hash_parent_short = $hash_parent;\n+\t\t\tif ($hash_parent =~ m/^[0-9a-fA-F]{40}$/) {\n+\t\t\t\t$hash_parent_short = substr($hash_parent, 0, 7);\n+\t\t\t}\n+\t\t\t$formats_nav .=\n+\t\t\t\t' (from: ' .\n+\t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n+\t\t\t\t hash=>$hash_parent)},\n+\t\t\t\t esc_html($hash_parent_short)) .\n+\t\t\t\t')';\n+\t\t} elsif (!$co{'parent'}) {\n+\t\t\t# --root commitdiff\n+\t\t\t$formats_nav .= ' (initial)';\n+\t\t} elsif (scalar @{$co{'parents'}} == 1) {\n+\t\t\t# single parent commit\n+\t\t\t$formats_nav .=\n+\t\t\t\t' (parent: ' .\n+\t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n+\t\t\t\t hash=>$co{'parent'})},\n+\t\t\t\t esc_html(substr($co{'parent'}, 0, 7))) .\n+\t\t\t\t')';\n+\t\t} else {\n+\t\t\t# merge commit\n+\t\t\t$formats_nav .=\n+\t\t\t\t' (merge: ' .\n+\t\t\t\tjoin(' ', map {\n+\t\t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n+\t\t\t\t\t hash=>$_)},\n+\t\t\t\t\t esc_html(substr($_, 0, 7)));\n+\t\t\t\t} @{$co{'parents'}} ) .\n+\t\t\t\t')';\n+\t\t}\n+\t}\n+\n \tif (!defined $hash_parent) {\n \t\t$hash_parent = $co{'parent'} || '--root';\n \t}\n@@ -3317,10 +3362,6 @@ sub git_commitdiff {\n \tif ($format eq 'html') {\n \t\tmy $refs = git_get_references();\n \t\tmy $ref = format_ref_marker($refs, $co{'id'});\n-\t\tmy $formats_nav =\n-\t\t\t$cgi->a({-href => href(action=>\"commitdiff_plain\",\n-\t\t\t hash=>$hash, hash_parent=>$hash_parent)},\n-\t\t\t \"raw\");\n \n \t\tgit_header_html(undef, $expires);\n \t\tgit_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);\n-- \n1.4.2.1\n"},{"id":"29683","messageId":"200610230043.11524.jnareb@gmail.com","threadId":"6005","inReplyTo":"200610230037.57183.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Add \"next\" link to commitdiff view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-22T22:43:11Z","receivedAt":"2006-10-22T22:43:11Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"BTW there are some errors in displaying commitdiff of initial commit, \ntake for example a=commitdiff;h=161332a521fe10c41979bcd493d95e2ac562b7f\nfor git.git repository in gitweb.\n\nNeeds fixing.\n-- \nJakub Narebski\nPoland\n"},{"id":"29688","messageId":"7vd58k0wmx.fsf@assigned-by-dhcp.cox.net","threadId":"6005","inReplyTo":"200610230037.57183.jnareb@gmail.com","subject":"Re: [PATCH] gitweb: Add \"next\" link to commitdiff view","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-22T23:16:22Z","receivedAt":"2006-10-22T23:16:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Add a kind of \"next\" view in the bottom part of navigation bar for\n> \"commitdiff\" view.\n>\n> For commitdiff between two commits:\n> (from: _commit_)\n> For commitdiff for one single parent commit:\n> (parent: _commit_)\n> For commitdiff for one merge commit\n> (merge: _commit_ _commit_ ...)\n> For commitdiff for root (parentless) commit\n> (initial)\n> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on\n> display, everything is perhaps unnecessary esc_html on display.\n>\n> Signed-off-by: Jakub Narebski \n\nWould it even be necessary to use any SHA-1 name in these cases,\nI wonder. Would it make the page less useful if we replace all\nof the above _commit_ with a fixed string, say, \"parent\"?\n\nI always hated gitweb diffs that prefix each filepair with their\nfull 40-byte SHA-1 blob object names. It just adds noise to the\noutput without adding any meaningful information.\n"},{"id":"29691","messageId":"200610230141.54413.jnareb@gmail.com","threadId":"6005","inReplyTo":"7vd58k0wmx.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] gitweb: Add \"next\" link to commitdiff view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-22T23:41:54Z","receivedAt":"2006-10-22T23:41:54Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n>> Add a kind of \"next\" view in the bottom part of navigation bar for\n>> \"commitdiff\" view.\n>>\n>> For commitdiff between two commits:\n>> (from: _commit_)\nPerhaps we should use \"(from: _commit_ to: _commit_)\" here...\n\n>> For commitdiff for one single parent commit:\n>> (parent: _commit_)\n>> For commitdiff for one merge commit\n>> (merge: _commit_ _commit_ ...)\n>> For commitdiff for root (parentless) commit\n>> (initial)\n>> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on\n>> display, everything is perhaps unnecessary esc_html on display.\n>>\n>> Signed-off-by: Jakub Narebski \n> \n> Would it even be necessary to use any SHA-1 name in these cases,\n> I wonder. Would it make the page less useful if we replace all\n> of the above _commit_ with a fixed string, say, \"parent\"?\n\nI decided on using _shortened_ SHA1 because I didn't like neither \n\"(parent parent ...) \" nor \"(parent1 parent2 ...)\" for merges. Perhaps \nI should have used 8-characters abbreviation, like in git_blame2.\nAnd I was inspired by git-show output for merges:\n\n commit ff49fae6a547e5c70117970e01c53b64d983cd10\n Merge: 7ad4ee7... 75f9007... 14eab2b... 0b35995... eee4609...\n\n> I always hated gitweb diffs that prefix each filepair with their\n> full 40-byte SHA-1 blob object names. It just adds noise to the\n> output without adding any meaningful information.\n\nI always thought about this only as a (somewhat sophisticated) separator \nmarking where individual patch (patch for given files) begin. And\na place to click (non-hidden link) for blob before and after. Please \nremember that this gitweb diff header was from the times where we \ndidn't have difftree in commitdiff view.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"29773","messageId":"200610240008.08325.jnareb@gmail.com","threadId":"6005","inReplyTo":"7vd58k0wmx.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH 2/1] gitweb: Use fixed string for \"next\" link in commitdiff view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-23T22:08:08Z","receivedAt":"2006-10-23T22:08:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Use fixed string instead of shortened SHA1 identifier of commit\nas a name for \"mext\" link in commitdiff view.\n\nSigned-off-by: Jakub Narebski \n---\n\nJunio C Hamano wrote:\n> Jakub Narebski writes:\n> \n>> Add a kind of \"next\" link in the bottom part of navigation bar for\n>> \"commitdiff\" view.\n>>\n>> For commitdiff between two commits:\n>> (from: _commit_)\n>> For commitdiff for one single parent commit:\n>> (parent: _commit_)\n>> For commitdiff for one merge commit\n>> (merge: _commit_ _commit_ ...)\n>> For commitdiff for root (parentless) commit\n>> (initial)\n>> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on\n>> display, everything is perhaps unnecessary esc_html on display.\n> \n> Would it even be necessary to use any SHA-1 name in these cases,\n> I wonder. Would it make the page less useful if we replace all\n> of the above _commit_ with a fixed string, say, \"parent\"?\n> \n> I always hated gitweb diffs that prefix each filepair with their\n> full 40-byte SHA-1 blob object names. It just adds noise to the\n> output without adding any meaningful information.\n\nHere you have. I used patch on top of previous one instead of amending\nprevious patch because I think it is interesting to have this history\nof this feature.\n\nThis patch doesn't add any functionality.\n\n\n gitweb/gitweb.perl | 16 ++++++----------\n 1 files changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4241d5c..255487d 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3291,15 +3291,11 @@ sub git_commitdiff {\n \n \t\tif (defined $hash_parent) {\n \t\t\t# commitdiff with two commits given\n-\t\t\tmy $hash_parent_short = $hash_parent;\n-\t\t\tif ($hash_parent =~ m/^[0-9a-fA-F]{40}$/) {\n-\t\t\t\t$hash_parent_short = substr($hash_parent, 0, 7);\n-\t\t\t}\n \t\t\t$formats_nav .=\n-\t\t\t\t' (from: ' .\n+\t\t\t\t' (' .\n \t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n \t\t\t\t hash=>$hash_parent)},\n-\t\t\t\t esc_html($hash_parent_short)) .\n+\t\t\t\t 'from') .\n \t\t\t\t')';\n \t\t} elsif (!$co{'parent'}) {\n \t\t\t# --root commitdiff\n@@ -3307,19 +3303,19 @@ sub git_commitdiff {\n \t\t} elsif (scalar @{$co{'parents'}} == 1) {\n \t\t\t# single parent commit\n \t\t\t$formats_nav .=\n-\t\t\t\t' (parent: ' .\n+\t\t\t\t' (' .\n \t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n \t\t\t\t hash=>$co{'parent'})},\n-\t\t\t\t esc_html(substr($co{'parent'}, 0, 7))) .\n+\t\t\t\t 'parent') .\n \t\t\t\t')';\n \t\t} else {\n \t\t\t# merge commit\n \t\t\t$formats_nav .=\n-\t\t\t\t' (merge: ' .\n+\t\t\t\t' (' .\n \t\t\t\tjoin(' ', map {\n \t\t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n \t\t\t\t\t hash=>$_)},\n-\t\t\t\t\t esc_html(substr($_, 0, 7)));\n+\t\t\t\t\t 'parent');\n \t\t\t\t} @{$co{'parents'}} ) .\n \t\t\t\t')';\n \t\t}\n-- \n1.4.2.1\n"},{"id":"29783","messageId":"20061023232117.GV20017@pasky.or.cz","threadId":"6005","inReplyTo":"200610240008.08325.jnareb@gmail.com","subject":"Re: [PATCH 2/1] gitweb: Use fixed string for \"next\" link in commitdiff view","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-10-23T23:21:17Z","receivedAt":"2006-10-23T23:21:17Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Tue, Oct 24, 2006 at 12:08:08AM CEST, I got a letter\nwhere Jakub Narebski said that...\n> @@ -3307,19 +3303,19 @@ sub git_commitdiff {\n> \t\t} elsif (scalar @{$co{'parents'}} == 1) {\n> \t\t\t# single parent commit\n> \t\t\t$formats_nav .=\n> -\t\t\t\t' (parent: ' .\n> +\t\t\t\t' (' .\n> \t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n> \t\t\t\t hash=>$co{'parent'})},\n> -\t\t\t\t esc_html(substr($co{'parent'}, 0, 7))) .\n> +\t\t\t\t 'parent') .\n> \t\t\t\t')';\n> \t\t} else {\n> \t\t\t# merge commit\n> \t\t\t$formats_nav .=\n> -\t\t\t\t' (merge: ' .\n> +\t\t\t\t' (' .\n> \t\t\t\tjoin(' ', map {\n> \t\t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n> \t\t\t\t\t hash=>$_)},\n> -\t\t\t\t\t esc_html(substr($_, 0, 7)));\n> +\t\t\t\t\t 'parent');\n> \t\t\t\t} @{$co{'parents'}} ) .\n> \t\t\t\t')';\n> \t\t}\n\nFor people not used to git, it would be more informative to keep the\n'merge' text there.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i\n---\n gitweb/gitweb.perl | 17 +++++++----------\n 1 files changed, 7 insertions(+), 10 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4241d5c..9c81214 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3289,17 +3289,14 @@ sub git_commitdiff {\n \t\t\t hash=>$hash, hash_parent=>$hash_parent)},\n \t\t\t \"raw\");\n \n-\t\tif (defined $hash_parent) {\n+\t\tif (defined $hash_parent && $hash_parent !~ m/^-/) {\n \t\t\t# commitdiff with two commits given\n-\t\t\tmy $hash_parent_short = $hash_parent;\n-\t\t\tif ($hash_parent =~ m/^[0-9a-fA-F]{40}$/) {\n-\t\t\t\t$hash_parent_short = substr($hash_parent, 0, 7);\n-\t\t\t}\n+\t\t\t# second \"commit\" is not in fact diff option\n \t\t\t$formats_nav .=\n-\t\t\t\t' (from: ' .\n+\t\t\t\t' (' .\n \t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n \t\t\t\t hash=>$hash_parent)},\n-\t\t\t\t esc_html($hash_parent_short)) .\n+\t\t\t\t 'from') .\n \t\t\t\t')';\n \t\t} elsif (!$co{'parent'}) {\n \t\t\t# --root commitdiff\n@@ -3307,10 +3304,10 @@ sub git_commitdiff {\n \t\t} elsif (scalar @{$co{'parents'}} == 1) {\n \t\t\t# single parent commit\n \t\t\t$formats_nav .=\n-\t\t\t\t' (parent: ' .\n+\t\t\t\t' (' .\n \t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n \t\t\t\t hash=>$co{'parent'})},\n-\t\t\t\t esc_html(substr($co{'parent'}, 0, 7))) .\n+\t\t\t\t 'parent') .\n \t\t\t\t')';\n \t\t} else {\n \t\t\t# merge commit\n@@ -3319,7 +3316,7 @@ sub git_commitdiff {\n \t\t\t\tjoin(' ', map {\n \t\t\t\t\t$cgi->a({-href => href(action=>\"commitdiff\",\n \t\t\t\t\t hash=>$_)},\n-\t\t\t\t\t esc_html(substr($_, 0, 7)));\n+\t\t\t\t\t 'parent');\n \t\t\t\t} @{$co{'parents'}} ) .\n \t\t\t\t')';\n \t\t}\n-- \n1.4.2.1\n"},{"id":"29835","messageId":"7vfydehy2q.fsf@assigned-by-dhcp.cox.net","threadId":"6005","inReplyTo":"200610241104.18517.jnareb@gmail.com","subject":"Re: [PATCH 2/1 (amend)] gitweb: Use fixed string for \"next\" link in commitdiff view","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-24T09:17:49Z","receivedAt":"2006-10-24T09:17:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Use fixed string instead of shortened SHA1 identifier of commit\n> as a name for \"mext\" link in commitdiff view.\n>\n> While at it make sure that we don't mistake option for a second\n> commit.\n>\n> Signed-off-by: Jakub Narebski \n> ---\n\nQ1. (amend) to fix what?\nQ2. \"mext\"?\n"},{"id":"29836","messageId":"200610241128.34532.jnareb@gmail.com","threadId":"6005","inReplyTo":"7vfydehy2q.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/1 (amend)] gitweb: Use fixed string for \"next\" link in commitdiff view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-24T09:28:34Z","receivedAt":"2006-10-24T09:28:34Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia wtorek 24. października 2006 11:17, Junio C Hamano napisał:\n> Jakub Narebski writes:\n> \n>> Use fixed string instead of shortened SHA1 identifier of commit\n>> as a name for \"mext\" link in commitdiff view.\n>>\n>> While at it make sure that we don't mistake option for a second\n>> commit.\n>>\n>> Signed-off-by: Jakub Narebski \n>> ---\n> \n> Q1. (amend) to fix what?\n\nPetr \"Pasky\" Baudis wrote:\n>>               } else {\n>>                       # merge commit\n>>                       $formats_nav .=\n>> -                             ' (merge: ' .\n>> +                             ' (' .\n[...]\n> For people not used to git, it would be more informative to keep the\n> 'merge' text there.\n\nIt also fixes detection if it is diff of two specified commits. Although\n$hash_parent is set to '--root' for rootloess commit later, we could\nhave it set by hand. This is also preparation for later using '--cc'\n(although we wouldn't generate such URL, I don't think).\n\n> Q2. \"mext\"?\nOops.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"29852","messageId":"20061024114923.GD20017@pasky.or.cz","threadId":"6005","inReplyTo":"200610240008.08325.jnareb@gmail.com","subject":"Re: [PATCH 2/1] gitweb: Use fixed string for \"next\" link in commitdiff view","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-10-24T11:49:23Z","receivedAt":"2006-10-24T11:49:23Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Tue, Oct 24, 2006 at 12:08:08AM CEST, I got a letter\nwhere Jakub Narebski said that...\n> Use fixed string instead of shortened SHA1 identifier of commit\n> as a name for \"mext\" link in commitdiff view.\n> \n> Signed-off-by: Jakub Narebski \n\nSo I've read what the patch actually does this time, and I really\ndislike it.\n\n> Junio C Hamano wrote:\n> > Jakub Narebski writes:\n> > \n> >> Add a kind of \"next\" link in the bottom part of navigation bar for\n> >> \"commitdiff\" view.\n> >>\n> >> For commitdiff between two commits:\n> >> (from: _commit_)\n> >> For commitdiff for one single parent commit:\n> >> (parent: _commit_)\n> >> For commitdiff for one merge commit\n> >> (merge: _commit_ _commit_ ...)\n> >> For commitdiff for root (parentless) commit\n> >> (initial)\n> >> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on\n> >> display, everything is perhaps unnecessary esc_html on display.\n> > \n> > I always hated gitweb diffs that prefix each filepair with their\n> > full 40-byte SHA-1 blob object names. It just adds noise to the\n> > output without adding any meaningful information.\n\nI agree, using the shortened SHA1 would be definitely an improvement\nhere, but\n\n> > Would it even be necessary to use any SHA-1 name in these cases,\n> > I wonder. Would it make the page less useful if we replace all\n> > of the above _commit_ with a fixed string, say, \"parent\"?\n\nI really disagree here - what's the point of not using SHA-1? The extra\nstring carries zero information in comparison with the previous state\nand I just can't see how it *improves* stuff. If you're walking in a\nmaze and making marks on walls, it's still more useful if you have\ncorridors named by \"A\", \"B\", \"C\", \"D\" on junctions if you sometimes want\nto walk back to the marked corridors.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i Dear diary, on Tue, Oct 24, 2006 at 12:08:08AM CEST, I got a letter\n> where Jakub Narebski said that...\n>> Use fixed string instead of shortened SHA1 identifier of commit\n>> as a name for \"mext\" link in commitdiff view.\n>> \n>> Signed-off-by: Jakub Narebski \n> \n> So I've read what the patch actually does this time, and I really\n> dislike it.\n> \n>> Junio C Hamano wrote:\n>>> Jakub Narebski writes:\n[...]\n>>>> For commitdiff for one merge commit\n>>>> (merge: _commit_ _commit_ ...)\n[...]\n>>>> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on\n>>>> display, everything is perhaps unnecessary esc_html on display.\n>>> \n>>> I always hated gitweb diffs that prefix each filepair with their\n>>> full 40-byte SHA-1 blob object names. It just adds noise to the\n>>> output without adding any meaningful information.\n> \n> I agree, using the shortened SHA1 would be definitely an improvement\n> here, but\n\nI'm planning on revamping gitweb diff output. Any ideas?\n\n>>> Would it even be necessary to use any SHA-1 name in these cases,\n>>> I wonder. Would it make the page less useful if we replace all\n>>> of the above _commit_ with a fixed string, say, \"parent\"?\n> \n> I really disagree here - what's the point of not using SHA-1? The extra\n> string carries zero information in comparison with the previous state\n> and I just can't see how it *improves* stuff. If you're walking in a\n> maze and making marks on walls, it's still more useful if you have\n> corridors named by \"A\", \"B\", \"C\", \"D\" on junctions if you sometimes want\n> to walk back to the marked corridors.\n\nThat's why instead of resending the patch/amending the commit, I have\nput changing shortened SHA1 to constant name in new patch.\n\nBy the way, because those links to parents commitdiffs are just ordinary\nlinks, you can see when those links are visited.\n-- \nJakub Narebski\nPoland\n"},{"id":"29867","messageId":"20061024152718.GE20017@pasky.or.cz","threadId":"6005","inReplyTo":"200610241359.57389.jnareb@gmail.com","subject":"Re: [PATCH 2/1] gitweb: Use fixed string for \"next\" link in commitdiff view","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-10-24T15:27:18Z","receivedAt":"2006-10-24T15:27:18Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Tue, Oct 24, 2006 at 01:59:57PM CEST, I got a letter\nwhere Jakub Narebski said that...\n> >>> Would it even be necessary to use any SHA-1 name in these cases,\n> >>> I wonder. Would it make the page less useful if we replace all\n> >>> of the above _commit_ with a fixed string, say, \"parent\"?\n> > \n> > I really disagree here - what's the point of not using SHA-1? The extra\n> > string carries zero information in comparison with the previous state\n> > and I just can't see how it *improves* stuff. If you're walking in a\n> > maze and making marks on walls, it's still more useful if you have\n> > corridors named by \"A\", \"B\", \"C\", \"D\" on junctions if you sometimes want\n> > to walk back to the marked corridors.\n> \n> That's why instead of resending the patch/amending the commit, I have\n> put changing shortened SHA1 to constant name in new patch.\n\nHuh, I don't quite understand. You threw away all the \"A\", \"B\", \"C\", \"D\"\nand kept only the marks.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i writes:\n\n>> > Would it even be necessary to use any SHA-1 name in these cases,\n>> > I wonder. Would it make the page less useful if we replace all\n>> > of the above _commit_ with a fixed string, say, \"parent\"?\n>\n> I really disagree here - what's the point of not using SHA-1? The extra\n> string carries zero information in comparison with the previous state\n> and I just can't see how it *improves* stuff. If you're walking in a\n> maze and making marks on walls, it's still more useful if you have\n> corridors named by \"A\", \"B\", \"C\", \"D\" on junctions if you sometimes want\n> to walk back to the marked corridors.\n\nI think people would recognize A B C D as names but not 40- or\n8- hexadecimal letters.\n\nI do not care much either way, actually, but I think it might\nmake more sense to use abbreviated object names. On the other\nhand it may be Ok to have full 40 letters depending on the\nlayout (e.g. the set of merge parents are shown on a single line\nin which case it would not fit, etc.).\n"},{"id":"29876","messageId":"20061024172627.GU18879@pasky.or.cz","threadId":"6005","inReplyTo":"7vu01thbvb.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/1] gitweb: Use fixed string for \"next\" link in commitdiff view","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-10-24T17:26:27Z","receivedAt":"2006-10-24T17:26:27Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Tue, Oct 24, 2006 at 07:17:28PM CEST, Junio C Hamano wrote:\n> Petr Baudis writes:\n> \n> >> > Would it even be necessary to use any SHA-1 name in these cases,\n> >> > I wonder. Would it make the page less useful if we replace all\n> >> > of the above _commit_ with a fixed string, say, \"parent\"?\n> >\n> > I really disagree here - what's the point of not using SHA-1? The extra\n> > string carries zero information in comparison with the previous state\n> > and I just can't see how it *improves* stuff. If you're walking in a\n> > maze and making marks on walls, it's still more useful if you have\n> > corridors named by \"A\", \"B\", \"C\", \"D\" on junctions if you sometimes want\n> > to walk back to the marked corridors.\n> \n> I think people would recognize A B C D as names but not 40- or\n> 8- hexadecimal letters.\n\n40-digit hex numbers is insane, I agree. But at least I personally tend\nto recognize 8-digit hex numbers when dancing around them intensively\nfor a few minutes. Besides, it can be just \"now I took the 8c5 way\",\nwhich is much easier to train your neurons too than \"now I took the\nfourth, uh, or was it the fifth parent? one, two, three, four, fifth...\nhmm, what's in the statusbar?\".\n\nMy point is that this does not improve the situation, and some people\n(me) think it makes it worse, so what's the point of the change?\n\n> I do not care much either way, actually, but I think it might\n> make more sense to use abbreviated object names. On the other\n> hand it may be Ok to have full 40 letters depending on the\n> layout (e.g. the set of merge parents are shown on a single line\n> in which case it would not fit, etc.).\n\nYes, I'm all for abbreviated names, but I'm against just writing\n\"parent\" everywhere.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i writes:\n\n> On Tue, Oct 24, 2006 at 07:17:28PM CEST, Junio C Hamano wrote:\n>> Petr Baudis writes:\n>> \n>> >> > Would it even be necessary to use any SHA-1 name in these cases,\n>> >> > I wonder. Would it make the page less useful if we replace all\n>> >> > of the above _commit_ with a fixed string, say, \"parent\"?\n>> >\n>> > I really disagree here - what's the point of not using SHA-1? The extra\n>> > string carries zero information in comparison with the previous state\n>> > and I just can't see how it *improves* stuff. If you're walking in a\n>> > maze and making marks on walls, it's still more useful if you have\n>> > corridors named by \"A\", \"B\", \"C\", \"D\" on junctions if you sometimes want\n>> > to walk back to the marked corridors.\n>> \n>> I think people would recognize A B C D as names but not 40- or\n>> 8- hexadecimal letters.\n>\n> 40-digit hex numbers is insane, I agree. But at least I personally tend\n> to recognize 8-digit hex numbers when dancing around them intensively\n> for a few minutes. Besides, it can be just \"now I took the 8c5 way\",\n> which is much easier to train your neurons too than \"now I took the\n> fourth, uh, or was it the fifth parent? one, two, three, four, fifth...\n> hmm, what's in the statusbar?\".\n>\n> My point is that this does not improve the situation, and some people\n> (me) think it makes it worse, so what's the point of the change?\n>\n>> I do not care much either way, actually, but I think it might\n>> make more sense to use abbreviated object names. On the other\n>> hand it may be Ok to have full 40 letters depending on the\n>> layout (e.g. the set of merge parents are shown on a single line\n>> in which case it would not fit, etc.).\n>\n> Yes, I'm all for abbreviated names, but I'm against just writing\n> \"parent\" everywhere.\n\nFully agreed. Please make it so, if you so are inclined,\nperhaps between you and Jakub?\n"},{"id":"29882","messageId":"ehlmfg$bee$2@sea.gmane.org","threadId":"6005","inReplyTo":"7viri9h8m4.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/1] gitweb: Use fixed string for \"next\" link in commitdiff view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-24T18:37:02Z","receivedAt":"2006-10-24T18:37:02Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Petr Baudis writes:\n\n>> Yes, I'm all for abbreviated names, but I'm against just writing\n>> \"parent\" everywhere.\n> \n> Fully agreed.  Please make it so, if you so are inclined,\n> perhaps between you and Jakub?\n\nI'm all for abbreviated names (that was my first solution, wasn't it).\nThis patch was because of Junio mail:\n> Would it even be necessary to use any SHA-1 name in these cases,\n> I wonder. Would it make the page less useful if we replace all\n> of the above _commit_ with a fixed string, say, \"parent\"?\n\nSo please drop this patch.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"}]}