gitweb: cloud tags feature produces malformed XML for errors

gitweb: cloud tags feature produces malformed XML for errors

[Jonathan Nieder]

(resending, censored for vger; sorry for the noise)
Hi Jakub et al,

Uwe Kleine-König wrote [1]:

> I experiment with $feature{'ctags'}.  After installing
> libhtml-tagcloud-perl (0.34-1) and adding
>
>	$feature{'ctags'}{'default'} = [1];
>
> to gitweb.conf and doing
>
> 	mkdir ctags
> 	echo Linux > ctags/Linux
>
> in the only repository served by gitweb makes iceweasel barf on the
> output (see attachment).

With chromium I get:

	This page contains the following errors:

	error on line 26 at column 6: XML declaration allowed only at the start of the document
	Below is a rendering of the page up to the first error.

First, we hit

	if ($show_ctags) {
		my %ctags;
		foreach my $p (@projects) {
			foreach my $ct (keys %{$p->{'ctags'}}) {
				$ctags{$ct} += $p->{'ctags'}->{$ct};

which produces a warning warning in error.log:

	index.cgi: Argument "Linux" isn't numeric in addition (+) at /usr/share/gitweb/index.cgi line 4819.

in error.log.  Then we hit git_show_project_tagcloud, which dies in
"$cloud->html_and_css($count);" with

	HTML::TagCloud..
	index.cgi: Can't take log of 0 at /usr/share/perl5/HTML/TagCloud.pm line 67.

For some reason, the result is an _embedded_ error page:

	<form method="get" action="/gitweb/" enctype="application/x-www-form-urlencoded"> 
	<p class="projsearch">Search:
	<input type="text" name="s"  /> 
	</p> 
	</form> 
	Content-type: {a content type which shall not be named}
	 
	<?xml version="1.0" encoding="utf-8"?> 
 [...]
	500 - Internal Server Error

So I suspect there are two bugs here.

Ideas?
Jonathan

[1] http://bugs.debian.org/616005

Re: gitweb: cloud tags feature produces malformed XML for errors

[Jakub Narebski]

On Tue, 1 Mar 2011, Jonathan Nieder wrote:
> Uwe Kleine-König wrote [1]:
> [1] http://bugs.debian.org/616005

> > I experiment with $feature{'ctags'}.  After installing
> > libhtml-tagcloud-perl (0.34-1) and adding
> >
> >	$feature{'ctags'}{'default'} = [1];
> >
> > to gitweb.conf and doing
> >
> > 	mkdir ctags
> > 	echo Linux > ctags/Linux

This is invalid usage.  See below for details.

> >
> > in the only repository served by gitweb makes iceweasel barf on the
> > output (see attachment).
> 
> With chromium I get:
> 
> 	This page contains the following errors:
> 
> 	error on line 26 at column 6: XML declaration allowed only at the start of the document
> 	Below is a rendering of the page up to the first error.
> 
> First, we hit
> 
> 	if ($show_ctags) {
> 		my %ctags;
> 		foreach my $p (@projects) {
> 			foreach my $ct (keys %{$p->{'ctags'}}) {
> 				$ctags{$ct} += $p->{'ctags'}->{$ct};
> 
> which produces a warning warning in error.log:
> 
> 	index.cgi: Argument "Linux" isn't numeric in addition (+) at /usr/share/gitweb/index.cgi line 4819.
> 
> in error.log.

Well, blame Pasky for not describing 'ctags' in more detail.  Value of
tag is its weight, so instead of

 	mkdir ctags
	echo Linux > ctags/Linux

one should use

 	mkdir ctags
	echo 1 > ctags/Linux

Admittedly gitweb should be more defensive about invalid input...

> Then we hit git_show_project_tagcloud, which dies in 
> "$cloud->html_and_css($count);" with
> 
> 	HTML::TagCloud..
> 	index.cgi: Can't take log of 0 at /usr/share/perl5/HTML/TagCloud.pm line 67.
> 
> For some reason, the result is an _embedded_ error page:
> 
> 	<form method="get" action="/gitweb/" enctype="application/x-www-form-urlencoded"> 
> 	<p class="projsearch">Search:
> 	<input type="text" name="s"  /> 
> 	</p> 
> 	</form> 
> 	Content-type: {a content type which shall not be named}
> 	 
> 	<?xml version="1.0" encoding="utf-8"?> 
>  [...]
> 	500 - Internal Server Error
> 
> So I suspect there are two bugs here.

This I think is caused by the fact that error ("die") occurs after gitweb
have send some output to web browser already.  That would be harder to fix.


-- 
Jakub Narebski
Poland

Re: gitweb: cloud tags feature produces malformed XML for errors

[Jakub Narebski]

On Wed, 2 Mar 2011, Jakub Narebski wrote:
> On Tue, 1 Mar 2011, Jonathan Nieder wrote:
> > Uwe Kleine-König wrote [1]:
> > [1] http://bugs.debian.org/616005
> 
> > > I experiment with $feature{'ctags'}.  After installing
> > > libhtml-tagcloud-perl (0.34-1) and adding
> > >
> > >	$feature{'ctags'}{'default'} = [1];

This is also invalid... but this one actually is described:

 # gitweb by itself can show existing tags, but it does not handle
 # tagging itself; you need an external application for that.
 # For an example script, check Girocco's cgi/tagproj.cgi.
 # You may want to install the HTML::TagCloud Perl module to get
 # a pretty tag cloud instead of just a list of tags.

 # To enable system wide have in $GITWEB_CONFIG
 # $feature{'ctags'}{'default'} = ['path_to_tag_script'];
 # Project specific override is not supported.

Using "$feature{'ctags'}{'default'} = [1];" would lead to errors when 
you would want to create a tag from web interface.

-- 
Jakub Narebski
Poland

Re: gitweb: cloud tags feature produces malformed XML for errors

[Uwe Kleine-König]

On Wed, Mar 02, 2011 at 09:24:28AM +0100, Jakub Narebski wrote:
> On Wed, 2 Mar 2011, Jakub Narebski wrote:
> > On Tue, 1 Mar 2011, Jonathan Nieder wrote:
> > > Uwe Kleine-König wrote [1]:
> > > [1] http://bugs.debian.org/616005
> > 
> > > > I experiment with $feature{'ctags'}.  After installing
> > > > libhtml-tagcloud-perl (0.34-1) and adding
> > > >
> > > >	$feature{'ctags'}{'default'} = [1];
> 
> This is also invalid... but this one actually is described:
> 
>  # gitweb by itself can show existing tags, but it does not handle
>  # tagging itself; you need an external application for that.
>  # For an example script, check Girocco's cgi/tagproj.cgi.
>  # You may want to install the HTML::TagCloud Perl module to get
>  # a pretty tag cloud instead of just a list of tags.
> 
>  # To enable system wide have in $GITWEB_CONFIG
>  # $feature{'ctags'}{'default'} = ['path_to_tag_script'];
>  # Project specific override is not supported.
> 
> Using "$feature{'ctags'}{'default'} = [1];" would lead to errors when 
> you would want to create a tag from web interface.
Yeah, I was aware of that (but of course I should have written that).  I
also tried ... = ['/bin/sh'] and a few others.  Obviously this is
pre-production :-)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: gitweb: cloud tags feature produces malformed XML for errors

[J.H.]

On 03/01/2011 02:21 PM, Jonathan Nieder wrote:
> (resending, censored for vger; sorry for the noise)
> Hi Jakub et al,
> 
> Uwe Kleine-König wrote [1]:
> 
>> I experiment with $feature{'ctags'}.  After installing
>> libhtml-tagcloud-perl (0.34-1) and adding
>>
>> 	$feature{'ctags'}{'default'} = [1];
>>
>> to gitweb.conf and doing
>>
>> 	mkdir ctags
>> 	echo Linux > ctags/Linux
>>
>> in the only repository served by gitweb makes iceweasel barf on the
>> output (see attachment).
> 
> With chromium I get:
> 
> 	This page contains the following errors:
> 
> 	error on line 26 at column 6: XML declaration allowed only at the start of the document
> 	Below is a rendering of the page up to the first error.
> 
> First, we hit
> 
> 	if ($show_ctags) {
> 		my %ctags;
> 		foreach my $p (@projects) {
> 			foreach my $ct (keys %{$p->{'ctags'}}) {
> 				$ctags{$ct} += $p->{'ctags'}->{$ct};
> 
> which produces a warning warning in error.log:
> 
> 	index.cgi: Argument "Linux" isn't numeric in addition (+) at /usr/share/gitweb/index.cgi line 4819.
> 
> in error.log.  Then we hit git_show_project_tagcloud, which dies in
> "$cloud->html_and_css($count);" with
> 
> 	HTML::TagCloud..
> 	index.cgi: Can't take log of 0 at /usr/share/perl5/HTML/TagCloud.pm line 67.
> 
> For some reason, the result is an _embedded_ error page:
> 
> 	<form method="get" action="/gitweb/" enctype="application/x-www-form-urlencoded"> 
> 	<p class="projsearch">Search:
> 	<input type="text" name="s"  /> 
> 	</p> 
> 	</form> 
> 	Content-type: {a content type which shall not be named}
> 	 
> 	<?xml version="1.0" encoding="utf-8"?> 
>  [...]
> 	500 - Internal Server Error
> 
> So I suspect there are two bugs here.
> 

As a general note the cloud tags code that's there has a whole slew of
problems, including a pretty trivial way to crash gitweb entirely if the
tags file is malformed in any way to what it's expecting.

I've said it before, I'd rather see that code reverted or completely
overhauled, and as far as I know repo.or.cz is the only place even using
the code at all.

I also think I reported this particular error a couple of years ago
(with the file being malformed and causing all kinds of problems).  The
fact that the tags don't follow the repository when it's cloned more or
less makes this entire feature a giant PITA, couple that with no good
documentation on how to create the tag files (I think the file itself
needs to have filename of tag, and *ONLY* contain 1 for it's contains
for it to work)

Linux:
1
EOF

- John 'Warthog9' Hawley

Re: gitweb: cloud tags feature produces malformed XML for errors

[Jakub Narebski]

On Wed, 2 Mar 2011 02:06, J.H. wrote:

> As a general note the cloud tags code that's there has a whole slew of
> problems, including a pretty trivial way to crash gitweb entirely if the
> tags file is malformed in any way to what it's expecting.

That's not a huge problem, I think; it would be enough to add some
validation to parsing ctags code (e.g. treat any contents that is
not a single number as a 1).
 
> I've said it before, I'd rather see that code reverted or completely
> overhauled, and as far as I know repo.or.cz is the only place even using
> the code at all.
> 
> I also think I reported this particular error a couple of years ago
> (with the file being malformed and causing all kinds of problems).  The
> fact that the tags don't follow the repository when it's cloned more or
> less makes this entire feature a giant PITA, couple that with no good
> documentation on how to create the tag files (I think the file itself
> needs to have filename of tag, and *ONLY* contain 1 for it's contains
> for it to work)
> 
> Linux:
> 1
> EOF

Well, neither does README.html follow the repository when it is cloned,
and usually neither does description ('description' file is generated by
git default template, and description file overrides `gitweb.description'
config variable).

Documentation always can be added, either as comments for the 'ctags'
feature, or in gitweb/README (or, in the future, in gitweb manpage).


What is most important that makes this feature to be considered for
removal (or rehauling) is that only half of this feature is implemented
in gitweb: the displaying part.  There is half-attempt of providing
some web interface for managing tags... which needs external script with
strict coupling, doesn't offer any access control as far as I know, do
not allow deleting tags, etc.

-- 
Jakub Narebski
Poland

Re: gitweb: cloud tags feature produces malformed XML for errors

[Uwe Kleine-König]

Hello Jakub,

On Wed, Mar 02, 2011 at 10:18:44PM +0100, Jakub Narebski wrote:
> What is most important that makes this feature to be considered for
> removal (or rehauling) is that only half of this feature is implemented
> in gitweb: the displaying part.  There is half-attempt of providing
> some web interface for managing tags... which needs external script with
> strict coupling, doesn't offer any access control as far as I know, do
> not allow deleting tags, etc.
For a small set of repositories the need to hand-edit the tags is OK
IMHO.  That's what I intended to do.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: gitweb: cloud tags feature produces malformed XML for errors

[Jakub Narebski]

On Wed, 2 Mar 2011, Uwe Kleine-König wrote:
> On Wed, Mar 02, 2011 at 10:18:44PM +0100, Jakub Narebski wrote:

> > What is most important that makes this feature to be considered for
> > removal (or rehauling) is that only half of this feature is implemented
> > in gitweb: the displaying part.  There is half-attempt of providing
> > some web interface for managing tags... which needs external script with
> > strict coupling, doesn't offer any access control as far as I know, do
> > not allow deleting tags, etc.
>
> For a small set of repositories the need to hand-edit the tags is OK
> IMHO.  That's what I intended to do.

So what would you like to see?

1. Hardening parsing of ctags files, so that gitweb does not crash on
   malformed entries, but e.g. just ignores them.

2. Generating tag cloud upfront, before sending any output to browser,
   to catch error better (and perhaps separate CSS for HTML::TagCloud).

3. Describe format of ctags files, either in comments in code, or in
   gitweb/README.

4. Either:

   A. Remove editing ctags from gitweb, or
   B. Add some simple generation of ctags file to gitweb

or should we remove ctags feature altogether?
-- 
Jakub Narebski
Poland

Re: gitweb: cloud tags feature produces malformed XML for errors

[Uwe Kleine-König]

Hello Jakub,

On Thu, Mar 03, 2011 at 01:42:15AM +0100, Jakub Narebski wrote:
> On Wed, 2 Mar 2011, Uwe Kleine-König wrote:
> > On Wed, Mar 02, 2011 at 10:18:44PM +0100, Jakub Narebski wrote:
> 
> > > What is most important that makes this feature to be considered for
> > > removal (or rehauling) is that only half of this feature is implemented
> > > in gitweb: the displaying part.  There is half-attempt of providing
> > > some web interface for managing tags... which needs external script with
> > > strict coupling, doesn't offer any access control as far as I know, do
> > > not allow deleting tags, etc.
> >
> > For a small set of repositories the need to hand-edit the tags is OK
> > IMHO.  That's what I intended to do.
> 
> So what would you like to see?
> 
> 1. Hardening parsing of ctags files, so that gitweb does not crash on
>    malformed entries, but e.g. just ignores them.
> 
> 2. Generating tag cloud upfront, before sending any output to browser,
>    to catch error better (and perhaps separate CSS for HTML::TagCloud).
> 
> 3. Describe format of ctags files, either in comments in code, or in
>    gitweb/README.
> 
> 4. Either:
> 
>    A. Remove editing ctags from gitweb, or
>    B. Add some simple generation of ctags file to gitweb
> 
yes :-)  (1-3, 4a)  I wouldn't mind 4b, but it should be possible to
disable the possibility to publically edit tags via gitweb.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[RFC/PATCH] gitweb: Change the way "content tags" ('ctags') are handled

[Jakub Narebski]

On Thu, 3 March 2011, Uwe Kleine-König wrote:
> On Thu, Mar 03, 2011 at 01:42:15AM +0100, Jakub Narebski wrote:
>> On Wed, 2 Mar 2011, Uwe Kleine-König wrote:
>>> On Wed, Mar 02, 2011 at 10:18:44PM +0100, Jakub Narebski wrote:

>>>> What is most important that makes this feature to be considered for
>>>> removal (or rehauling) is that only half of this feature is implemented
>>>> in gitweb: the displaying part.  There is half-attempt of providing
>>>> some web interface for managing tags... which needs external script with
>>>> strict coupling, doesn't offer any access control as far as I know, do
>>>> not allow deleting tags, etc.
>>>
>>> For a small set of repositories the need to hand-edit the tags is OK
>>> IMHO.  That's what I intended to do.
>> 
>> So what would you like to see?
>> 
>> 1. Hardening parsing of ctags files, so that gitweb does not crash on
>>    malformed entries, but e.g. just ignores them.

Done.
 
>> 2. Generating tag cloud upfront, before sending any output to browser,
>>    to catch error better (and perhaps separate CSS for HTML::TagCloud).

Not done, but with 1. it should be not very necessary... unless another
bug is found in related code, that is.

>> 3. Describe format of ctags files, either in comments in code, or in
>>    gitweb/README.

Done (comments in code).

>> 4. Either:
>> 
>>    A. Remove editing ctags from gitweb, or
>>    B. Add some simple generation of ctags file to gitweb

Done 4.A.

> yes :-)  (1-3, 4a)  I wouldn't mind 4b, but it should be possible to
> disable the possibility to publically edit tags via gitweb.


The patch below (addressing those issues) is based on

  "[PATCHv2/RFC] gitweb: Restructure projects list generation"
  http://thread.gmane.org/gmane.comp.version-control.git/167996/focus=168321
  http://repo.or.cz/w/git/jnareb-git.git/commit/420071752d13dcecd59e794d82285e7e142ef75f
  ('gitweb/web' branch in http://repo.or.cz/w/git/jnareb-git.git repository)

-- >8 --
Subject: [RFC/PATCH] gitweb: Change the way "content tags" ('ctags') are handled

The major change is removing the ability to edit content tags (ctags)
in a web browser.

The interface was created by gitweb, while actual editing of tags was
to be done by external script; the API was not defined, and neither
was provided example implementation.  Such split is also a bit fragile
- interface and implementation have to be kept in sync.  Gitweb
provided only ability to add tags; you could not edit tags nor delete
them.


Format of ctags is now described in the comment above git_get_project_ctags
subroutine.  Gitweb now is more robust with respect to original ctags
format; it also accepts two new formats: $GIT_DIR/ctags file, with one
content tag per line, and multi-value `gitweb.ctag' config variable.

Gathering all ctags of all project is now put in git_gather_all_ctags
subroutine, making git_project_list_body more clear.

git_populate_project_tagcloud subroutine now generates data used for
tag cloud, including generation of ctag link, also in the case
HTML::TagCloud module is unavailable.  Links are now generated using
href() subroutine - this is more robust, as ctags might contain '?',
';' and '=' special characters that need to be escaped in query param.
Shown tags are HTML-escaped.

The generation of tag cloud in git_show_project_tagcloud in the case
when HTML::TagCloud is not available is now changed slightly.

The 'content tags' field on project summary page is made more in line
with other fields in "projects_list" table.  Because one cannot now
add new tags from web interface, this field is no longer displayed
when there are no content tags for given project.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |  141 +++++++++++++++++++++++++++++++++++----------------
 1 files changed, 97 insertions(+), 44 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 996b647..4d80818 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -412,20 +412,23 @@ our %feature = (
 		'override' => 0,
 		'default' => []},
 
-	# Allow gitweb scan project content tags described in ctags/
-	# of project repository, and display the popular Web 2.0-ish
-	# "tag cloud" near the project list. Note that this is something
-	# COMPLETELY different from the normal Git tags.
+	# Allow gitweb scan project content tags of project repository,
+	# and display the popular Web 2.0-ish "tag cloud" near the projects
+	# list.  Note that this is something COMPLETELY different from the
+	# normal Git tags.
 
 	# gitweb by itself can show existing tags, but it does not handle
-	# tagging itself; you need an external application for that.
-	# For an example script, check Girocco's cgi/tagproj.cgi.
+	# tagging itself; you need to do it externally, outside gitweb.
+	# The format is described in git_get_project_ctags() subroutine.
 	# You may want to install the HTML::TagCloud Perl module to get
 	# a pretty tag cloud instead of just a list of tags.
 
 	# To enable system wide have in $GITWEB_CONFIG
-	# $feature{'ctags'}{'default'} = ['path_to_tag_script'];
+	# $feature{'ctags'}{'default'} = [1];
 	# Project specific override is not supported.
+
+	# In the future whether ctags editing is enabled might depend
+	# on the value, but using 1 should always mean no editing of ctags.
 	'ctags' => {
 		'override' => 0,
 		'default' => [0]},
@@ -703,6 +706,7 @@ our @cgi_param_mapping = (
 	snapshot_format => "sf",
 	extra_options => "opt",
 	search_use_regexp => "sr",
+	ctag => "by_tag",
 	# this must be last entry (for manipulation from JavaScript)
 	javascript => "js"
 );
@@ -2564,23 +2568,66 @@ sub git_get_project_description {
 	return $descr;
 }
 
+# supported formats:
+# * $GIT_DIR/ctags/<tagname> file (in 'ctags' subdirectory)
+#   - if its contents is a number, use it as tag weight,
+#   - otherwise add a tag with weight 1
+# * $GIT_DIR/ctags file, each line is a tag (with weight 1)
+#   the same value multiple times increases tag weight
+# * `gitweb.ctag' multi-valued repo config variable
 sub git_get_project_ctags {
-	my $path = shift;
+	my $project = shift;
 	my $ctags = {};
 
-	$git_dir = "$projectroot/$path";
-	opendir my $dh, "$git_dir/ctags"
-		or return $ctags;
-	foreach (grep { -f $_ } map { "$git_dir/ctags/$_" } readdir($dh)) {
-		open my $ct, '<', $_ or next;
-		my $val = <$ct>;
-		chomp $val;
-		close $ct;
-		my $ctag = $_; $ctag =~ s#.*/##;
-		$ctags->{$ctag} = $val;
+	$git_dir = "$projectroot/$project";
+	if (opendir my $dh, "$git_dir/ctags") {
+		my @files = grep { -f $_ } map { "$git_dir/ctags/$_" } readdir($dh);
+		foreach my $tagfile (@files) {
+			open my $ct, '<', $tagfile
+				or next;
+			my $val = <$ct>;
+			chomp $val if $val;
+			close $ct;
+
+			(my $ctag = $tagfile) =~ s#.*/##;
+			if ($val =~ /\d+/) {
+				$ctags->{$ctag} = $val;
+			} else {
+				$ctags->{$ctag} = 1;
+			}
+		}
+		closedir $dh;
+
+	} elsif (open my $fh, '<', "$git_dir/ctags") {
+		while (my $line = <$fh>) {
+			chomp $line;
+			$ctags->{$line}++ if $line;
+		}
+		close $fh;
+
+	} else {
+		my $taglist = config_to_multi(git_get_project_config('ctag'));
+		foreach my $tag (@$taglist) {
+			$ctags->{$tag}++;
+		}
 	}
-	closedir $dh;
-	$ctags;
+
+	return $ctags;
+}
+
+# return hash, where keys are content tags ('ctags'),
+# and values are sum of weights of given tag in every project
+sub git_gather_all_ctags {
+	my $projects = shift;
+	my $ctags = {};
+
+	foreach my $p (@$projects) {
+		foreach my $ct (keys %{$p->{'ctags'}}) {
+			$ctags->{$ct} += $p->{'ctags'}->{$ct};
+		}
+	}
+
+	return $ctags;
 }
 
 sub git_populate_project_tagcloud {
@@ -2600,31 +2647,41 @@ sub git_populate_project_tagcloud {
 	my $cloud;
 	if (eval { require HTML::TagCloud; 1; }) {
 		$cloud = HTML::TagCloud->new;
-		foreach (sort keys %ctags_lc) {
+		foreach my $ctag (sort keys %ctags_lc) {
 			# Pad the title with spaces so that the cloud looks
 			# less crammed.
-			my $title = $ctags_lc{$_}->{topname};
+			my $title = esc_html($ctags_lc{$ctag}->{topname});
 			$title =~ s/ /&nbsp;/g;
 			$title =~ s/^/&nbsp;/g;
 			$title =~ s/$/&nbsp;/g;
-			$cloud->add($title, $home_link."?by_tag=".$_, $ctags_lc{$_}->{count});
+			$cloud->add($title, href(project=>undef, ctag=>$ctag),
+			            $ctags_lc{$ctag}->{count});
 		}
 	} else {
-		$cloud = \%ctags_lc;
+		$cloud = {};
+		foreach my $ctag (keys %ctags_lc) {
+			my $title = $ctags_lc{$ctag}->{topname};
+			$cloud->{$ctag}{count} = $ctags_lc{$ctag}->{count};
+			$cloud->{$ctag}{ctag} =
+				$cgi->a({-href=>href(project=>undef, ctag=>$ctag)},
+			          esc_html($title, -nbsp=>1));
+		}
 	}
-	$cloud;
+	return $cloud;
 }
 
 sub git_show_project_tagcloud {
 	my ($cloud, $count) = @_;
-	print STDERR ref($cloud)."..\n";
 	if (ref $cloud eq 'HTML::TagCloud') {
 		return $cloud->html_and_css($count);
 	} else {
-		my @tags = sort { $cloud->{$a}->{count} <=> $cloud->{$b}->{count} } keys %$cloud;
-		return '<p align="center">' . join (', ', map {
-			$cgi->a({-href=>"$home_link?by_tag=$_"}, $cloud->{$_}->{topname})
-		} splice(@tags, 0, $count)) . '</p>';
+		my @tags = sort { $cloud->{$a}->{'count'} <=> $cloud->{$b}->{'count'} } keys %$cloud;
+		return
+			'<div id="htmltagcloud"'.($project ? '' : ' align="center"').'>' .
+			join (', ', map {
+				$cloud->{$_}->{'ctag'}
+			} splice(@tags, 0, $count)) .
+			'</div>';
 	}
 }
 
@@ -4905,13 +4962,8 @@ sub git_project_list_body {
 	@projects = sort_projects_list(\@projects, $order);
 
 	if ($show_ctags) {
-		my %ctags;
-		foreach my $p (@projects) {
-			foreach my $ct (keys %{$p->{'ctags'}}) {
-				$ctags{$ct} += $p->{'ctags'}->{$ct};
-			}
-		}
-		my $cloud = git_populate_project_tagcloud(\%ctags);
+		my $ctags = git_gather_all_ctags(\@projects);
+		my $cloud = git_populate_project_tagcloud($ctags);
 		print git_show_project_tagcloud($cloud, 64);
 	}
 
@@ -5507,13 +5559,14 @@ sub git_summary {
 	my $show_ctags = gitweb_check_feature('ctags');
 	if ($show_ctags) {
 		my $ctags = git_get_project_ctags($project);
-		my $cloud = git_populate_project_tagcloud($ctags);
-		print "<tr id=\"metadata_ctags\"><td>Content tags:<br />";
-		print "</td>\n<td>" unless %$ctags;
-		print "<form action=\"$show_ctags\" method=\"post\"><input type=\"hidden\" name=\"p\" value=\"$project\" />Add: <input type=\"text\" name=\"t\" size=\"8\" /></form>";
-		print "</td>\n<td>" if %$ctags;
-		print git_show_project_tagcloud($cloud, 48);
-		print "</td></tr>";
+		if (%$ctags) {
+			# without ability to add tags, don't show if there are none
+			my $cloud = git_populate_project_tagcloud($ctags);
+			print "<tr id=\"metadata_ctags\">" .
+			      "<td>content tags</td>" .
+			      "<td>".git_show_project_tagcloud($cloud, 48)."</td>" .
+			      "</tr>\n";
+		}
 	}
 
 	print "</table>\n";
-- 
1.7.3

[PATCH 2/1] gitweb: Mark matched 'ctag' / contents tag (?by_tag=foo)

[Jakub Narebski]

It might have been hard to discover that current view is limited to
projects with given content tag (ctag), as it was distinquished only
in gitweb URL.  Mark matched contents tag in the tag cloud using
"match" class, for easier discovery.

This commit introduces a bit of further code duplication in
git_populate_project_tagcloud().

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4d80818..7ba8a72 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2645,6 +2645,7 @@ sub git_populate_project_tagcloud {
 	}
 
 	my $cloud;
+	my $matched = $cgi->param('by_tag');
 	if (eval { require HTML::TagCloud; 1; }) {
 		$cloud = HTML::TagCloud->new;
 		foreach my $ctag (sort keys %ctags_lc) {
@@ -2654,17 +2655,22 @@ sub git_populate_project_tagcloud {
 			$title =~ s/ /&nbsp;/g;
 			$title =~ s/^/&nbsp;/g;
 			$title =~ s/$/&nbsp;/g;
+			if (defined $matched && $matched eq $ctag) {
+				$title = qq(<span class="match">$title</span>);
+			}
 			$cloud->add($title, href(project=>undef, ctag=>$ctag),
 			            $ctags_lc{$ctag}->{count});
 		}
 	} else {
 		$cloud = {};
 		foreach my $ctag (keys %ctags_lc) {
-			my $title = $ctags_lc{$ctag}->{topname};
+			my $title = esc_html($ctags_lc{$ctag}->{topname}, -nbsp=>1);
+			if (defined $matched && $matched eq $ctag) {
+				$title = qq(<span class="match">$title</span>);
+			}
 			$cloud->{$ctag}{count} = $ctags_lc{$ctag}->{count};
 			$cloud->{$ctag}{ctag} =
-				$cgi->a({-href=>href(project=>undef, ctag=>$ctag)},
-			          esc_html($title, -nbsp=>1));
+				$cgi->a({-href=>href(project=>undef, ctag=>$ctag)}, $title);
 		}
 	}
 	return $cloud;
-- 
1.7.3

Re: [PATCH 2/1] gitweb: Mark matched 'ctag' / contents tag (?by_tag=foo)

[Petr Baudis]

On Wed, Mar 09, 2011 at 03:04:59PM +0100, Jakub Narebski wrote:
> It might have been hard to discover that current view is limited to
> projects with given content tag (ctag), as it was distinquished only
> in gitweb URL.  Mark matched contents tag in the tag cloud using
> "match" class, for easier discovery.
> 
> This commit introduces a bit of further code duplication in
> git_populate_project_tagcloud().
> 
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>

Acked-by: Petr Baudis <pasky@suse.cz>

[PATCH] gitweb: do misparse nonnumeric content tag files that contain a digit

[Jonathan Nieder]

v1.7.6-rc0~27^2~4 (gitweb: Change the way "content tags" ('ctags') are
handled, 2011-04-29) tried to make gitweb's tag cloud feature more
intuitive for webmasters by checking whether the ctags/<label> under
a project's .git dir contains a number (representing the strength of
association to <label>) before treating it as one.

So after that change, after putting '$feature{'ctags'}{'default'} =
[1];' in your $GITWEB_CONFIG, you could do

	echo Linux >.git/ctags/linux

and gitweb would treat that as a request to tag the current repository
with the Linux tag, instead of the previous behavior of writing an
error page embedded in the projects list that triggers error messages
from Chromium and Firefox about malformed XML.

Unfortunately the pattern (\d+) used to match numbers is too loose,
and the "XML declaration allowed only at the start of the document"
error can still be experienced if you write "Linux-2.6" in place of
"Linux" in the example above.  Fix it by tightening the pattern to
^\d+$.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi,

Jakub Narebski wrote:
>> On Thu, Mar 03, 2011 at 01:42:15AM +0100, Jakub Narebski wrote:

>>> 1. Hardening parsing of ctags files, so that gitweb does not crash on
>>>    malformed entries, but e.g. just ignores them.
>
> Done.

Sorry for a (long-) delayed response.  Based on testing rc0 today, it
works well; thanks!  Patch to fix a small detail noticed while trying
'-1' follows.

 gitweb/gitweb.perl                     |    2 +-
 t/t9500-gitweb-standalone-no-errors.sh |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ebf2d1c..1b83a8d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2644,7 +2644,7 @@ sub git_get_project_ctags {
 			close $ct;
 
 			(my $ctag = $tagfile) =~ s#.*/##;
-			if ($val =~ /\d+/) {
+			if ($val =~ /^\d+$/) {
 				$ctags->{$ctag} = $val;
 			} else {
 				$ctags->{$ctag} = 1;
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index f5648a6..5329715 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -644,6 +644,14 @@ test_expect_success \
 	'ctags: search projects by non existent tag' \
 	'gitweb_run "by_tag=non-existent"'
 
+test_expect_success \
+	'ctags: malformed tag weights' \
+	'mkdir -p .git/ctags &&
+	 echo "not-a-number" > .git/ctags/nan &&
+	 echo "not-a-number-2" > .git/ctags/nan2 &&
+	 echo "0.1" >.git/ctags/floating-point &&
+	 gitweb_run'
+
 # ----------------------------------------------------------------------
 # categories
 
-- 
1.7.6.rc0

Re: [PATCH] gitweb: do misparse nonnumeric content tag files that contain a digit

[Jonathan Nieder]

Jonathan Nieder wrote:

> [Subject: [PATCH] gitweb: do misparse nonnumeric content tag files that contain a digit]

Sigh, it's only after sending that the obvious typos jump out.

"do" should be "do not" in the subject.  Sorry for the trouble.

Re: gitweb: cloud tags feature produces malformed XML for errors

[Petr Baudis]

On Wed, Mar 02, 2011 at 10:18:44PM +0100, Jakub Narebski wrote:
> Well, neither does README.html follow the repository when it is cloned,
> and usually neither does description ('description' file is generated by
> git default template, and description file overrides `gitweb.description'
> config variable).
> 
> Documentation always can be added, either as comments for the 'ctags'
> feature, or in gitweb/README (or, in the future, in gitweb manpage).
> 
> 
> What is most important that makes this feature to be considered for
> removal (or rehauling) is that only half of this feature is implemented
> in gitweb: the displaying part.  There is half-attempt of providing
> some web interface for managing tags... which needs external script with
> strict coupling, doesn't offer any access control as far as I know, do
> not allow deleting tags, etc.

Well, but your argument above also applies here - gitweb does not allow
users to modify README.html or desrciption either. Who knows where ctags
may come from, they might even be autogenerated based on something.
I admit that the code could be a bit more resilient to invalid contents
of the files, but I do not see that as a major problem either (it is
possible to confuse gitweb by putting invalid stuff into other
repository files too).

-- 
				Petr "Pasky" Baudis
Computer science education cannot make an expert programmer any more
than studying brushes and pigment can make an expert painter. --esr

Re: gitweb: cloud tags feature produces malformed XML for errors

[Jakub Narebski]

On Thu, 3 March 2011, Petr Baudis wrote:
> On Wed, Mar 02, 2011 at 10:18:44PM +0100, Jakub Narebski wrote:

> > Well, neither does README.html follow the repository when it is cloned,
> > and usually neither does description ('description' file is generated by
> > git default template, and description file overrides `gitweb.description'
> > config variable).
> > 
> > Documentation always can be added, either as comments for the 'ctags'
> > feature, or in gitweb/README (or, in the future, in gitweb manpage).
> > 
> > 
> > What is most important that makes this feature to be considered for
> > removal (or rehauling) is that only half of this feature is implemented
> > in gitweb: the displaying part.  There is half-attempt of providing
> > some web interface for managing tags... which needs external script with
> > strict coupling, doesn't offer any access control as far as I know, do
> > not allow deleting tags, etc.
> 
> Well, but your argument above also applies here - gitweb does not allow
> users to modify README.html or desrciption either. Who knows where ctags
> may come from, they might even be autogenerated based on something.
> I admit that the code could be a bit more resilient to invalid contents
> of the files, but I do not see that as a major problem either (it is
> possible to confuse gitweb by putting invalid stuff into other
> repository files too).

Well, if not the whole 'ctags' / folksonomy feature, then perhaps (as
proposed in other subthread of this discussion, and what Uwe agreed upon)
to remove "write" support for content tags.

Current solution with gitweb providing interface, and third-party script
providing implementation for creating tags, with unspecified API given by
gitweb, and no way to edit / delete tags from web interface, seems 
half-baked at best.


BTW. if I remember correctly [failed] GSoC 2010 project was among others
to add ability to edit description and README.html from gitweb; is there
any code about this in Pavan repository?

-- 
Jakub Narebski
Poland