Joey Hess <joey@xxxxxxxxxxx> writes: > 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/) > > 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. I think having LINK elements also for 'summary' page would be a good idea. This microformat is I think mainly for machines, and machines can I guess read better a few LINK elements in fairly small HEAD of page, than scan all of many link (A) elements on the page for those matching vcs-* microformat. Beside I am not sure if for example hyperlinking SCP-style repository URL makes sense at all; I am also not sure if hyperlinking links on which you cannot click on makes good sense (unless you use SPAN or ABBR instead of A to mark repo links...) > > 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. Also it is not necessary: if some tool want to get repo links for given project, it can get 'summary' page; if some tool want to get list of all repos, it can access one of projects list actions. > > 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. By the way, do you have any benchmarks for that? > > 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. I would also add that, from what I understand, you have made git_get_project_url_list() subroutine to be self-sufficient: it now considers both per-repository configuration (gitweb.url in config, cloneurl file in $GIT_DIR) and global gitweb configuration (@git_base_url_list variable). Simplification of code so it always return list and does nto check contents is a side issue, orthogonal to issue mentioned above. > > Signed-off-by: Joey Hess <joey@xxxxxxxxxxxxxxx> > --- > gitweb/gitweb.perl | 78 +++++++++++++++++++++++++++++++++++++++++---------- > 1 files changed, 62 insertions(+), 16 deletions(-) > > This incorporates Giuseppe Bilotta's feedback, and uses new features > of the microformat. You can see this version running at > http://git.ikiwiki.info/ > > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > index 99f71b4..c238717 100755 > --- a/gitweb/gitweb.perl > +++ b/gitweb/gitweb.perl > @@ -2020,9 +2020,14 @@ sub git_get_path_by_hash { > ## ...................................................................... > ## git utility functions, directly accessing git repository > > +{ > +my %project_descriptions; # cache > + Won't we get warnings (and perhaps errors) from mod_perl? Shouldn't this be "our %project_descriptions;"? > sub git_get_project_description { > my $path = shift; > > + return $project_descriptions{$path} if exists $project_descriptions{$path}; > + > $git_dir = "$projectroot/$path"; > open my $fd, "$git_dir/description" > or return git_get_project_config('description'); > @@ -2031,7 +2036,9 @@ sub git_get_project_description { > if (defined $descr) { > chomp $descr; > } > - return $descr; > + return $project_descriptions{$path}=$descr; > +} > + > } If we use 'title="$project git repository" for 'rel="vcs-git"' links, is it still worth it extra complication to avoid double calculation of project description in the case of 'summary' view for a project? Because IIRC for 'projects_list' view it is already cached in @projects list as 'descr' key... > > sub git_get_project_ctags { > @@ -2099,18 +2106,30 @@ sub git_show_project_tagcloud { > } > } > > +{ > +my %project_url_lists; # cache > + Same question: would it work correctly for mod_perl? > sub git_get_project_url_list { > + # use per project git URL list in $projectroot/$path/cloneurl > + # or make project git URL from git base URL and project name > my $path = shift; > > + return @{$project_url_lists{$path}} if exists $project_url_lists{$path}; > + > + my @ret; > $git_dir = "$projectroot/$path"; > - open my $fd, "$git_dir/cloneurl" > - or return wantarray ? > - @{ config_to_multi(git_get_project_config('url')) } : > - config_to_multi(git_get_project_config('url')); > - my @git_project_url_list = map { chomp; $_ } <$fd>; > - close $fd; > + if (open my $fd, "$git_dir/cloneurl") { > + @ret = map { chomp; $_ } <$fd>; > + close $fd; > + } else { > + @ret = @{ config_to_multi(git_get_project_config('url')) }; > + } > + @ret=map { "$_/$project" } @git_base_url_list if ! @ret; Style: + @ret = map { "$_/$project" } @git_base_url_list if !@ret; or even + @ret = map { "$_/$project" } @git_base_url_list unless @ret; > + > + $project_url_lists{$path}=\@ret; > + return @ret; > +} > > - return wantarray ? @git_project_url_list : \@git_project_url_list; > } Again: is it worth caching? It is only for 'summary'; for 'projects_list' it might be better to extend @projects list instead > > sub git_get_projects_list { > @@ -2856,6 +2875,7 @@ sub blob_contenttype { > sub git_header_html { > my $status = shift || "200 OK"; > my $expires = shift; > + my $extraheader = shift; > > my $title = "$site_name"; > if (defined $project) { > @@ -2953,6 +2973,8 @@ EOF > print qq(<link rel="shortcut icon" href="$favicon" type="image/png" />\n); > } > > + print $extraheader if defined $extraheader; > + > print "</head>\n" . > "<body>\n"; > Good solution, but shouldn't this be better put into separate commit, simply extending git_header_html to allow to add extra data (no need to name it $extraheader I think, $extra would be enough) to the HTML header (HEAD element contents)? > @@ -4365,6 +4387,26 @@ sub git_search_grep_body { > print "</table>\n"; > } > > +sub git_link_title { > + my $project=shift; > + > + my $description=git_get_project_description($project); > + return $project.(length $description ? " - $description" : ""); > +} Style (whitespace around '='), and the fact that IMHO "$project git repository" is better than "$project - $description", also because of "Unnamed repository; edit this file to name it for gitweb." default template > + > +# generates header with links to the specified projects > +sub git_links_header { Good abstraction, but I'm not so sure about subroutine name. > + my $ret=''; > + foreach my $project (@_) { Style: I'd rather use named variables, like "my @projects = @_"; also everywhere else we use spaces around '=' usually. > + # rel=vcs-* microformat > + my $title=git_link_title($project); Good abstraction. > + foreach my $url git_get_project_url_list($project) { > + $ret.=qq{<link rel="vcs-git" href="$url" title="$title"/>\n} To be HTML compatibile, it is better to use > + $ret.=qq{<link rel="vcs-git" href="$url" title="$title" />\n} (note the space before "/>"). > + } > + } > + return $ret; > +} > + > ## ====================================================================== > ## ====================================================================== > ## 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); > git_print_page_nav('',''); > git_print_header_div('summary', "$project forks"); > git_project_list_body(\@list, $order); > @@ -4468,14 +4514,14 @@ sub git_summary { > print "<tr id=\"metadata_lchange\"><td>last change</td><td>$cd{'rfc2822'}</td></tr>\n"; > } > > - # use per project git URL list in $projectroot/$project/cloneurl > - # or make project git URL from git base URL and project name > my $url_tag = "URL"; > - my @url_list = git_get_project_url_list($project); > - @url_list = map { "$_/$project" } @git_base_url_list unless @url_list; > - foreach my $git_url (@url_list) { > + my $title=git_link_title($project); > + foreach my $git_url (git_get_project_url_list($project)) { > next unless $git_url; > - print "<tr class=\"metadata_url\"><td>$url_tag</td><td>$git_url</td></tr>\n"; > + print "<tr class=\"metadata_url\"><td>$url_tag</td><td>". > + # rel=vcs-* microformat > + "<a rel=\"vcs-git\" href=\"$git_url\" title=\"$title\">$git_url</a>". > + "</td></tr>\n"; > $url_tag = ""; > } Non clickable hyperlink... hmmm... > > -- > 1.5.6.5 > > > > -- > see shy jo -- Jakub Narebski Poland ShadeHawk on #git -- 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
