threads / patch / 6005

patchgitweb: Add "next" link to commitdiff view

Subject: [PATCH] gitweb: Add "next" link to commitdiff view

## tl;dr

16 messages between Oct 22, 2006 and Oct 24, 2006. Diffs are folded; open one to read it.

replies: 15people: 3as markdown or json

Jakub Narebski· Oct 22, 2006, 22:37 UTC · lore

Add a kind of "next" view in the bottom part of navigation bar for "commitdiff" view.

For commitdiff between two commits:
  (from: _commit_)
For commitdiff for one single parent commit:
  (parent: _commit_)
For commitdiff for one merge commit
  (merge: _commit_ _commit_ ...)
For commitdiff for root (parentless) commit
  (initial)
where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
display, everything is perhaps unnecessary esc_html on display.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---

Junio (and others), is it what you wanted? The idea was to change "commitdiff" view only in minimal way, and preserve similarity to "commit" format.

SHA1 ids are printed in monospace (fixed width) font. BTW. should we put '...' after shortened SHA1? Should those '...' be included in link?

 gitweb/gitweb.perl |   49 +++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 45 insertions(+), 4 deletions(-)
Show changes to gitweb/gitweb.perl +45 −4
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c9e57f0..4241d5c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3280,6 +3280,51 @@ sub git_commitdiff {
 	if (!%co) {
 		die_error(undef, "Unknown commit object");
 	}
+
+	# we need to prepare $formats_nav before any parameter munging
+	my $formats_nav;
+	if ($format eq 'html') {
+		$formats_nav =
+			$cgi->a({-href => href(action=>"commitdiff_plain",
+			                       hash=>$hash, hash_parent=>$hash_parent)},
+			        "raw");
+
+		if (defined $hash_parent) {
+			# commitdiff with two commits given
+			my $hash_parent_short = $hash_parent;
+			if ($hash_parent =~ m/^[0-9a-fA-F]{40}$/) {
+				$hash_parent_short = substr($hash_parent, 0, 7);
+			}
+			$formats_nav .=
+				' (from: ' .
+				$cgi->a({-href => href(action=>"commitdiff",
+				                       hash=>$hash_parent)},
+				        esc_html($hash_parent_short)) .
+				')';
+		} elsif (!$co{'parent'}) {
+			# --root commitdiff
+			$formats_nav .= ' (initial)';
+		} elsif (scalar @{$co{'parents'}} == 1) {
+			# single parent commit
+			$formats_nav .=
+				' (parent: ' .
+				$cgi->a({-href => href(action=>"commitdiff",
+				                       hash=>$co{'parent'})},
+				        esc_html(substr($co{'parent'}, 0, 7))) .
+				')';
+		} else {
+			# merge commit
+			$formats_nav .=
+				' (merge: ' .
+				join(' ', map {
+					$cgi->a({-href => href(action=>"commitdiff",
+					                       hash=>$_)},
+					        esc_html(substr($_, 0, 7)));
+				} @{$co{'parents'}} ) .
+				')';
+		}
+	}
+
 	if (!defined $hash_parent) {
 		$hash_parent = $co{'parent'} || '--root';
 	}
@@ -3317,10 +3362,6 @@ sub git_commitdiff {
 	if ($format eq 'html') {
 		my $refs = git_get_references();
 		my $ref = format_ref_marker($refs, $co{'id'});
-		my $formats_nav =
-			$cgi->a({-href => href(action=>"commitdiff_plain",
-			                       hash=>$hash, hash_parent=>$hash_parent)},
-			        "raw");
 
 		git_header_html(undef, $expires);
 		git_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);

-- 
1.4.2.1
Jakub Narebski· Oct 22, 2006, 22:43 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: Add "next" link to commitdiff view

BTW there are some errors in displaying commitdiff of initial commit, take for example a=commitdiff;h=161332a521fe10c41979bcd493d95e2ac562b7f for git.git repository in gitweb.

Needs fixing.
-- 
Jakub Narebski
Poland
Junio C Hamano· Oct 22, 2006, 23:16 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: Add "next" link to commitdiff view

Jakub Narebski <jnareb@gmail.com> writes:
Show 15 quoted lines
> Add a kind of "next" view in the bottom part of navigation bar for
> "commitdiff" view.
>
> For commitdiff between two commits:
>   (from: _commit_)
> For commitdiff for one single parent commit:
>   (parent: _commit_)
> For commitdiff for one merge commit
>   (merge: _commit_ _commit_ ...)
> For commitdiff for root (parentless) commit
>   (initial)
> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
> display, everything is perhaps unnecessary esc_html on display.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>

Would it even be necessary to use any SHA-1 name in these cases, I wonder. Would it make the page less useful if we replace all of the above _commit_ with a fixed string, say, "parent"?

I always hated gitweb diffs that prefix each filepair with their full 40-byte SHA-1 blob object names. It just adds noise to the output without adding any meaningful information.

Jakub Narebski· Oct 22, 2006, 23:41 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: Add "next" link to commitdiff view

Junio C Hamano wrote:
Show 7 quoted lines
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Add a kind of "next" view in the bottom part of navigation bar for
>> "commitdiff" view.
>>
>> For commitdiff between two commits:
>>   (from: _commit_)
Perhaps we should use "(from: _commit_ to: _commit_)" here...
Show 14 quoted lines
>> For commitdiff for one single parent commit:
>>   (parent: _commit_)
>> For commitdiff for one merge commit
>>   (merge: _commit_ _commit_ ...)
>> For commitdiff for root (parentless) commit
>>   (initial)
>> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
>> display, everything is perhaps unnecessary esc_html on display.
>>
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> 
> Would it even be necessary to use any SHA-1 name in these cases,
> I wonder.  Would it make the page less useful if we replace all
> of the above _commit_ with a fixed string, say, "parent"?

I decided on using _shortened_ SHA1 because I didn't like neither "(parent parent ...) " nor "(parent1 parent2 ...)" for merges. Perhaps I should have used 8-characters abbreviation, like in git_blame2. And I was inspired by git-show output for merges:

 commit ff49fae6a547e5c70117970e01c53b64d983cd10
 Merge: 7ad4ee7... 75f9007... 14eab2b... 0b35995... eee4609...
> I always hated gitweb diffs that prefix each filepair with their
> full 40-byte SHA-1 blob object names.  It just adds noise to the
> output without adding any meaningful information.

I always thought about this only as a (somewhat sophisticated) separator marking where individual patch (patch for given files) begin. And a place to click (non-hidden link) for blob before and after. Please remember that this gitweb diff header was from the times where we didn't have difftree in commitdiff view.

-- 
Jakub Narebski
Poland
Jakub Narebski· Oct 23, 2006, 22:08 UTC · re: Junio C Hamano · lore

[PATCH 2/1] gitweb: Use fixed string for "next" link in commitdiff view

Use fixed string instead of shortened SHA1 identifier of commit as a name for "mext" link in commitdiff view.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Junio C Hamano wrote:
Show 23 quoted lines
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Add a kind of "next" link in the bottom part of navigation bar for
>> "commitdiff" view.
>>
>> For commitdiff between two commits:
>>   (from: _commit_)
>> For commitdiff for one single parent commit:
>>   (parent: _commit_)
>> For commitdiff for one merge commit
>>   (merge: _commit_ _commit_ ...)
>> For commitdiff for root (parentless) commit
>>   (initial)
>> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
>> display, everything is perhaps unnecessary esc_html on display.
> 
> Would it even be necessary to use any SHA-1 name in these cases,
> I wonder.  Would it make the page less useful if we replace all
> of the above _commit_ with a fixed string, say, "parent"?
> 
> I always hated gitweb diffs that prefix each filepair with their
> full 40-byte SHA-1 blob object names.  It just adds noise to the
> output without adding any meaningful information.

Here you have. I used patch on top of previous one instead of amending previous patch because I think it is interesting to have this history of this feature.

This patch doesn't add any functionality.
 gitweb/gitweb.perl |   16 ++++++----------
 1 files changed, 6 insertions(+), 10 deletions(-)
Show changes to gitweb/gitweb.perl +6 −10
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4241d5c..255487d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3291,15 +3291,11 @@ sub git_commitdiff {
 
 		if (defined $hash_parent) {
 			# commitdiff with two commits given
-			my $hash_parent_short = $hash_parent;
-			if ($hash_parent =~ m/^[0-9a-fA-F]{40}$/) {
-				$hash_parent_short = substr($hash_parent, 0, 7);
-			}
 			$formats_nav .=
-				' (from: ' .
+				' (' .
 				$cgi->a({-href => href(action=>"commitdiff",
 				                       hash=>$hash_parent)},
-				        esc_html($hash_parent_short)) .
+				        'from') .
 				')';
 		} elsif (!$co{'parent'}) {
 			# --root commitdiff
@@ -3307,19 +3303,19 @@ sub git_commitdiff {
 		} elsif (scalar @{$co{'parents'}} == 1) {
 			# single parent commit
 			$formats_nav .=
-				' (parent: ' .
+				' (' .
 				$cgi->a({-href => href(action=>"commitdiff",
 				                       hash=>$co{'parent'})},
-				        esc_html(substr($co{'parent'}, 0, 7))) .
+				        'parent') .
 				')';
 		} else {
 			# merge commit
 			$formats_nav .=
-				' (merge: ' .
+				' (' .
 				join(' ', map {
 					$cgi->a({-href => href(action=>"commitdiff",
 					                       hash=>$_)},
-					        esc_html(substr($_, 0, 7)));
+					        'parent');
 				} @{$co{'parents'}} ) .
 				')';
 		}

-- 
1.4.2.1
Petr Baudis· Oct 23, 2006, 23:21 UTC · re: Jakub Narebski · lore

Re: [PATCH 2/1] gitweb: Use fixed string for "next" link in commitdiff view

Dear diary, on Tue, Oct 24, 2006 at 12:08:08AM CEST, I got a letter where Jakub Narebski <jnareb@gmail.com> said that...

Show 24 quoted lines
> @@ -3307,19 +3303,19 @@ sub git_commitdiff {
>  		} elsif (scalar @{$co{'parents'}} == 1) {
>  			# single parent commit
>  			$formats_nav .=
> -				' (parent: ' .
> +				' (' .
>  				$cgi->a({-href => href(action=>"commitdiff",
>  				                       hash=>$co{'parent'})},
> -				        esc_html(substr($co{'parent'}, 0, 7))) .
> +				        'parent') .
>  				')';
>  		} else {
>  			# merge commit
>  			$formats_nav .=
> -				' (merge: ' .
> +				' (' .
>  				join(' ', map {
>  					$cgi->a({-href => href(action=>"commitdiff",
>  					                       hash=>$_)},
> -					        esc_html(substr($_, 0, 7)));
> +					        'parent');
>  				} @{$co{'parents'}} ) .
>  				')';
>  		}

