Re: [PATCH] gitweb: ref markers link to named shortlogs

From: Jakub Narebski <jnareb@gmail.com>
Date: Aug 22, 2008, 08:49 UTC
Message-ID: <200808221049.21337.jnareb@gmail.com>
In-Reply-To: <cb7bb73a0808220021w5d30d3c8i88ad85611035d2c5@mail.gmail.com>

On Fri, 22 Aug 2008, Giuseppe Bilotta wrote:

Show 15 quoted lines

>>> --- a/gitweb/gitweb.css
>>> +++ b/gitweb/gitweb.css
>>
>>> +span.refs span a {
>>> +     text-decoration: none;
>>> +     color: inherit;
>>> +}
>>
>> Possible improvement:
>>
>> We would probably want to make this link discoverable, by adding
>> underline on :hover, like for other "hidden links" in gitweb (for
>> example in commitdiff view).
> 
> Can do that.

Additional idea: it would be nice to know if clicking on ref marker
would lead us to 'shortlog' view, or to 'tag' view; so perhaps we should
distinguish somehow indirect refs, for example using bold font-weight.

Show 28 quoted lines

>>>                       my ($type, $name) = qw();
>>> +                     my $git_type = git_get_type($ref);
>>>                       # e.g. tags/v2.6.11 or heads/next
>>>                       if ($ref =~ m!^(.*?)s?/(.*)$!) {
>>>                               $type = $1;
>>
>> git_get_type calls 'git cat-file -t', so for each ref shown you make
>> *additional call* to git command (additional fork).  Not good, especially
>> that you can get information if a ref is a tag (indirect reference)
>> or not one can get from within git_get_references; which in turn
>> uses "git show-refs --dereference" and used to use either
>> "git peek-remote ." or ".git/info/refs" file.  If there is <name>^{},
>> then <name> is indirect reference: is a tag.
>>
>> As we display ref markers only for log-like views, marker can be tag
>> or can be "lightweight reference" and be only a commit (in theory
>> we could show ref markers also for tree and blob items, but it is not
>> important now).
> 
> By looking at git_get_reference() what I see is basically the use of
> the same field as $type in format_ref_marker(). I can probably use
> that, although it means that any future extensions to ref marker
> display will need to hack the routine too. (This would mean that the
> patch would be more similar to my original patch
> http://marc.info/?l=git&m=121769155017642&w=2 ).
> 
> If this is not what you're suggesting, then I'm afraid I don't fully
> grasp your idea.

No, that is not what I was suggesting.

What format_ref_marker() uses is not exactly 'type' of reference, but more 'kind of' reference. It is based on reference namespace, not on type of object the reference is at (points to). So code based on this info (like your v3 patch) would fail on lightweight tag, i.e. if there is ref in 'refs/tags' namespace which points directly to commit, and not to tag object.

But 'git show-ref --dereference' _has_ information about whether given reference points directly or indirectly to given object ($refs->{$id}), but currently we neither save it, nor use it. For example we can have:

  781c1834f5419bdf81bb7f3750170ccd6b809174 refs/heads/maint
  ...
  124c62e8781a8f03ee0256bee78f7b392e3920af refs/stash
  ...
  89e6fcde639d65823e8113c307067441701ac74f refs/tags/Attic/gitweb/parse_rev_list
  b69a41a384d19fe253b9f4f34c9019ad96ca571d refs/tags/Attic/gitweb/patchset_body
  781c1834f5419bdf81bb7f3750170ccd6b809174 refs/tags/TEMP
  ...
  07cca3b30ee2b5d060e44e5b18d7c22929c63d1a refs/tags/v1.5.6.5
  781c1834f5419bdf81bb7f3750170ccd6b809174 refs/tags/v1.5.6.5^{}

Now in this example we have three refs pointing to commit object 781c1834: refs/heads/maint, refs/tags/TEMP and refs/tags/v1.5.6.5. From those only refs/tags/v1.5.6.5 is (via) tag, even though TEMP is in tags namespace. Currently git_get_references() strips '^{}' indirect reference marker from the output (from refname), and doesn't make use of it. One solution would be to not stip it in git_get_references(), but leave it, and strip it and make use of it (if ref ends with '^{}' it must be tag object) in format_ref_marker().

But that is just a proposal...

-- 
Jakub Narebski
Poland