From: Junio C Hamano <gitster@pobox•com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail•com>
Cc: git@vger•kernel.org, Jakub Narebski <jnareb@gmail•com>
Subject: Re: [PATCH v2 3/3] gitweb: Link to "git describe"'d commits in log messages
Date: Thu, 06 Oct 2016 12:51:07 -0700 [thread overview]
Message-ID: <xmqqy421i7dg.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20161006091135.29590-4-avarab@gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Thu, 6 Oct 2016 09:11:35 +0000")
Ævar Arnfjörð Bjarmason <avarab@gmail•com> writes:
> Change the log formatting function to know about "git describe" output
> such as "v2.8.0-4-g867ad08", in addition to just plain "867ad08".
>
> There are still many valid refnames that we don't link to
> e.g. v2.10.0-rc1~2^2~1 is also a valid way to refer to
> v2.8.0-4-g867ad08, but I'm not supporting that with this commit,
> similarly it's trivially possible to create some refnames like
> "æ/var-gf6727b0" or which won't be picked up by this regex.
Not a serious counter-proposal or suggestion, and certainly not an
objection to the patch I am responding to, but I wonder if it is so
bad if we made the 867ad08 into link when showing v2.8.0-4-g867ad08.
IOW, in addition to \b followed by [0-9a-f]+ followed by \b, if we
allowed an optional 'g' in front of the hex, e.g.
- $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
+ $line =~ s{\bg?([0-9a-fA-F]{7,40})\b}{
wouldn't that be much simpler, covers more cases and sufficient?
> There's surely room for improvement here, but I just wanted to address
> the very common case of sticking "git describe" output into commit
> messages without trying to link to all possible refnames, that's going
> to be a rather futile exercise given that this is free text, and it
> would be prohibitively expensive to look up whether the references in
> question exist in our repository.
>
> There was on-list discussion about how we could do better than this
> patch. Junio suggested to update parse_commits() to call a new
> "gitweb--helper" command which would pass each of the revision
> candidates through "rev-parse --verify --quiet". That would cut down
> on our false positives (e.g. we'll link to "deadbeef"), and also allow
> us to be more aggressive in selecting candidate revisions.
>
> That may be too expensive to work in practice, or it may
> not. Investigating that would be a good follow-up to this patch.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail•com>
> ---
> gitweb/gitweb.perl | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 92b5e91..7cf68f0 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2036,10 +2036,24 @@ sub format_log_line_html {
> my $line = shift;
>
> $line = esc_html($line, -nbsp=>1);
> - $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
> + $line =~ s{
> + \b
> + (
> + # The output of "git describe", e.g. v2.10.0-297-gf6727b0
> + # or hadoop-20160921-113441-20-g094fb7d
> + (?<!-) # see strbuf_check_tag_ref(). Tags can't start with -
> + [A-Za-z0-9.-]+
> + (?!\.) # refs can't end with ".", see check_refname_format()
> + -g[0-9a-fA-F]{7,40}
> + |
> + # Just a normal looking Git SHA1
> + [0-9a-fA-F]{7,40}
> + )
> + \b
> + }{
> $cgi->a({-href => href(action=>"object", hash=>$1),
> -class => "text"}, $1);
> - }eg;
> + }egx;
>
> return $line;
> }
next prev parent reply other threads:[~2016-10-06 19:51 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-06 9:11 [PATCH v2 0/3] gitweb: Be smarter about linking to SHA1s in log messages Ævar Arnfjörð Bjarmason
2016-10-06 9:11 ` [PATCH v2 1/3] gitweb: Fix a typo in a comment Ævar Arnfjörð Bjarmason
2016-10-14 17:34 ` Jakub Narębski
2016-10-06 9:11 ` [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+ Ævar Arnfjörð Bjarmason
2016-10-06 19:44 ` Junio C Hamano
2016-10-14 17:45 ` Jakub Narębski
2016-10-14 18:40 ` Junio C Hamano
2016-10-15 8:11 ` Ævar Arnfjörð Bjarmason
2016-10-17 16:54 ` Junio C Hamano
2016-10-17 19:46 ` Ævar Arnfjörð Bjarmason
2016-10-06 9:11 ` [PATCH v2 3/3] gitweb: Link to "git describe"'d commits in log messages Ævar Arnfjörð Bjarmason
2016-10-06 19:51 ` Junio C Hamano [this message]
2016-10-09 11:20 ` Ævar Arnfjörð Bjarmason
2016-10-14 17:50 ` Ævar Arnfjörð Bjarmason
2016-10-14 19:00 ` Junio C Hamano
2016-10-14 20:06 ` Jakub Narębski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqy421i7dg.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=avarab@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jnareb@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox