Re: [PATCH v4 13/35] ls-refs: introduce ls-refs server command

From: Brandon Williams <bmwill@google.com>
Date: Mar 13, 2018, 21:27 UTC
Message-ID: <20180313212710.GD7638@google.com>
In-Reply-To: <xmqqmuzqb2ph.fsf@gitster-ct.c.googlers.com>

On 03/02, Junio C Hamano wrote:

Show 31 quoted lines

> Brandon Williams <bmwill@google.com> writes:
> 
> > + ls-refs
> > +---------
> > +
> > +`ls-refs` is the command used to request a reference advertisement in v2.
> > +Unlike the current reference advertisement, ls-refs takes in arguments
> > +which can be used to limit the refs sent from the server.
> 
> OK.
> 
> > +Additional features not supported in the base command will be advertised
> > +as the value of the command in the capability advertisement in the form
> > +of a space separated list of features, e.g.  "<command>=<feature 1>
> > +<feature 2>".
> 
> Doesn't this explain the general convention that applies to any
> command, not just ls-refs command?  As a part of ls-refs section,
> <command> in the above explanation is always a constant "ls-refs",
> right?
> 
> It is a bit unclear how <feature N> in the above description are
> related to "arguments" in the following paragraph.  Do the server
> that can show symref and peeled tags and that can limit the output
> with ref-pattern advertise these three as supported features, i.e.
> 
> 	ls-refs=symrefs peel ref-pattern
> 
> or something?  Would there a case where a "feature" does not
> correspond 1:1 to an argument to the command, and if so how would
> the server and the client negotiate use of such a feature?

I mention earlier in the document that the values of each capability are to be defined by the capability itself, so I'm just documenting what the value advertised means. And a feature could mean a couple things and doesn't necessarily mean it affects the arguments which can be provided, and it definitely doesn't mean that each argument that can be provided must be advertised as a feature. If you look at the patch that introduces shallow, the shallow feature adds lots of arguments that a client can that use in its request.

Show 13 quoted lines

> 
> > +    ref-pattern <pattern>
> > +	When specified, only references matching one of the provided
> > +	patterns are displayed.  A pattern is either a valid refname
> > +	(e.g.  refs/heads/master), in which a ref must match the pattern
> > +	exactly, or a prefix of a ref followed by a single '*' wildcard
> > +	character (e.g. refs/heads/*), in which a ref must have a prefix
> > +	equal to the pattern up to the wildcard character.
> 
> I thought the recent concensus was left-anchored prefix match that
> honors /-directory boundary, i.e. no explicit asterisk and just
> saying "refs/heads" is enough to match "refs/heads" itself and
> "refs/heads/master" but not "refs/headscarf"?

I don't think there was a consensus at the time, but in the next revision I'll have them be prefixes instead of containing wildcards.

-- 
Brandon Williams