Re: [PATCH v1 3/5] list-objects-filter: implement composite filters

From: Jeff King <peff@peff.net>
Date: Jun 4, 2019, 16:13 UTC
Message-ID: <20190604161332.GA29603@sigill.intra.peff.net>
In-Reply-To: <20190603222247.GG4641@comcast.net>

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

Show 8 quoted lines

> 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).

Show 5 quoted lines

> -	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