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

Re: Fwd: Possibly nicer pathspec syntax?

From
Duy Nguyen <pclouds@gmail.com>
Date
Feb 9, 2017, 13:27 UTC
Message-ID
<CACsJy8AbSrcqEiDByyTha9UdQvwFFXkjzKpAn6H96WD+aMdh1Q@mail.gmail.com>
In-Reply-To
<xmqqy3xgwpiq.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 9, 2017 at 12:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 47 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Wed, Feb 8, 2017 at 12:12 PM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>> Two-patch series to follow.
>>
>> glossary-content.txt update for both patches would be nice.
>
> I am no longer worried about it as I saw somebody actually sent
> follow-up patches on this, but I want to pick your brain on one
> thing that is related to this codepath.
>
> We have PATHSPEC_PREFER_CWD and PATHSPEC_PREFER_FULL bits in flags,
> added at fc12261fea ("parse_pathspec: add PATHSPEC_PREFER_{CWD,FULL}
> flags", 2013-07-14), and I think the intent is some commands when
> given no pathspec work on all paths in the current subdirectory
> while others work on the full tree, regardless of where you are.
> "grep" is in the former camp, "log" is in the latter.  And there is
> a check to catch a bug in a caller that sets both.
>
> I am wondering about this hunk (this is from the original commit
> that added it):
>
>         if (!entry) {
>                 static const char *raw[2];
>
> +               if (flags & PATHSPEC_PREFER_FULL)
> +                       return;
> +
> +               if (!(flags & PATHSPEC_PREFER_CWD))
> +                       die("BUG: PATHSPEC_PREFER_CWD requires arguments");
> +
>                 pathspec->items = item = xmalloc(sizeof(*item));
>                 memset(item, 0, sizeof(*item));
>                 item->match = prefix;
>                 ... returns a single entry pathspec to cover cwd ...
>
> The BUG message is given when
>
>  - The command got no pathspec from the caller; and
>  - PATHSPEC_PREFER_FULL is not set; and
>  - PATHSPEC_PREFER_CWD is NOT set.
>
> but the message says that the caller must have args when it sets
> prefer-cwd.  Is this a simple typo?  If so what should it say?
>
>         die("BUG: one of PATHSPEC_PREFER_FULL or _CWD must be set");

Without reading through your next mail, I'd say "BUG: this command requires arguments".

Show 6 quoted lines
> Does this third possibility (i.e. a caller is allowed to pass
> "flags" that does not prefer either) exist to support a command
> where the caller MUST have at least one pathspec?  If that were the
> case, this wouldn't be a BUG but an end-user error, e.g.
>
>         die("at least one pathspec element is required");

Or this. Yes. I might have just been defensive at then and kept the third option open.

> If you know offhand which callers pass neither of the two
> PATHSPEC_PREFER_* bits and remember for what purpose you allowed
> them to do so, please remind me.  I'll keep digging in the meantime.

I don't usually remember what I ate yesterday and this commit was from 2013 :D But I'll see if your findings spark anything in my brain.

-- 
Duy
Previous: Duy NguyenNext: Mike Hommey
Message 14 of 17 in “Fwd: Possibly nicer pathspec syntax?”
  1. Linus TorvaldsFeb 7, 2017
  2. Junio C HamanoFeb 8, 2017
  3. Linus TorvaldsFeb 8, 2017
  4. Junio C HamanoFeb 8, 2017
  5. Linus TorvaldsFeb 8, 2017
  6. Junio C HamanoFeb 8, 2017
  7. Linus TorvaldsFeb 8, 2017
  8. Junio C HamanoFeb 8, 2017
  9. Linus TorvaldsFeb 8, 2017
  10. Duy NguyenFeb 8, 2017
  11. Junio C HamanoFeb 8, 2017
  12. Junio C HamanoFeb 8, 2017
  13. Duy NguyenFeb 9, 2017
  14. Duy NguyenFeb 9, 2017
  15. Mike HommeyFeb 8, 2017
  16. Linus TorvaldsFeb 8, 2017
  17. Mike HommeyFeb 8, 2017

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.