Re: [PATCH v2] pathspec: advice: long and short forms are incompatible
From: Junio C Hamano <hidden>
Date: 2021-03-30 20:37:55
Stavros Ntentos [off-list ref] writes:
quoted
It may be more helpful if, rather than looking at what comes after '(', we looked at what came before '(' and helped the user write them out in the longformI don't see any explicit code in parsing the shortform magics, except: /* Special case alias for '!' */ if (ch == '^') { *magic |= PATHSPEC_EXCLUDE; continue; } and therefore I would like to avoid such task (although I love well-written DWIMs-or-close-to-them).
I am not yet interested in doing DWIM, but a good advice is always
welcome.
Along the lines of the attached illustration patch, perhaps.
$ git show --oneline --stat -- ':/!(glob)**/*.txt'
hint: ':/!(...': cannot mix short- and longform pathspec magic
hint: instead, spell the shortform magic '/!' as 'top,exclude' inside :(...
Documentation/config/advice.txt | 3 +++
advice.c | 3 +++
advice.h | 2 ++
pathspec.c | 29 +++++++++++++++++++++++++++++
4 files changed, 37 insertions(+)
diff --git c/Documentation/config/advice.txt w/Documentation/config/advice.txt
index acbd0c09aa..05a3cbc164 100644
--- c/Documentation/config/advice.txt
+++ w/Documentation/config/advice.txt@@ -119,4 +119,7 @@ advice.*:: addEmptyPathspec:: Advice shown if a user runs the add command without providing the pathspec parameter. + mixedPathspecMagic:: + Advice shown if a user tries to mix short- and + longform pathspec magic. --
diff --git c/advice.c w/advice.c
index 164742305f..9426bc5295 100644
--- c/advice.c
+++ w/advice.c@@ -33,6 +33,7 @@ int advice_checkout_ambiguous_remote_branch_name = 1; int advice_submodule_alternate_error_strategy_die = 1; int advice_add_ignored_file = 1; int advice_add_empty_pathspec = 1; +int advice_mixed_pathspec_magic = 1; static int advice_use_color = -1; static char advice_colors[][COLOR_MAXLEN] = {
@@ -95,6 +96,7 @@ static struct { { "submoduleAlternateErrorStrategyDie", &advice_submodule_alternate_error_strategy_die }, { "addIgnoredFile", &advice_add_ignored_file }, { "addEmptyPathspec", &advice_add_empty_pathspec }, + { "mixedPathspecMagic", &advice_mixed_pathspec_magic }, /* make this an alias for backward compatibility */ { "pushNonFastForward", &advice_push_update_rejected }
@@ -113,6 +115,7 @@ static struct { [ADVICE_GRAFT_FILE_DEPRECATED] = { "graftFileDeprecated", 1 }, [ADVICE_IGNORED_HOOK] = { "ignoredHook", 1 }, [ADVICE_IMPLICIT_IDENTITY] = { "implicitIdentity", 1 }, + [ADVICE_MIXED_PATHSPEC_MAGIC] = { "mixedPathspecMagic", 1 }, [ADVICE_NESTED_TAG] = { "nestedTag", 1 }, [ADVICE_OBJECT_NAME_WARNING] = { "objectNameWarning", 1 }, [ADVICE_PUSH_ALREADY_EXISTS] = { "pushAlreadyExists", 1 },
diff --git c/advice.h w/advice.h
index bc2432980a..d56c1896a0 100644
--- c/advice.h
+++ w/advice.h@@ -33,6 +33,7 @@ extern int advice_checkout_ambiguous_remote_branch_name; extern int advice_submodule_alternate_error_strategy_die; extern int advice_add_ignored_file; extern int advice_add_empty_pathspec; +extern int advice_mixed_pathspec_magic; /* * To add a new advice, you need to:
@@ -51,6 +52,7 @@ extern int advice_add_empty_pathspec; ADVICE_GRAFT_FILE_DEPRECATED, ADVICE_IGNORED_HOOK, ADVICE_IMPLICIT_IDENTITY, + ADVICE_MIXED_PATHSPEC_MAGIC, ADVICE_NESTED_TAG, ADVICE_OBJECT_NAME_WARNING, ADVICE_PUSH_ALREADY_EXISTS,
diff --git c/pathspec.c w/pathspec.c
index 18b3be362a..ce9d1738a8 100644
--- c/pathspec.c
+++ w/pathspec.c@@ -336,6 +336,32 @@ static const char *parse_long_magic(unsigned *magic, int *prefix_len, return pos; } +/* + * Give hint for a common mistake of mixing short and long + * form of pathspec magic + */ +static void warn_mixed_magic(unsigned magic, const char *elem, const char *pos) +{ + struct strbuf longform = STRBUF_INIT; + int i; + + if (!advice_enabled(ADVICE_MIXED_PATHSPEC_MAGIC)) + return; + for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) { + if (pathspec_magic[i].bit & magic) { + if (longform.len) + strbuf_addch(&longform, ','); + strbuf_addstr(&longform, pathspec_magic[i].name); + } + } + advise(_("'%.*s(...': cannot mix short- and longform pathspec magic"), + (int)(pos - elem), elem); + advise(_("instead, spell the shortform magic '%.*s' as '%s' inside :(..."), + (int)(pos - (elem + 1)), elem + 1, + longform.buf); +} + + /* * Parse the pathspec element looking for short magic *
@@ -356,6 +382,9 @@ static const char *parse_short_magic(unsigned *magic, const char *elem) continue; } + if (ch == '(') + warn_mixed_magic(*magic, elem, pos); + if (!is_pathspec_magic(ch)) break;