Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
On Mon, Oct 17, 2016 at 6:54 PM, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > >> As far as I can tell the only outstanding "change this" is your >> s/SHA1/SHA-1/ in , do >> you want to fix that up or should I submit another series? > > I think I did that already myself while queuing. Could you fetch > what I queued on 'pu' to double check? Thanks, looked at it, looks good to me! > I think the diff between what was posted and what is queued (I just > checked) looks like this: > > -gitweb: Link to 7-char+ SHA1s, not only 8-char+ > +gitweb: link to 7-char+ SHA-1s, not only 8-char+ > > Change the minimum length of an abbreviated object identifier in the > commit message gitweb tries to turn into link from 8 hexchars to 7. > > @@ -5,16 +12,18 @@ > SHA-1 in commit log message links to "object" view", 2006-12-10), but > the default abbreviation length is 7, and has been for a long time. > > -It's still possible to reference SHA1s down to 4 characters in length, > +It's still possible to reference SHA-1s down to 4 characters in length, > see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make > git actually produce that, so I doubt anyone is putting that into log > -messages in practice, but people definitely do put 7 character SHA1s > +messages in practice, but people definitely do put 7 character SHA-1s > into log messages. > > I think it's fairly dubious to link to things matching [0-9a-fA-F] > here as opposed to just [0-9a-f], that dates back to the initial > version of gitweb from 161332a ("first working version", > -2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce > +2005-08-07). Git will accept all-caps SHA-1s, but didn't ever produce > them as far as I can tell. > > Signed-off-by: Ævar Arnfjörð Bjarmason > +Acked-by: Jakub Narębski > +Signed-off-by: Junio C Hamano
Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
Ævar Arnfjörð Bjarmason writes: > As far as I can tell the only outstanding "change this" is your > s/SHA1/SHA-1/ in , do > you want to fix that up or should I submit another series? I think I did that already myself while queuing. Could you fetch what I queued on 'pu' to double check? I think the diff between what was posted and what is queued (I just checked) looks like this: -gitweb: Link to 7-char+ SHA1s, not only 8-char+ +gitweb: link to 7-char+ SHA-1s, not only 8-char+ Change the minimum length of an abbreviated object identifier in the commit message gitweb tries to turn into link from 8 hexchars to 7. @@ -5,16 +12,18 @@ SHA-1 in commit log message links to "object" view", 2006-12-10), but the default abbreviation length is 7, and has been for a long time. -It's still possible to reference SHA1s down to 4 characters in length, +It's still possible to reference SHA-1s down to 4 characters in length, see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make git actually produce that, so I doubt anyone is putting that into log -messages in practice, but people definitely do put 7 character SHA1s +messages in practice, but people definitely do put 7 character SHA-1s into log messages. I think it's fairly dubious to link to things matching [0-9a-fA-F] here as opposed to just [0-9a-f], that dates back to the initial version of gitweb from 161332a ("first working version", -2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce +2005-08-07). Git will accept all-caps SHA-1s, but didn't ever produce them as far as I can tell. Signed-off-by: Ævar Arnfjörð Bjarmason +Acked-by: Jakub Narębski +Signed-off-by: Junio C Hamano
Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
On Fri, Oct 14, 2016 at 8:40 PM, Junio C Hamano wrote: > Jakub Narębski writes: > >> s/SHA1/SHA-1/g in above paragraph (for correctness and consistency). >>> >>> I think it's fairly dubious to link to things matching [0-9a-fA-F] >>> here as opposed to just [0-9a-f], that dates back to the initial >>> version of gitweb from 161332a ("first working version", >>> 2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce >>> them as far as I can tell. >> >> All right. If we decide to be more strict in what we accept, we can >> do it in a separate commit. >> >>> >>> Signed-off-by: Ævar Arnfjörð Bjarmason >> >> Acked-by: Jakub Narębski > > Thanks for a review. As the topic is not yet in 'next', I'll squish > in your Acked-by: to them. I saw them only for 1 & 2/3; would > another for 3/3 be coming soon? As far as I can tell the only outstanding "change this" is your s/SHA1/SHA-1/ in , do you want to fix that up or should I submit another series? >> >>> --- >>> gitweb/gitweb.perl | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl >>> index cba7405..92b5e91 100755 >>> --- a/gitweb/gitweb.perl >>> +++ b/gitweb/gitweb.perl >>> @@ -2036,7 +2036,7 @@ sub format_log_line_html { >>> my $line = shift; >>> >>> $line = esc_html($line, -nbsp=>1); >>> -$line =~ s{\b([0-9a-fA-F]{8,40})\b}{ >>> +$line =~ s{\b([0-9a-fA-F]{7,40})\b}{ >> >> By the way, it is quite long commit message for one character change. >> Not that it is a bad thing... >> >>> $cgi->a({-href => href(action=>"object", hash=>$1), >>> -class => "text"}, $1); >>> }eg; >>>
Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
Jakub Narębski writes: > s/SHA1/SHA-1/g in above paragraph (for correctness and consistency). >> >> I think it's fairly dubious to link to things matching [0-9a-fA-F] >> here as opposed to just [0-9a-f], that dates back to the initial >> version of gitweb from 161332a ("first working version", >> 2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce >> them as far as I can tell. > > All right. If we decide to be more strict in what we accept, we can > do it in a separate commit. > >> >> Signed-off-by: Ævar Arnfjörð Bjarmason > > Acked-by: Jakub Narębski Thanks for a review. As the topic is not yet in 'next', I'll squish in your Acked-by: to them. I saw them only for 1 & 2/3; would another for 3/3 be coming soon? > >> --- >> gitweb/gitweb.perl | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl >> index cba7405..92b5e91 100755 >> --- a/gitweb/gitweb.perl >> +++ b/gitweb/gitweb.perl >> @@ -2036,7 +2036,7 @@ sub format_log_line_html { >> my $line = shift; >> >> $line = esc_html($line, -nbsp=>1); >> -$line =~ s{\b([0-9a-fA-F]{8,40})\b}{ >> +$line =~ s{\b([0-9a-fA-F]{7,40})\b}{ > > By the way, it is quite long commit message for one character change. > Not that it is a bad thing... > >> $cgi->a({-href => href(action=>"object", hash=>$1), >> -class => "text"}, $1); >> }eg; >>
Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
W dniu 06.10.2016 o 11:11, Ævar Arnfjörð Bjarmason pisze: > Change the minimum length of an abbreviated object identifier in the > commit message gitweb tries to turn into link from 8 hexchars to 7. > > This arbitrary minimum length of 8 was introduced in bfe2191 ("gitweb: > SHA-1 in commit log message links to "object" view", 2006-12-10), but > the default abbreviation length is 7, and has been for a long time. > > It's still possible to reference SHA1s down to 4 characters in length, > see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make > git actually produce that, so I doubt anyone is putting that into log > messages in practice, but people definitely do put 7 character SHA1s > into log messages. That's nice explanation why we want to support 7 character SHA-1 (it is the default, and was seen in the wild), but don't need to support shorter lengths. Another reason (just for completeness; you don't need to put it in the commit message) to not go below 7 characters is that with decreasing length there is more and more chance for false positives (like 'beef' or 'caffee' for 4-char or 5-char hexstring), as I wrote previously. s/SHA1/SHA-1/g in above paragraph (for correctness and consistency). > > I think it's fairly dubious to link to things matching [0-9a-fA-F] > here as opposed to just [0-9a-f], that dates back to the initial > version of gitweb from 161332a ("first working version", > 2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce > them as far as I can tell. All right. If we decide to be more strict in what we accept, we can do it in a separate commit. > > Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Jakub Narębski > --- > gitweb/gitweb.perl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > index cba7405..92b5e91 100755 > --- a/gitweb/gitweb.perl > +++ b/gitweb/gitweb.perl > @@ -2036,7 +2036,7 @@ sub format_log_line_html { > my $line = shift; > > $line = esc_html($line, -nbsp=>1); > - $line =~ s{\b([0-9a-fA-F]{8,40})\b}{ > + $line =~ s{\b([0-9a-fA-F]{7,40})\b}{ By the way, it is quite long commit message for one character change. Not that it is a bad thing... > $cgi->a({-href => href(action=>"object", hash=>$1), > -class => "text"}, $1); > }eg; >
Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
Ævar Arnfjörð Bjarmason writes: > Change the minimum length of an abbreviated object identifier in the > commit message gitweb tries to turn into link from 8 hexchars to 7. > > This arbitrary minimum length of 8 was introduced in bfe2191 ("gitweb: > SHA-1 in commit log message links to "object" view", 2006-12-10), but > the default abbreviation length is 7, and has been for a long time. > > It's still possible to reference SHA1s down to 4 characters in length, > see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make > git actually produce that, so I doubt anyone is putting that into log > messages in practice, but people definitely do put 7 character SHA1s > into log messages. > > I think it's fairly dubious to link to things matching [0-9a-fA-F] > here as opposed to just [0-9a-f], that dates back to the initial > version of gitweb from 161332a ("first working version", > 2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce > them as far as I can tell. > > Signed-off-by: Ævar Arnfjörð Bjarmason > --- s/SHA1/SHA-1/g. I agree that A-F are dubious. > gitweb/gitweb.perl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > index cba7405..92b5e91 100755 > --- a/gitweb/gitweb.perl > +++ b/gitweb/gitweb.perl > @@ -2036,7 +2036,7 @@ sub format_log_line_html { > my $line = shift; > > $line = esc_html($line, -nbsp=>1); > - $line =~ s{\b([0-9a-fA-F]{8,40})\b}{ > + $line =~ s{\b([0-9a-fA-F]{7,40})\b}{ > $cgi->a({-href => href(action=>"object", hash=>$1), > -class => "text"}, $1); > }eg;
[PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
Change the minimum length of an abbreviated object identifier in the commit message gitweb tries to turn into link from 8 hexchars to 7. This arbitrary minimum length of 8 was introduced in bfe2191 ("gitweb: SHA-1 in commit log message links to "object" view", 2006-12-10), but the default abbreviation length is 7, and has been for a long time. It's still possible to reference SHA1s down to 4 characters in length, see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make git actually produce that, so I doubt anyone is putting that into log messages in practice, but people definitely do put 7 character SHA1s into log messages. I think it's fairly dubious to link to things matching [0-9a-fA-F] here as opposed to just [0-9a-f], that dates back to the initial version of gitweb from 161332a ("first working version", 2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce them as far as I can tell. Signed-off-by: Ævar Arnfjörð Bjarmason --- gitweb/gitweb.perl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index cba7405..92b5e91 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -2036,7 +2036,7 @@ sub format_log_line_html { my $line = shift; $line = esc_html($line, -nbsp=>1); - $line =~ s{\b([0-9a-fA-F]{8,40})\b}{ + $line =~ s{\b([0-9a-fA-F]{7,40})\b}{ $cgi->a({-href => href(action=>"object", hash=>$1), -class => "text"}, $1); }eg; -- 2.9.3