Re: [PATCH] gitweb: support the rel=vcs-* microformat

> On Thursday 08 January 2009 00:24, Joey Hess wrote:
> 
> > The rel=vcs-* microformat allows a web page to indicate the locations of
> > repositories related to it in a machine-parseable manner.
> > (See http://kitenet.net/~joey/rfc/rel-vcs/)
> 
> Have you considered submitting the microformat to microformats.org?
> That would make the microformat more official and would be an good
> first step to have wider coverage of it, and additional reviews.

> 
> > Make gitweb use the microformat if it has been configured with project url
> > information in any of the usual ways. On the project summary page, the
> > repository URL display is simply marked up using the microformat. On the
> > project list page and forks list page, the microformat is embedded in the
> > header, since the URLs do not appear on the page.
> > 
> > The microformat could be included on other pages too, but I've skipped
> > doing so for now, since it would mean reading another file for every page
> > displayed.
> > 
> > There is a small overhead in including the microformat on project list
> > and forks list pages, but getting the project descriptions for those pages
> > already incurs a similar overhead, and the ability to get every repo url
> > in one place seems worthwhile.
> 
> I agree with this, although people with very large project lists may
> differ ... do we have timings on these?

>  
> > This changes git_get_project_url_list() to not check wantarray, and only
> > return in list context -- the only way it is used AFAICS. It memoizes
> > both that function and git_get_project_description(), to avoid redundant
> > file reads.
> 
> You may want to consider splitting the patch into three: memoizing
> of git_get_project_description(), reworking of
> git_get_project_url_list(), and the actual rel=vc-* insertions.

> >  sub git_get_project_description {
> >       my $path = shift;
> >  
> > +     return $project_descriptions{$path} if exists $project_descriptions{$path};
> > +
> 
> This line is bordering on the 80 characters, so you may want to
> consider moving 'my $descr' here, with something such as
> 
> my $descr = $project_descriptions{$path};
> return $descr if exists $descr;
> 
> Also, I'm no perl guru so I'm not sure about exists vs defined here.

> >  ## ======================================================================
> >  ## ======================================================================
> >  ## actions
> > @@ -4380,7 +4422,9 @@ sub git_project_list {
> >               die_error(404, "No projects found");
> >       }
> >  
> > -     git_header_html();
> > +     my $extraheader=git_links_header(map { $_->{path} } @list);
> > +
> > +     git_header_html(undef, undef, $extraheader);
> >       if (-f $home_text) {
> >               print "<div class=\"index_include\">\n";
> >               insert_file($home_text);
> > @@ -4405,8 +4449,10 @@ sub git_forks {
> >       if (!@list) {
> >               die_error(404, "No forks found");
> >       }
> > +     
> > +     my $extraheader=git_links_header(map { $_->{path} } @list);
> >  
> > -     git_header_html();
> > +     git_header_html(undef, undef, $extraheader);
> 
> This makes me wonder if it would be worth it to turn git_header_html
> into -param => value style, but I'm not really sure it's worth it.