For people not used to git, it would be more informative to keep the 'merge' text there.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)
Jakub Narebski· Oct 24, 2006, 09:04 UTC · re: Petr Baudis · lore

[PATCH 2/1 (amend)] gitweb: Use fixed string for "next" link in commitdiff view

Use fixed string instead of shortened SHA1 identifier of commit as a name for "mext" link in commitdiff view.

While at it make sure that we don't mistake option for a second commit.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   17 +++++++----------
 1 files changed, 7 insertions(+), 10 deletions(-)
Show changes to gitweb/gitweb.perl +7 −10
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4241d5c..9c81214 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3289,17 +3289,14 @@ sub git_commitdiff {
 			                       hash=>$hash, hash_parent=>$hash_parent)},
 			        "raw");
 
-		if (defined $hash_parent) {
+		if (defined $hash_parent && $hash_parent !~ m/^-/) {
 			# commitdiff with two commits given
-			my $hash_parent_short = $hash_parent;
-			if ($hash_parent =~ m/^[0-9a-fA-F]{40}$/) {
-				$hash_parent_short = substr($hash_parent, 0, 7);
-			}
+			# second "commit" is not in fact diff option
 			$formats_nav .=
-				' (from: ' .
+				' (' .
 				$cgi->a({-href => href(action=>"commitdiff",
 				                       hash=>$hash_parent)},
-				        esc_html($hash_parent_short)) .
+				        'from') .
 				')';
 		} elsif (!$co{'parent'}) {
 			# --root commitdiff
@@ -3307,10 +3304,10 @@ sub git_commitdiff {
 		} elsif (scalar @{$co{'parents'}} == 1) {
 			# single parent commit
 			$formats_nav .=
-				' (parent: ' .
+				' (' .
 				$cgi->a({-href => href(action=>"commitdiff",
 				                       hash=>$co{'parent'})},
-				        esc_html(substr($co{'parent'}, 0, 7))) .
+				        'parent') .
 				')';
 		} else {
 			# merge commit
@@ -3319,7 +3316,7 @@ sub git_commitdiff {
 				join(' ', map {
 					$cgi->a({-href => href(action=>"commitdiff",
 					                       hash=>$_)},
-					        esc_html(substr($_, 0, 7)));
+					        'parent');
 				} @{$co{'parents'}} ) .
 				')';
 		}

