Web lists-archives.com

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






On 5/29/2019 7:27 PM, Matthew DeVore wrote:
On Wed, May 29, 2019 at 05:29:14PM -0400, Jeff Hostetler wrote:
Was sparse:path filter the only reason for needing all the URL encoding?
The sparse:oid form allows values <ref>:<path> and these (or at least
the <path> portion) may contain special characters.  So don't we need to
URL encode this form too?

Oh, I missed this. I was only thinking an oid was allowed after "sparse:". So as
I suspected I was overlooking something obvious.

Now I just want to understand the objection to URL encoding a little better. I
haven't worked with in a project that requires a lot of boilerplate before, so I
may be asking obvious things again. If so, sorry in advance.

So the objections, as I interpret them so far, are that:

  a the URL encoding/decoding complicates the code base
  b explaining the URL encoding, while it allows for future expansion, requires
    some verbose documentation in git-rev-list that is potentially distracting or
    confusing
  c there may be a better way to allow for future expansion that does not require
    URL encoding
  d the URL encoding is unpleasant to use (note that my patchset makes it
    optional for the user to use and it is only mandatory in sending it over the
    wire)

I think these are reasonable and I'm willing to stop digging my heels in :) Does
the above sum everything up?


My primary concern was how awkward it would be to use the URL
encoding syntax on the command line, but as you say, that can be
avoided by using the multiple --filter args.

And to be honest, the wire format is hidden from user view, so it
doesn't really matter there.  So either approach is fine.  I was
hoping that the "filters (plural)" approach would let us avoid URL
encoding, but that comes with its own baggage as you suggested.
And besides, URL encoding is well-understood.

And I don't want to prematurely complicate this with ANDs ORs and
XORs as you mention in another thread.

So don't let me stop this effort.


BTW, I don't think I've seen this mentioned anywhere and I don't
remember if this got into the code or not.  But we discussed having
a repo-local config setting to remember the filter-spec used by the
partial clone that would be inherited by a subsequent (partial) fetch.
Or would be set by the first partial fetch following a normal clone.
Having a single composite filter spec would help with this.

Jeff