From: Junio C Hamano <gitster@pobox•com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 1/3] pathspec: catch prepending :(prefix) on pathspec with short magic
Date: Thu, 05 Sep 2013 11:39:56 -0700 [thread overview]
Message-ID: <xmqqppsni03n.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1378352440-25410-1-git-send-email-pclouds@gmail.com> ("Nguyễn Thái Ngọc Duy"'s message of "Thu, 5 Sep 2013 10:40:38 +0700")
Nguyễn Thái Ngọc Duy <pclouds@gmail•com> writes:
> :(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.
prev parent reply other threads:[~2013-09-05 18:40 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-05 3:40 [PATCH 1/3] pathspec: catch prepending :(prefix) on pathspec with short magic Nguyễn Thái Ngọc Duy
2013-09-05 3:40 ` [PATCH 2/3] add: lift the pathspec magic restriction on "add -p" Nguyễn Thái Ngọc Duy
2013-09-05 3:40 ` [PATCH 3/3] add--interactive: fix external command invocation on Windows Nguyễn Thái Ngọc Duy
2013-09-05 18:39 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqppsni03n.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox