Re: [PATCHv2 GSOC 06/11] gitweb: Create Gitweb::Escape module
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Jul 16, 2010, 09:01 UTC
- Message-ID
- <201007161101.23766.jnareb@gmail.com>
- In-Reply-To
- <1279178951-23712-7-git-send-email-pavan.sss1991@gmail.com>
On Thu, 15 Jul 2010, Pavan Kumar Sunkara wrote:
Show 18 quoted lines
> Create a Gitweb::Escape module in 'gitweb/lib/Gitweb/Escape.pm' > to store all the quoting/unquoting and escaping subroutines > regarding the gitweb.perl script. > > This module imports $fallback_encoding variable from > Gitweb::Config module to use it in sub 'to_utf8' > > Subroutines moved: > to_utf8 > esc_param > esc_url > esc_html > esc_path > quot_cec > quot_upr > untabify > > Update gitweb/Makefile to install Gitweb::Escape module alongside gitweb
Nice and straightforward refactoring.
For what it is worth, ACK from me.
[...]
Show 18 quoted lines
> +# quote unsafe chars, but keep the slash, even when it's not
> +# correct, but quoted slashes look too horrible in bookmarks
> +sub esc_param {
> + my $str = shift;
> + return undef unless defined $str;
> + $str =~ s/([^A-Za-z0-9\-_.~()\/:@ ]+)/CGI::escape($1)/eg;
> + $str =~ s/ /\+/g;
> + return $str;
> +}
> +
> +# quote unsafe chars in whole URL, so some charactrs cannot be quoted
> +sub esc_url {
> + my $str = shift;
> + return undef unless defined $str;
> + $str =~ s/([^A-Za-z0-9\-_.~();\/;?:@&= ]+)/CGI::escape($1)/eg;
> + $str =~ s/ /\+/g;
> + return $str;
> +}I see that here (or rather in corresponding preimage) you have dependency on first patch in series, i.e. "gitweb: fix esc_url".
If you had send first patch as a separate email, not as part of series, you would have need to mention in cover letter that the series (without first patch) is based on / requires "gitweb: fix esc_url".
-- Jakub Narebski Poland