-- 
1.4.2.1
Junio C Hamano· Oct 24, 2006, 09:17 UTC · re: Jakub Narebski · lore

Re: [PATCH 2/1 (amend)] gitweb: Use fixed string for "next" link in commitdiff view

Jakub Narebski <jnareb@gmail.com> writes:
Show 8 quoted lines
> Use fixed string instead of shortened SHA1 identifier of commit
> as a name for "mext" link in commitdiff view.
>
> While at it make sure that we don't mistake option for a second
> commit.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---

Q1. (amend) to fix what? Q2. "mext"?

Jakub Narebski· Oct 24, 2006, 09:28 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/1 (amend)] gitweb: Use fixed string for "next" link in commitdiff view

Dnia wtorek 24. października 2006 11:17, Junio C Hamano napisał:
Show 12 quoted lines
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Use fixed string instead of shortened SHA1 identifier of commit
>> as a name for "mext" link in commitdiff view.
>>
>> While at it make sure that we don't mistake option for a second
>> commit.
>>
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>> ---
> 
> Q1. (amend) to fix what?
Petr "Pasky" Baudis wrote:
Show 5 quoted lines
>>               } else {
>>                       # merge commit
>>                       $formats_nav .=
>> -                             ' (merge: ' .
>> +                             ' (' .
[...]
> For people not used to git, it would be more informative to keep the
> 'merge' text there.

It also fixes detection if it is diff of two specified commits. Although $hash_parent is set to '--root' for rootloess commit later, we could have it set by hand. This is also preparation for later using '--cc' (although we wouldn't generate such URL, I don't think).

> Q2. "mext"?
Oops.
-- 
Jakub Narebski
Poland
Petr Baudis· Oct 24, 2006, 11:49 UTC · re: Jakub Narebski · lore

Re: [PATCH 2/1] gitweb: Use fixed string for "next" link in commitdiff view

Dear diary, on Tue, Oct 24, 2006 at 12:08:08AM CEST, I got a letter where Jakub Narebski <jnareb@gmail.com> said that...

> Use fixed string instead of shortened SHA1 identifier of commit
> as a name for "mext" link in commitdiff view.
> 
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>

So I've read what the patch actually does this time, and I really dislike it.

Show 20 quoted lines
> Junio C Hamano wrote:
> > Jakub Narebski <jnareb@gmail.com> writes:
> > 
> >> Add a kind of "next" link in the bottom part of navigation bar for
> >> "commitdiff" view.
> >>
> >> For commitdiff between two commits:
> >>   (from: _commit_)
> >> For commitdiff for one single parent commit:
> >>   (parent: _commit_)
> >> For commitdiff for one merge commit
> >>   (merge: _commit_ _commit_ ...)
> >> For commitdiff for root (parentless) commit
> >>   (initial)
> >> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
> >> display, everything is perhaps unnecessary esc_html on display.
> > 
> > I always hated gitweb diffs that prefix each filepair with their
> > full 40-byte SHA-1 blob object names.  It just adds noise to the
> > output without adding any meaningful information.

I agree, using the shortened SHA1 would be definitely an improvement here, but

> > Would it even be necessary to use any SHA-1 name in these cases,
> > I wonder.  Would it make the page less useful if we replace all
> > of the above _commit_ with a fixed string, say, "parent"?

I really disagree here - what's the point of not using SHA-1? The extra string carries zero information in comparison with the previous state and I just can't see how it *improves* stuff. If you're walking in a maze and making marks on walls, it's still more useful if you have corridors named by "A", "B", "C", "D" on junctions if you sometimes want to walk back to the marked corridors.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)
Jakub Narebski· Oct 24, 2006, 11:59 UTC · re: Petr Baudis · lore

