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
