Junio C Hamano wrote: > Derrick Stolee <derrickstolee@xxxxxxxxxx> writes: > >> On 8/17/2022 3:56 AM, Shaoxuan Yuan wrote: >>> Add a --sparse option to `git-grep`. This option is mainly used to: >>> >>> If searching in the index (using --cached): >>> >>> With --sparse, proceed the action when the current cache_entry is >> >> This phrasing is awkward. It might be better to reframe to describe the >> _why_ before the _what_ > > Thanks for an excellent suggestion. As a project participant, I > could guess the motivation, but couldn't link the parts of the > proposed log message to what I thought was being said X-<. The > below is much clearer. > >> When the '--cached' option is used with the 'git grep' command, the >> search is limited to the blobs found in the index, not in the worktree. >> If the user has enabled sparse-checkout, this might present more results >> than they would like, since the files outside of the sparse-checkout are >> unlikely to be important to them. > > Great. As an explanation of the reasoning behind the design > decision, I do not think it is bad to go even stronger than "might > ... would like" and assume or declare that those users who use > sparse-checkout are the ones who do NOT want to see the parts of the > tree that are sparsed out. And based on that assumption, "grep" and > "grep --cached" should not bother reporting hit from the part that > the user is not interested in. > > By stating the design and the reasoning behind that decision clearly > like so, we allow future developers to reconsider the earlier design > decision more easily. In 7 years, they may find that the Git users > in their era use sparse-checkout even when they still care about the > contents in the sparsed out area, in which case the basic assumption > behind this change is no longer valid and would allow them to make > "grep" and "grep --cached" behave differently. > >> Change the default behavior of 'git grep' to focus on the files within >> the sparse-checkout definition. To enable the previous behavior, add a >> '--sparse' option to 'git grep' that triggers the old behavior that >> inspects paths outside of the sparse-checkout definition when paired >> with the '--cached' option. > > Yup. Is that "--sparse" or "--unsparse"? We are busting the sparse > boundary and looking for everything, and calling the option to do so > "--sparse" somehow feels counter-intuitive, at least to me. It is a bit unintuitive, but '--sparse' is already used to mean "operate on SKIP_WORKTREE entries (i.e., pretend the repo isn't a sparse-checkout)" in both 'add' (0299a69694 (add: implement the --sparse option, 2021-09-24)) and 'rm' (f9786f9b85 (rm: add --sparse option, 2021-09-24)). The 'checkout-index' option '--ignore-skip-worktree-bits' indicates similar behavior (and is, IMO, similarly confusing with its use of "ignore"). I'm not sure '--unsparse' would fit as an alternative, though, since 'git grep' isn't really "unsparsifying" the repo (to me, that would imply updating the index to remove the 'SKIP_WORKTREE' flag). Rather, it's looking at files that are sparse when, by default, it does not. I still like the consistency of '--sparse' with existing similar options in other commands but, if we want to try something clearer here, maybe something like '--search-sparse' is more descriptive? > > Thanks.