Re: [PATCH 2/1] gitweb: Use fixed string for "next" link in commitdiff view

Petr Baudis wrote:
Show 12 quoted lines
> Dear diary, on Tue, Oct 24, 2006 at 12:08:08AM CEST, I got a letter
> where Jakub Narebski <jnareb@gmail.com> said that...
>> Use fixed string instead of shortened SHA1 identifier of commit
>> as a name for "mext" link in commitdiff view.
>> 
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> 
> So I've read what the patch actually does this time, and I really
> dislike it.
> 
>> Junio C Hamano wrote:
>>> Jakub Narebski <jnareb@gmail.com> writes:
[...]
>>>> For commitdiff for one merge commit
>>>>   (merge: _commit_ _commit_ ...)
[...]
Show 9 quoted lines
>>>> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
>>>> display, everything is perhaps unnecessary esc_html on display.
>>> 
>>> I always hated gitweb diffs that prefix each filepair with their
>>> full 40-byte SHA-1 blob object names.  It just adds noise to the
>>> output without adding any meaningful information.
> 
> I agree, using the shortened SHA1 would be definitely an improvement
> here, but
I'm planning on revamping gitweb diff output. Any ideas?
Show 10 quoted lines
>>> Would it even be necessary to use any SHA-1 name in these cases,
>>> I wonder.  Would it make the page less useful if we replace all
>>> of the above _commit_ with a fixed string, say, "parent"?
> 
> I really disagree here - what's the point of not using SHA-1? The extra
> string carries zero information in comparison with the previous state
> and I just can't see how it *improves* stuff. If you're walking in a
> maze and making marks on walls, it's still more useful if you have
> corridors named by "A", "B", "C", "D" on junctions if you sometimes want
> to walk back to the marked corridors.

