Re: [PATCH 3/3] gitweb: Add support to Link: tag

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 4, 2012 at 10:24 AM, Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
> Hi,
>
> On Wed, Jul 4, 2012 at 5:15 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Namhyung Kim <namhyung@xxxxxxxxxx> writes:
>>
>>> The tip tree is the one of major subsystem tree in the
>>> Linux kernel project. On the tip tree, the Link: tag is
>>> used for tracking the original discussion or context.
>>> Since it's ususally in the s-o-b area, it'd be better
>>> using same style with others.
>>>
>>> Also as it tends to contain a message-id sent from git
>>> send-email, a part of the line which has more than 8
>>> (hex-)digit characters would set a wrong hyperlink
>>> like [1]. Fix it by not using format_log_line_html().
>>>
>>> [1] git.kernel.org/?p=linux/kernel/git/tip/tip.git;a=commit;h=08942f6d5d992e9486b07653fd87ea8182a22fa0
>>>
>>> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
>>> ---
>>>  gitweb/gitweb.perl |    7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>>> index e0701af..d07bcb7 100755
>>> --- a/gitweb/gitweb.perl
>>> +++ b/gitweb/gitweb.perl
>>> @@ -4493,6 +4493,13 @@ sub git_print_log {
>>>                               print "<span class=\"signoff\">" . esc_html($line) . "</span><br/>\n";
>>>                       }
>>>                       next;
>>> +             } elsif ($line =~ m,^ *link[ :](http://[\w/~.@%&=?+-]*),i) {
>>
>> Hrm, I am somewhat confused.  This catches "link:http://..."; and
>> "link http://...";, but not "link: http://...";, which looks a lot
>> more natural looking at least to me.
>>
>
> Oops, right. Actually I found it after some local testing,
> but forgot to update the patch. Sorry :)
>
>
>> Looking at a random sample:
>>
>> http://git.kernel.org/?p=linux/kernel/git/tip/tip.git;a=commit;h=fe85227347738eb9b871bc163e7fb0db8b6cd2a0
>>
>> I see a "Buglink: " which I think deserves to be handled by this patch
>> but would not.  Probably the pattern needs to be loosened
>> sufficiently, e.g.
>>
>>         m,^\s*[a-z]*link: (https?://\S+),i
>>
>> to catch it as well.  Note that I am rejecting space before ":" and
>> requiring a space after ":" in the above.
>>
>
> I think 'l' in 'link' should be '[Ll]'. Otherwise looks good to me.
>

Oops (again), I missed 'i' at the end.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]