Re: [RFC PATCH 3/3] grep: add option to ignore sparsity patterns

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 27, 2020, 15:51 UTC
Message-ID: <xmqq369tiyo8.fsf@gitster.c.googlers.com>
In-Reply-To: <CABPp-BGVO3QdbfE84uF_3QDF0-y2iHHh6G5FAFzNRfeRitkuHw@mail.gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 6 quoted lines

> Sometimes, code that wasn't meant to be used together accidentally is
> used together or the docs suggest they can be used together.  ...
> ... but that's the
> type of accident that is easy to have in the implementation or docs
> because it doesn't even occur to people who understand the design and
> the data structures why anyone would attempt that.

The above is not limited to "git grep", but you said so clearly what I have felt, without being able to express myself in a satisfactory manner, for the last 10 years.

> ... (Side note: I think this kind of
> issues occurs fairly frequently, so I'm unlikely to assume options
> were meant to be supported together based solely on a lack of logic
> that would throw an error when both are specified.

Amen to that.

By the way, and I am so sorry to making the main issue of the discussion into a mere "by the way" point, but if I understand your message correctly, the primary conclusion in there is that a file that is not in the working tree, if the sparsity pattern tells us that it should not be checked out to the working tree, should not be sought in the index instead. I think I agree with that conclusion.

I however have some disagreement on a minor point, though.

"git grep -e '<pattern>' master" looks for the pattern in the commit at the tip of the master branch. "git grep -e '<pattern>' master pu" does so in these two commits. I do not think it is conceptually wrong to allow "git grep -e '<pattern>' --cached master pu" to look for three "commits", i.e. those two commits that already exist, plus the one you would be creating if you were to "git commit" right now. Similarly, I do not see a reason why we should forbid looking for the same pattern in the tracked files in the working tree at the same time we check tree object(s) and/or the index.

At least in principle.

There are two practical issues that makes these combinations problematic, but I do not think they are insurmountable.

 - Once you give an object on the command line, there is no syntax
   to let you say "oh, by the way, I want the working tree as well".
   If you are looking in the index, the working tree, and optionally
   in some objects, "--index" instead of "--cached" would be the
   standard way to tell the command "I want to affect both the index
   and the working tree", but there is no way to say "I want only
   tracked files in the working tree and these objects searched".
   We'd need a new syntax to express it if we wanted to allow the
   combination.

 - The lines found in the working tree and in the index are prefixed
   by the filename, while they are prefixed by the tree's name and a
   colon.  When output for the working tree and the index are
   combined, we cannot tell where each hit came from.  We need to
   change the output to allow us to tell them apart, by
   e.g. prefixing "<worktree>:" and "<index>:" in a way similar to
   we use "<revision>:".

Thanks.