That's why instead of resending the patch/amending the commit, I have put changing shortened SHA1 to constant name in new patch.

By the way, because those links to parents commitdiffs are just ordinary links, you can see when those links are visited.

-- 
Jakub Narebski
Poland
Petr Baudis· Oct 24, 2006, 15:27 UTC · re: Jakub Narebski · lore

Re: [PATCH 2/1] gitweb: Use fixed string for "next" link in commitdiff view

Dear diary, on Tue, Oct 24, 2006 at 01:59:57PM CEST, I got a letter where Jakub Narebski <jnareb@gmail.com> said that...

Show 13 quoted lines
> >>> Would it even be necessary to use any SHA-1 name in these cases,
> >>> I wonder.  Would it make the page less useful if we replace all
> >>> of the above _commit_ with a fixed string, say, "parent"?
> > 
> > I really disagree here - what's the point of not using SHA-1? The extra
> > string carries zero information in comparison with the previous state
> > and I just can't see how it *improves* stuff. If you're walking in a
> > maze and making marks on walls, it's still more useful if you have
> > corridors named by "A", "B", "C", "D" on junctions if you sometimes want
> > to walk back to the marked corridors.
> 
> That's why instead of resending the patch/amending the commit, I have
> put changing shortened SHA1 to constant name in new patch.

Huh, I don't quite understand. You threw away all the "A", "B", "C", "D" and kept only the marks.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)
Junio C Hamano· Oct 24, 2006, 17:17 UTC · re: Petr Baudis · lore

Re: [PATCH 2/1] gitweb: Use fixed string for "next" link in commitdiff view

Petr Baudis <pasky@suse.cz> writes:
Show 10 quoted lines
>> > Would it even be necessary to use any SHA-1 name in these cases,
>> > I wonder.  Would it make the page less useful if we replace all
>> > of the above _commit_ with a fixed string, say, "parent"?
>
> I really disagree here - what's the point of not using SHA-1? The extra
> string carries zero information in comparison with the previous state
> and I just can't see how it *improves* stuff. If you're walking in a
> maze and making marks on walls, it's still more useful if you have
> corridors named by "A", "B", "C", "D" on junctions if you sometimes want
> to walk back to the marked corridors.

I think people would recognize A B C D as names but not 40- or 8- hexadecimal letters.

I do not care much either way, actually, but I think it might make more sense to use abbreviated object names. On the other hand it may be Ok to have full 40 letters depending on the layout (e.g. the set of merge parents are shown on a single line in which case it would not fit, etc.).

Petr Baudis· Oct 24, 2006, 17:26 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/1] gitweb: Use fixed string for "next" link in commitdiff view

