From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Stavros Ntentos <stdedos@gmail.com>,
bagasdotme@gmail.com, git@vger.kernel.org, stdedos+git@gmail.com
Subject: Re: [RFC PATCH v1 1/2] pathspec: warn: long and short forms are incompatible
Date: Sat, 27 Mar 2021 11:36:20 -0700 [thread overview]
Message-ID: <xmqqa6qoqw9n.fsf@gitster.g> (raw)
In-Reply-To: <YF792/TkS3Ssw9NS@coredump.intra.peff.net> (Jeff King's message of "Sat, 27 Mar 2021 05:41:47 -0400")
Jeff King <peff@peff.net> writes:
> It also feels like any checks like this should be relying on the
> existing pathspec-magic parser a bit more. I don't know the pathspec
> code that well, but surely at some point it has a notion of which parts
> are magic flags (e.g., after parse_element_magic in init_pathspec_item).
Absolutely. parse_element_magic() decides, by looking at the
character after the initial ':', the we are looking at the longhand
form when magic sequence is introduced by ":(".
Otherwise, : must be followed by shorthand form, where the only
usable ones are '/' (synonym for top), '!' and '^' (synonym for
exclude), and ':' (end of short-form marker), but most importantly,
'(' will *never* be a valid shorthand form (because it is not part
of the GIT_PATHSPEC_MAGIC class of bytes).
So, presumably, inside parse_short_magic(), you could detect '('
and complain it as an attempt to switch to longform.
Here is to illustrate where to hook the check. With this, I can get
something like this:
$ git show -s --oneline -- ':!/(foobar)frotz' ':/(bar)nitfol' .
warning: :!/: cannot mix shortform magic with longform like (exclude)
warning: :/: cannot mix shortform magic with longform like (exclude)
84d06cdc06 Sync with v2.31.1
Note that this illustration does not show the best warning message.
To improve it to a bit more useful level, I think two things need to
happen on top.
* Use advice to allow users sequelch.
* Show a few letters after '(' (but pay attention to the end of the
string, which ends with either '\0' or ':'), and lose the
substring "like (exclude)" from the message.
That would have given
hint: ":!/(foo...": cannot mix shortform and longform magic
for the above example.
I initially thought it might make it even better if we looked ahead
of what comes after '(', took the substring before ',' or ')', and
looked up pathspec_magic[] array, BUT I do not think it is a good
idea. It would be confusing if we do not give the same advice to
":!(literol)" when the user does get one for ":!(literal)". So the
above "two things need to happen" list deliberately excludes an
"improvement" doing so.
pathspec.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git i/pathspec.c w/pathspec.c
index 18b3be362a..cd343d5b54 100644
--- i/pathspec.c
+++ w/pathspec.c
@@ -336,6 +336,9 @@ static const char *parse_long_magic(unsigned *magic, int *prefix_len,
return pos;
}
+static const char mixed_pathspec_magic[] =
+ N_("%.*s: cannot mix shortform magic with longform like (exclude)");
+
/*
* Parse the pathspec element looking for short magic
*
@@ -356,6 +359,16 @@ static const char *parse_short_magic(unsigned *magic, const char *elem)
continue;
}
+ if (ch == '(') {
+ /*
+ * a possible common mistake: once you started
+ * a shortform you cannot add (longform), e.g.
+ * ":!(top)"
+ */
+ warning(_(mixed_pathspec_magic),
+ (int)(pos-elem), elem);
+ }
+
if (!is_pathspec_magic(ch))
break;
next prev parent reply other threads:[~2021-03-27 18:37 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-26 20:44 Pathspec does not accept / does not warn for specific pathspecs Σταύρος Ντέντος
2021-02-26 23:27 ` Junio C Hamano
2021-03-25 10:22 ` [PATCH v1 0/1] pathspec: warn for a no-glob entry that contains `**` Σταύρος Ντέντος
2021-03-25 10:22 ` [PATCH v1 1/1] " Σταύρος Ντέντος
2021-03-25 11:00 ` Bagas Sanjaya
2021-03-25 11:04 ` Bagas Sanjaya
2021-03-25 16:09 ` Stavros Ntentos
2021-03-25 20:09 ` Junio C Hamano
2021-03-25 23:36 ` [PATCH v2 0/1] " Σταύρος Ντέντος
2021-03-25 23:36 ` [PATCH v2 1/1] " Σταύρος Ντέντος
2021-03-26 0:41 ` Junio C Hamano
2021-03-26 1:32 ` Eric Sunshine
2021-03-26 3:02 ` Junio C Hamano
2021-03-26 5:13 ` Jeff King
2021-03-26 15:43 ` Stavros Ntentos
2021-03-26 15:48 ` [PATCH v3 1/2] " Stavros Ntentos
2021-03-26 15:48 ` [PATCH v3 2/2] pathspec: convert no-glob warn to advice Stavros Ntentos
2021-03-26 2:40 ` [RFC PATCH v1 0/1] pathspec: warn: long and short forms are incompatible Σταύρος Ντέντος
2021-03-26 2:40 ` [RFC PATCH v1 1/2] " Σταύρος Ντέντος
2021-03-26 5:28 ` Jeff King
2021-03-26 16:16 ` Stavros Ntentos
2021-03-27 9:41 ` Jeff King
2021-03-27 18:36 ` Junio C Hamano [this message]
2021-03-28 15:44 ` Stavros Ntentos
2021-03-26 2:40 ` [RFC PATCH v1 2/2] fixup! " Σταύρος Ντέντος
2021-03-26 8:14 ` Bagas Sanjaya
2021-03-26 15:55 ` Stavros Ntentos
2021-03-28 15:26 ` [RFC PATCH v1 3/3] squash! " Stavros Ntentos
2021-03-26 2:44 ` [RFC PATCH v1 0/1] " Σταύρος Ντέντος
2021-03-26 2:44 ` [RFC PATCH v1] " Σταύρος Ντέντος
2021-04-03 12:26 ` [PATCH v3] pathspec: advice: " Stavros Ntentos
2021-04-04 7:19 ` Junio C Hamano
2021-04-11 15:07 ` Σταύρος Ντέντος
2021-04-11 19:10 ` Junio C Hamano
2021-04-11 20:53 ` Σταύρος Ντέντος
2021-03-28 15:45 ` [PATCH v2] " Stavros Ntentos
2021-03-28 19:12 ` Junio C Hamano
2021-03-30 17:37 ` Junio C Hamano
2021-03-30 19:05 ` Stavros Ntentos
2021-03-30 19:17 ` Stavros Ntentos
2021-03-30 20:36 ` Junio C Hamano
2021-04-03 12:48 ` [PATCH v3] " Stavros Ntentos
2021-04-03 12:51 ` [PATCH v4] " Stavros Ntentos
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=xmqqa6qoqw9n.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=bagasdotme@gmail.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=stdedos+git@gmail.com \
--cc=stdedos@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).