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

Re: [PATCH 31/35] pathspec: allow querying for attributes

From
Stefan Beller <sbeller@google.com>
Date
Nov 22, 2016, 17:26 UTC
Message-ID
<CAGZ79kYm4LfXK=1j-ayLawt+BojnkyM4h2RLQ=kfpPgMQbdBag@mail.gmail.com>
In-Reply-To
<CACsJy8BXjYOza_1mPCJJw+Mk1zksLLJMBNKvbAk8+1-bdAGJMw@mail.gmail.com>
On Tue, Nov 22, 2016 at 2:41 AM, Duy Nguyen <pclouds@gmail.com> wrote:
Show 14 quoted lines
> On Fri, Nov 11, 2016 at 3:34 AM, Stefan Beller <sbeller@google.com> wrote:
>> @@ -139,7 +140,8 @@ static size_t common_prefix_len(const struct pathspec *pathspec)
>>                        PATHSPEC_LITERAL |
>>                        PATHSPEC_GLOB |
>>                        PATHSPEC_ICASE |
>> -                      PATHSPEC_EXCLUDE);
>> +                      PATHSPEC_EXCLUDE |
>> +                      PATHSPEC_ATTR);
>
> Hmm.. common_prefix_len() has always been a bit relaxing and can cover
> more than needed. It's for early pruning. Exact pathspec matching
> _will_ be done later anyway.
>
> Is that obvious?

Yes it is. Not sure what your concern is, though.

Given the pathspec ":(attr:plumbing)Documentation/", the common_prefix_len is still able to figure out that any match has a prefix of strlen("Documentation/"), no matter what attr stuff is involved, because the attr stuff is also just reducing the matching set.

Now if we have such a pathspec it is easier to claim common_prefix_len supports attr as it is a correct thing to ignore the attrs completely, than to rewrite the call to common_prefix_len to be without attrs involved.

You *may* be able to improve it with knowledge of the attrs: ":(attr:internal-technical-api)Documentation/" may restrict the results to match only "Documentation/technical/", but as you said, we don't have to be exact here.

> I'm wondering if we need to add a line or two in the
> big comment code before this statement. I'm thinking it is and we
> probably don't need more comments...
Do I misunderstand the code completely here?

Thanks, Stefan

> --
> Duy
Previous: Duy NguyenNext: Duy Nguyen
Message 24 of 43 in “[PATCHv3 00/35] Revamp the attr subsystem!”
  1. Stefan BellerNov 10, 2016
  2. 02/35 attr.c: use strchrnul() to scan for one lineStefan Beller, Nov 10, 2016
  3. 06/35 attr.c: mark where #if DEBUG ends more clearlyStefan Beller, Nov 10, 2016
  4. 05/35 attr.c: complete a sentence in a commentStefan Beller, Nov 10, 2016
  5. 09/35 attr.c: plug small leak in parse_attr_line()Stefan Beller, Nov 10, 2016
  6. 10/35 attr: rename function and struct related to checking attributesStefan Beller, Nov 10, 2016
  7. 12/35 attr: convert git_all_attrs() to use "struct git_attr_check"Stefan Beller, Nov 10, 2016
  8. 11/35 attr: (re)introduce git_check_attr() and struct git_attr_checkStefan Beller, Nov 10, 2016
  9. 13/35 attr: convert git_check_attrs() callers to use the new APIStefan Beller, Nov 10, 2016
  10. 18/35 attr.c: add push_stack() helperStefan Beller, Nov 10, 2016
  11. 20/35 attr.c: rename a local variable checkStefan Beller, Nov 10, 2016
  12. 22/35 attr.c: introduce empty_attr_check_elems()Stefan Beller, Nov 10, 2016
  13. 19/35 attr.c: pass struct git_attr_check down the callchainStefan Beller, Nov 10, 2016
  14. 17/35 attr: support quoting pathname patterns in C styleStefan Beller, Nov 10, 2016
  15. 16/35 attr: expose validity check for attribute namesStefan Beller, Nov 10, 2016
  16. 34/35 clone: add --init-submodule=<pathspec> switchStefan Beller, Nov 10, 2016
  17. 26/35 attr: convert to new threadsafe APIStefan Beller, Nov 10, 2016
  18. 35/35 completion: clone can initialize specific submodulesStefan Beller, Nov 10, 2016
  19. 27/35 attr: keep attr stack for each checkStefan Beller, Nov 10, 2016
  20. 33/35 submodule update: add `--init-default-path` switchStefan Beller, Nov 10, 2016
  21. 28/35 Documentation: fix a typoStefan Beller, Nov 10, 2016
  22. 31/35 pathspec: allow querying for attributesStefan Beller, Nov 10, 2016
  23. Duy NguyenNov 22, 2016
  24. Stefan BellerNov 22, 2016
  25. Duy NguyenNov 23, 2016
  26. Brandon WilliamsNov 28, 2016
  27. Brandon WilliamsNov 28, 2016
  28. Brandon WilliamsNov 28, 2016
  29. Stefan BellerNov 29, 2016
  30. 32/35 pathspec: allow escaped query valuesStefan Beller, Nov 10, 2016
  31. 30/35 pathspec: move prefix check out of the inner loopStefan Beller, Nov 10, 2016
  32. 29/35 pathspec: move long magic parsing out of prefix_pathspecStefan Beller, Nov 10, 2016
  33. 23/35 attr.c: always pass check[] to collect_some_attrs()Stefan Beller, Nov 10, 2016
  34. 25/35 attr: make git_check_attr_counted staticStefan Beller, Nov 10, 2016
  35. 24/35 attr.c: outline the future plans by heavily commentingStefan Beller, Nov 10, 2016
  36. 21/35 attr.c: correct ugly hack for git_all_attrs()Stefan Beller, Nov 10, 2016
  37. 14/35 attr: retire git_check_attrs() APIStefan Beller, Nov 10, 2016
  38. 01/35 commit.c: use strchrnul() to scan for one lineStefan Beller, Nov 10, 2016
  39. 15/35 attr: add counted string version of git_check_attr()Stefan Beller, Nov 10, 2016
  40. 08/35 attr.c: tighten constness around "git_attr" structureStefan Beller, Nov 10, 2016
  41. 03/35 attr.c: update a stale comment on "struct match_attr"Stefan Beller, Nov 10, 2016
  42. 07/35 attr.c: simplify macroexpand_one()Stefan Beller, Nov 10, 2016
  43. 04/35 attr.c: explain the lack of attr-name syntax check in parse_attr()Stefan Beller, Nov 10, 2016

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.