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

Re: [PATCH 1/3] pathspec: catch prepending :(prefix) on pathspec with short magic

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 5, 2013, 18:39 UTC
Message-ID
<xmqqppsni03n.fsf@gitster.dls.corp.google.com>
In-Reply-To
<1378352440-25410-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 35 quoted lines
> :(prefix) is in the long form. Suppose people pass :!foo with '!'
> being the short form of magic 'bar', the code will happily turn it to
> :(prefix..)!foo, which makes '!' part of the path and no longer a magic.
>
> The correct form must be ':(prefix..,bar)foo', but as so far we
> haven't had any magic in short form yet (*), the code to convert from
> short form to long one will be inactive anyway. Let's postpone it
> until a real short form magic appears.
>
> (*) The short form magic '/' is a special case and won't be caught by
> this die(), which is correct. When '/' magic is detected, prefixlen is
> set back to 0 and the whole "if (prefixlen..)" block is skipped.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  fixes on top of nd/magic-pathspec.
>
>  pathspec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/pathspec.c b/pathspec.c
> index d9f4143..62fde50 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -231,7 +231,9 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>  		const char *start = elt;
>  		if (prefixlen && !literal_global) {
>  			/* Preserve the actual prefix length of each pattern */
> -			if (long_magic_end) {
> +			if (short_magic)
> +				die("BUG: prefixing on short magic is not supported");
> +			else if (long_magic_end) {
>  				strbuf_add(&sb, start, long_magic_end - start);
>  				strbuf_addf(&sb, ",prefix:%d", prefixlen);
>  				start = long_magic_end;
Good.

I wonder if we should start thinking about removing the big "NEEDSWORK" comment in front of this function.

Also the pathspec_magic[] array was the table-driven way that was meant to be enhanced to fit future needs to parse all supported types of pathspec magic, but it seems that "prefix:12" magic is implemented using a custom/special case code. We may want to see if we want to enrich the parser to fold this to match the table-driven approach better someday---this is not urgent as we are not adding any new pathspec magic now.

Will queue.  Thanks.
Previous: Nguyễn Thái Ngọc Duy
Message 4 of 4 in “pathspec: catch prepending :(prefix) on pathspec with short magic”
  1. 1/3 pathspec: catch prepending :(prefix) on pathspec with short magicNguyễn Thái Ngọc Duy, Sep 5, 2013
  2. 2/3 add: lift the pathspec magic restriction on "add -p"Nguyễn Thái Ngọc Duy, Sep 5, 2013
  3. 3/3 add--interactive: fix external command invocation on WindowsNguyễn Thái Ngọc Duy, Sep 5, 2013
  4. Junio C HamanoSep 5, 2013

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.