Re: [PATCH 8/8] pathspec: convert parse_pathspec to take an index

[Junio C Hamano <gitster@pobox.com>]

Brandon Williams <bmwill@google.com> writes:

> ls-files is the only command (that I know of) which does cache pruning
> based on the common prefix of all pathspecs given.  If there is a
> submodule named 'sub' and you provided a pathspec 'sub/', the matching
> logic can handle this but the cache pruning logic would prune all
> entries from the index which don't have a leading 'sub/' which is the
> common prefix of all pathspecs (if you didn't strip off the slash).
> Meaning you'd prune the submodule 'sub' when  you really didn't want to.
> This could probably be fixed to have the cache pruning logic to prune by
> ignoring the trailing slash always.
>
> So there's another option here if you don't feel quite right about
> piping through an index into parse_pathspec just yet.

Oh, don't get me wrong---I do not think passing an index_state
instance throughout the callchain (and perhaps later we may pass an
instance of "repository" instead) is a wrong move in the longer term
for various APIs.  I was just wondering if we have callers that can
benefit from this change immediately---manipulators like "commit" do
already use multiple instances of index_state structure.

But perhaps you are right---it may be wrong for the contents of the
current index (or any index) to affect how a pathspec element is
parsed in the first place.  If the current code (before this series)
uses the_index only for error checking, we may want to separate that
out of the parse_pathspec() callchain, so that it does not even look
at any index (not just the_index).  And that may be a better change
overall.

Thanks.