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

Brandon Williams [off-list ref] 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.