On Tue, Oct 24, 2006 at 07:17:28PM CEST, Junio C Hamano wrote:
Show 15 quoted lines
> Petr Baudis <pasky@suse.cz> writes:
> 
> >> > Would it even be necessary to use any SHA-1 name in these cases,
> >> > I wonder.  Would it make the page less useful if we replace all
> >> > of the above _commit_ with a fixed string, say, "parent"?
> >
> > I really disagree here - what's the point of not using SHA-1? The extra
> > string carries zero information in comparison with the previous state
> > and I just can't see how it *improves* stuff. If you're walking in a
> > maze and making marks on walls, it's still more useful if you have
> > corridors named by "A", "B", "C", "D" on junctions if you sometimes want
> > to walk back to the marked corridors.
> 
> I think people would recognize A B C D as names but not 40- or
> 8- hexadecimal letters.

40-digit hex numbers is insane, I agree. But at least I personally tend to recognize 8-digit hex numbers when dancing around them intensively for a few minutes. Besides, it can be just "now I took the 8c5 way", which is much easier to train your neurons too than "now I took the fourth, uh, or was it the fifth parent? one, two, three, four, fifth... hmm, what's in the statusbar?".

My point is that this does not improve the situation, and some people (me) think it makes it worse, so what's the point of the change?

Show 5 quoted lines
> I do not care much either way, actually, but I think it might
> make more sense to use abbreviated object names.  On the other
> hand it may be Ok to have full 40 letters depending on the
> layout (e.g. the set of merge parents are shown on a single line
> in which case it would not fit, etc.).

Yes, I'm all for abbreviated names, but I'm against just writing "parent" everywhere.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)
Junio C Hamano· Oct 24, 2006, 18:27 UTC · re: Petr Baudis · lore

Re: [PATCH 2/1] gitweb: Use fixed string for "next" link in commitdiff view

Petr Baudis <pasky@suse.cz> writes:
Show 35 quoted lines
> On Tue, Oct 24, 2006 at 07:17:28PM CEST, Junio C Hamano wrote:
>> Petr Baudis <pasky@suse.cz> writes:
>> 
>> >> > Would it even be necessary to use any SHA-1 name in these cases,
>> >> > I wonder.  Would it make the page less useful if we replace all
>> >> > of the above _commit_ with a fixed string, say, "parent"?
>> >
>> > I really disagree here - what's the point of not using SHA-1? The extra
>> > string carries zero information in comparison with the previous state
>> > and I just can't see how it *improves* stuff. If you're walking in a
>> > maze and making marks on walls, it's still more useful if you have
>> > corridors named by "A", "B", "C", "D" on junctions if you sometimes want
>> > to walk back to the marked corridors.
>> 
>> I think people would recognize A B C D as names but not 40- or
>> 8- hexadecimal letters.
>
> 40-digit hex numbers is insane, I agree. But at least I personally tend
> to recognize 8-digit hex numbers when dancing around them intensively
> for a few minutes. Besides, it can be just "now I took the 8c5 way",
> which is much easier to train your neurons too than "now I took the
> fourth, uh, or was it the fifth parent? one, two, three, four, fifth...
> hmm, what's in the statusbar?".
>
> My point is that this does not improve the situation, and some people
> (me) think it makes it worse, so what's the point of the change?
>
>> I do not care much either way, actually, but I think it might
>> make more sense to use abbreviated object names.  On the other
>> hand it may be Ok to have full 40 letters depending on the
>> layout (e.g. the set of merge parents are shown on a single line
>> in which case it would not fit, etc.).
>
> Yes, I'm all for abbreviated names, but I'm against just writing
> "parent" everywhere.

Fully agreed. Please make it so, if you so are inclined, perhaps between you and Jakub?

Jakub Narebski· Oct 24, 2006, 18:37 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/1] gitweb: Use fixed string for "next" link in commitdiff view

Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
Show 5 quoted lines
>> Yes, I'm all for abbreviated names, but I'm against just writing
>> "parent" everywhere.
> 
> Fully agreed.  Please make it so, if you so are inclined,
> perhaps between you and Jakub?

I'm all for abbreviated names (that was my first solution, wasn't it). This patch was because of Junio mail:

> Would it even be necessary to use any SHA-1 name in these cases,
> I wonder.  Would it make the page less useful if we replace all
> of the above _commit_ with a fixed string, say, "parent"?
So please drop this patch.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

← back to recent threads