git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Elijah Newren <newren@gmail.com>
Date
Mar 27, 2020, 19:01 UTC
Message-ID
<CABPp-BG+Wohx4idoh1KSVZvuQ6aUJ6OKACRMi=gkvQjzd2sWUw@mail.gmail.com>
In-Reply-To
<xmqq369tiyo8.fsf@gitster.c.googlers.com>
On Fri, Mar 27, 2020 at 8:51 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 27 quoted lines
>
> Elijah Newren <newren@gmail.com> writes:
>
> > 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.
Cool.
Show 36 quoted lines
> 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.

Ah, so you're saying that even though --cached and REVISION are incompatible today, that's not fundamental and we could conceivably let them or even more options be used together in the future and you even highlight how it could be made to sensibly work. I agree with what you say here: _if_ there is a way for users to explicitly specify that they want to search multiple versions (whether that is revisions or the index or the working tree), _and_ we have a way to distinguish which version we found the results from, then (and only then) it'd make sense to search the complete set of files from each of those versions and show the results for the matches we found.

That differs in multiple important ways from the SKIP_WORKTREE behavior I was railing against, and I think what you propose as a possibility in contrast would make sense.

Previous: Junio C HamanoNext: Matheus Tavares Bernardino
Message 31 of 62 in “grep: honor sparse checkout and add option to ignore it”
  1. 0/3 grep: honor sparse checkout and add option to ignore itMatheus Tavares, Mar 24, 2020
  2. 1/3 doc: grep: unify info on configuration variablesMatheus Tavares, Mar 24, 2020
  3. Elijah NewrenMar 24, 2020
  4. Junio C HamanoMar 24, 2020
  5. Matheus TavaresMar 24, 2020
  6. 2/3 grep: honor sparse checkout patternsMatheus Tavares, Mar 24, 2020
  7. Elijah NewrenMar 24, 2020
  8. Derrick StoleeMar 24, 2020
  9. Elijah NewrenMar 24, 2020
  10. Derrick StoleeMar 24, 2020
  11. Matheus Tavares BernardinoMar 24, 2020
  12. Matheus Tavares BernardinoMar 24, 2020
  13. Matheus Tavares BernardinoApr 21, 2020
  14. Elijah NewrenApr 21, 2020
  15. Derrick StoleeApr 22, 2020
  16. Matheus Tavares BernardinoApr 23, 2020
  17. 3/3 grep: add option to ignore sparsity patternsMatheus Tavares, Mar 24, 2020
  18. Elijah NewrenMar 24, 2020
  19. Junio C HamanoMar 24, 2020
  20. Elijah NewrenMar 24, 2020
  21. Junio C HamanoMar 25, 2020
  22. Matheus Tavares BernardinoMar 30, 2020
  23. Elijah NewrenMar 31, 2020
  24. Derrick StoleeMar 31, 2020
  25. Matheus Tavares BernardinoApr 27, 2020
  26. Elijah NewrenApr 29, 2020
  27. Elijah NewrenApr 29, 2020
  28. Matheus Tavares BernardinoMar 25, 2020
  29. Elijah NewrenMar 26, 2020
  30. Junio C HamanoMar 27, 2020
  31. Elijah NewrenMar 27, 2020
  32. Matheus Tavares BernardinoMar 30, 2020
  33. Elijah NewrenMar 31, 2020
  34. 0/4 grep: honor sparse checkout and add option to ignore itMatheus Tavares, May 10, 2020
  35. 1/4 doc: grep: unify info on configuration variablesMatheus Tavares, May 10, 2020
  36. 2/4 config: load the correct config.worktree fileMatheus Tavares, May 10, 2020
  37. Junio C HamanoMay 11, 2020
  38. Matheus Tavares BernardinoMay 12, 2020
  39. Junio C HamanoMay 12, 2020
  40. 3/4 grep: honor sparse checkout patternsMatheus Tavares, May 10, 2020
  41. Junio C HamanoMay 11, 2020
  42. Matheus Tavares BernardinoMay 13, 2020
  43. Junio C HamanoMay 13, 2020
  44. Elijah NewrenMay 21, 2020
  45. Matheus Tavares BernardinoMay 21, 2020
  46. Elijah NewrenMay 21, 2020
  47. Matheus Tavares BernardinoMay 22, 2020
  48. Elijah NewrenMay 22, 2020
  49. Elijah NewrenMay 22, 2020
  50. Matheus Tavares BernardinoMay 22, 2020
  51. Elijah NewrenMay 22, 2020
  52. Derrick StoleeJun 10, 2020
  53. Matheus Tavares BernardinoJun 10, 2020
  54. Derrick StoleeJun 10, 2020
  55. Matheus Tavares BernardinoJun 10, 2020
  56. Elijah NewrenJun 10, 2020
  57. Elijah NewrenJun 10, 2020
  58. Elijah NewrenMay 21, 2020
  59. 4/4 config: add setting to ignore sparsity patterns in some cmdsMatheus Tavares, May 10, 2020
  60. Matheus Tavares BernardinoMay 10, 2020
  61. Elijah NewrenMay 21, 2020
  62. Elijah NewrenMay 21, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.