On Mon, Jun 03, 2019 at 03:22:47PM -0700, Matthew DeVore wrote:

> On Mon, Jun 03, 2019 at 08:34:35AM -0400, Jeff King wrote:
> > Great. We might want to stop there, but it's possible could reuse even
> > more code. I didn't look closely before, but it seems this code is
> > decoding a URL. We already have a url_decode() routine in url.c. Could
> > it be reused?
> 
> Very nice. Here is an interdiff and the changes will be included in v3 of my
> patchset:

Nice to see a reduction in duplication (and I see you found some
problems in the existing code elsewhere; thanks for cleaning that up).

> -     return has_reserved_character(subspec, errbuf) ||
> -             url_decode(subspec, errbuf) ||
> -             gently_parse_list_objects_filter(
> -                     &filter_options->sub[new_index], subspec->buf, errbuf);
> +     decoded = url_percent_decode(subspec->buf);

I think you can get rid of has_reserved_character() now, too.

The reserved character list is still used on the encoding side. But I
think you could switch to strbuf_add_urlencode() there?

-Peff

Reply via email to