Re: [PATCH] gitweb: Make RSS feed output prettier

[Jakub Narebski <jnareb@gmail.com>]

Andreas Fuchs <asf@boinkor.net> wrote:

> * Wrap the commit message in <pre>
We use <div class="pre"> in "commit" view if I remember correctly.

> * Make file names into an unordered list
Good idea.

> * Add links (diff, conditional blame, history) to the file list.
I'd rather keep RSS output as simple as possible, no frills.

> ---
>  gitweb/gitweb.perl |   22 ++++++++++++++++------
>  1 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index e54a29e..2a79895 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4134,20 +4134,30 @@ XML
>                     "<content:encoded>" .
>                     "<![CDATA[\n";
>               my $comment = $co{'comment'};
> +             print "<pre>\n";
>               foreach my $line (@$comment) {
> -                     $line = to_utf8($line);
> -                     print "$line<br/>\n";
> +                     $line = to_utf8(esc_html($line));
esc_html does to_utf8, so to_utf8 is unnecessary (and spurious).
But it is a good catch: esc_html is certainly needed.

> +                     print "$line\n";
>               }
> -             print "<br/>\n";
> +             print "</pre><ul>\n";
>               foreach my $line (@difftree) {
>                       if (!($line =~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)([0-9]{0,3})\t(.*)$/)) {
>                               next;
>                       }
> -                     my $file = esc_path(unquote($7));
> +                     my $file_name = unquote($7);
> +                     my $file = esc_html($file_name);
We have introduced esc_path for escaping pathnames. Use it!

> +                     my $parent = $co{'parent'};
> +                     my $hash = git_get_hash_by_path($commit, $file_name);
> +                     my $hashparent = git_get_hash_by_path($parent, $file_name);
Two unnecessary calls to git command. Use 
     my %difftree = parse_difftree_raw_line($line)
instead. The conditions would probably be 
     next if (!$difftree{'from_id'});
(or equivalent).

> +
>                       $file = to_utf8($file);
> -                     print "$file<br/>\n";
> +                     print "<li>$file ";
> +                     print "[<a href=\"". esc_html("$my_url?p=$project;a=blobdiff;f=$file;h=$hash;hp=$hashparent;hb=$commit;hpb=$parent") ."\">diff</a>] ";
> +                     print "[<a href=\"". esc_html("$my_url?p=$project;a=blame;f=$file;hb=$commit") ."\">blame</a>] " if gitweb_check_feature('blame');
> +                     print "[<a href=\"". esc_html("$my_url?p=$project;a=history;f=$file;h=$commit") ."\">history</a>] ";
> +                     print "</li>\n";
esc_url, not esc_html here. Or use the href() subroutine with -full=>1
option (after applying the patch I send which added this to href()).

P.S. Please reply also to git mailing list.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git