From 8a42c9850177cc91e9f38779e8aca89682a02975 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 6 Apr 2011 16:11:56 -0700 Subject: magic pathspec: add tentative ":/path/from/top/level" pathspec support Support ":/" magic string that can be prefixed to a pathspec element to say "this names the path from the top-level of the working tree", when you are in the subdirectory. For example, you should be able to say: $ edit Makefile ;# top-level $ cd Documentation $ edit git.txt ;# in the subdirectory and then do one of three things, still inside the subdirectory: $ git add -u . ;# add only Documentation/git.txt $ git add -u :/ ;# add everything, including paths outside Documentation $ git add -u ;# whatever the default setting is. To truly support magic pathspec, the API needs to be restructured so that get_pathspec() and init_pathspec() are unified into one call. Currently, the former just prefixes the user supplied pathspec with the current subdirectory path, and the latter takes the output from the former and pre-parses them into a bit richer structure for easier handling. They should become a single API function that takes the current subdirectory path and the remainder of argv[] (after parsing --options and revision arguments from the command line) and returns an array of parsed pathspec elements, and "magic" should become attributes of struct pathspec_item. This patch implements only "top" magic because it can be hacked into the system without such a refactoring. The syntax for magic pathspec prefix is designed to be extensible yet simple to type to invoke a simple magic like "from the top". The parser for the magic prefix is hooked into get_pathspec() function in this patch, and it needs to be moved when we refactor the API. But we have to start from somewhere. Signed-off-by: Junio C Hamano --- setup.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 96 insertions(+), 2 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index 03cd84f2f..820ed05b2 100644 --- a/setup.c +++ b/setup.c @@ -126,6 +126,101 @@ void verify_non_filename(const char *prefix, const char *arg) "Use '--' to separate filenames from revisions", arg); } +/* + * Magic pathspec + * + * NEEDSWORK: These need to be moved to dir.h or even to a new + * pathspec.h when we restructure get_pathspec() users to use the + * "struct pathspec" interface. + * + * Possible future magic semantics include stuff like: + * + * { PATHSPEC_NOGLOB, '!', "noglob" }, + * { PATHSPEC_ICASE, '\0', "icase" }, + * { PATHSPEC_RECURSIVE, '*', "recursive" }, + * { PATHSPEC_REGEXP, '\0', "regexp" }, + * + */ +#define PATHSPEC_FROMTOP (1<<0) + +struct pathspec_magic { + unsigned bit; + char mnemonic; /* this cannot be ':'! */ + const char *name; +} pathspec_magic[] = { + { PATHSPEC_FROMTOP, '/', "top" }, +}; + +/* + * Take an element of a pathspec and check for magic signatures. + * Append the result to the prefix. + * + * For now, we only parse the syntax and throw out anything other than + * "top" magic. + * + * NEEDSWORK: This needs to be rewritten when we start migrating + * get_pathspec() users to use the "struct pathspec" interface. For + * example, a pathspec element may be marked as case-insensitive, but + * the prefix part must always match literally, and a single stupid + * string cannot express such a case. + */ +const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt) +{ + unsigned magic = 0; + const char *copyfrom = elt; + int i; + + if (elt[0] != ':') { + ; /* nothing to do */ + } else if (elt[1] == '(') { + /* longhand */ + const char *nextat; + for (copyfrom = elt + 2; + *copyfrom && *copyfrom != ')'; + copyfrom = nextat) { + size_t len = strcspn(copyfrom, ",)"); + if (copyfrom[len] == ')') + nextat = copyfrom + len; + else + nextat = copyfrom + len + 1; + if (!len) + continue; + for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) + if (strlen(pathspec_magic[i].name) == len && + !strncmp(pathspec_magic[i].name, copyfrom, len)) { + magic |= pathspec_magic[i].bit; + break; + } + if (ARRAY_SIZE(pathspec_magic) <= i) + die("Invalid pathspec magic '%.*s' in '%s'", + (int) len, copyfrom, elt); + } + if (*copyfrom == ')') + copyfrom++; + } else { + /* shorthand */ + for (copyfrom = elt + 1; + *copyfrom && *copyfrom != ':'; + copyfrom++) { + char ch = *copyfrom; + for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) + if (pathspec_magic[i].mnemonic == ch) { + magic |= pathspec_magic[i].bit; + break; + } + if (ARRAY_SIZE(pathspec_magic) <= i) + break; + } + if (*copyfrom == ':') + copyfrom++; + } + + if (magic & PATHSPEC_FROMTOP) + return xstrdup(copyfrom); + else + return prefix_path(prefix, prefixlen, copyfrom); +} + const char **get_pathspec(const char *prefix, const char **pathspec) { const char *entry = *pathspec; @@ -147,8 +242,7 @@ const char **get_pathspec(const char *prefix, const char **pathspec) dst = pathspec; prefixlen = prefix ? strlen(prefix) : 0; while (*src) { - const char *p = prefix_path(prefix, prefixlen, *src); - *(dst++) = p; + *(dst++) = prefix_pathspec(prefix, prefixlen, *src); src++; } *dst = NULL; -- cgit v1.2.1 From 2f6c9760debfb4705f6efb5862e2b3a23b2b951c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 8 Apr 2011 16:18:46 -0700 Subject: magic pathspec: futureproof shorthand form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier design was to take whatever non-alnum that the short format parser happens to support, leaving the rest as part of the pattern, so a version of git that knows '*' magic and a version that does not would have behaved differently when given ":*Makefile". The former would have applied the '*' magic to the pattern "Makefile", while the latter would used no magic to the pattern "*Makefile". Instead, just reserve all non-alnum ASCII letters that are neither glob nor regexp special as potential magic signature, and when we see a magic that is not supported, die with an error message, just like the longhand codepath does. With this, ":%#!*Makefile" will always mean "%#!" magic applied to the pattern "*Makefile", no matter what version of git is used (it is a different matter if the version of git supports all of these three magic matching rules). Also make ':' without anything else to mean "there is no pathspec". This would allow differences between "git log" and "git log ." run from the top level of the working tree (the latter simplifies no-op commits away from the history) to be expressed from a subdirectory by saying "git log :". Helped-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- setup.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index 820ed05b2..5048252d7 100644 --- a/setup.c +++ b/setup.c @@ -197,19 +197,26 @@ const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt) } if (*copyfrom == ')') copyfrom++; + } else if (!elt[1]) { + /* Just ':' -- no element! */ + return NULL; } else { /* shorthand */ for (copyfrom = elt + 1; *copyfrom && *copyfrom != ':'; copyfrom++) { char ch = *copyfrom; + + if (!is_pathspec_magic(ch)) + break; for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) if (pathspec_magic[i].mnemonic == ch) { magic |= pathspec_magic[i].bit; break; } if (ARRAY_SIZE(pathspec_magic) <= i) - break; + die("Unimplemented pathspec magic '%c' in '%s'", + ch, elt); } if (*copyfrom == ':') copyfrom++; -- cgit v1.2.1 From d0546e2d488b1ba185c430b638619ab1d91af509 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 6 Apr 2011 20:56:19 -0700 Subject: magic pathspec: add ":(icase)path" to match case insensitively Signed-off-by: Junio C Hamano --- setup.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index 5048252d7..51e354ca0 100644 --- a/setup.c +++ b/setup.c @@ -136,12 +136,12 @@ void verify_non_filename(const char *prefix, const char *arg) * Possible future magic semantics include stuff like: * * { PATHSPEC_NOGLOB, '!', "noglob" }, - * { PATHSPEC_ICASE, '\0', "icase" }, * { PATHSPEC_RECURSIVE, '*', "recursive" }, * { PATHSPEC_REGEXP, '\0', "regexp" }, * */ #define PATHSPEC_FROMTOP (1<<0) +#define PATHSPEC_ICASE (1<<1) struct pathspec_magic { unsigned bit; @@ -149,6 +149,7 @@ struct pathspec_magic { const char *name; } pathspec_magic[] = { { PATHSPEC_FROMTOP, '/', "top" }, + { PATHSPEC_ICASE, '\0', "icase" }, }; /* @@ -168,7 +169,8 @@ const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt) { unsigned magic = 0; const char *copyfrom = elt; - int i; + const char *retval; + int i, free_source = 0; if (elt[0] != ':') { ; /* nothing to do */ @@ -222,10 +224,31 @@ const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt) copyfrom++; } + if (magic & PATHSPEC_ICASE) { + struct strbuf sb = STRBUF_INIT; + for (i = 0; copyfrom[i]; i++) { + int ch = copyfrom[i]; + if (('a' <= ch && ch <= 'z') || + ('A' <= ch && ch <= 'Z')) { + strbuf_addf(&sb, "[%c%c]", + tolower(ch), toupper(ch)); + } else { + strbuf_addch(&sb, ch); + } + } + if (sb.len) { + free_source = 1; + copyfrom = strbuf_detach(&sb, NULL); + } + } + if (magic & PATHSPEC_FROMTOP) - return xstrdup(copyfrom); + retval = xstrdup(copyfrom); else - return prefix_path(prefix, prefixlen, copyfrom); + retval = prefix_path(prefix, prefixlen, copyfrom); + if (free_source) + free((char *)copyfrom); + return retval; } const char **get_pathspec(const char *prefix, const char **pathspec) -- cgit v1.2.1 From 6d9429271013898df103f7e77ed0736cdfab01b8 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 10 May 2011 10:23:41 -0700 Subject: Revert "magic pathspec: add ":(icase)path" to match case insensitively" This reverts commit d0546e2d488b1ba185c430b638619ab1d91af509, which was only meant to be a Proof-of-concept used during the discussion. The real implementation of the feature needs to wait until we migrate all the code to use "struct pathspec", not "char **", to represent richer semantics given to pathspec. --- setup.c | 31 ++++--------------------------- 1 file changed, 4 insertions(+), 27 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index 51e354ca0..5048252d7 100644 --- a/setup.c +++ b/setup.c @@ -136,12 +136,12 @@ void verify_non_filename(const char *prefix, const char *arg) * Possible future magic semantics include stuff like: * * { PATHSPEC_NOGLOB, '!', "noglob" }, + * { PATHSPEC_ICASE, '\0', "icase" }, * { PATHSPEC_RECURSIVE, '*', "recursive" }, * { PATHSPEC_REGEXP, '\0', "regexp" }, * */ #define PATHSPEC_FROMTOP (1<<0) -#define PATHSPEC_ICASE (1<<1) struct pathspec_magic { unsigned bit; @@ -149,7 +149,6 @@ struct pathspec_magic { const char *name; } pathspec_magic[] = { { PATHSPEC_FROMTOP, '/', "top" }, - { PATHSPEC_ICASE, '\0', "icase" }, }; /* @@ -169,8 +168,7 @@ const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt) { unsigned magic = 0; const char *copyfrom = elt; - const char *retval; - int i, free_source = 0; + int i; if (elt[0] != ':') { ; /* nothing to do */ @@ -224,31 +222,10 @@ const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt) copyfrom++; } - if (magic & PATHSPEC_ICASE) { - struct strbuf sb = STRBUF_INIT; - for (i = 0; copyfrom[i]; i++) { - int ch = copyfrom[i]; - if (('a' <= ch && ch <= 'z') || - ('A' <= ch && ch <= 'Z')) { - strbuf_addf(&sb, "[%c%c]", - tolower(ch), toupper(ch)); - } else { - strbuf_addch(&sb, ch); - } - } - if (sb.len) { - free_source = 1; - copyfrom = strbuf_detach(&sb, NULL); - } - } - if (magic & PATHSPEC_FROMTOP) - retval = xstrdup(copyfrom); + return xstrdup(copyfrom); else - retval = prefix_path(prefix, prefixlen, copyfrom); - if (free_source) - free((char *)copyfrom); - return retval; + return prefix_path(prefix, prefixlen, copyfrom); } const char **get_pathspec(const char *prefix, const char **pathspec) -- cgit v1.2.1 From b060ce7de42b357af013909039da3f08a68f3c0b Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 10 May 2011 12:07:12 -0700 Subject: pathspec: drop "lone : means no pathspec" from get_pathspec() We may want to give the pathspec subsystem such a feature, but not while we are still using get_pathspec() that returns a stupid "char **" that loses subtle nuances that existed in the input string. In the meantime, the callers of get_pathspec() that want to support it could do an equivalent before feeding their argv[] to the function themselves quite easily. Signed-off-by: Junio C Hamano --- setup.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index 5048252d7..84f71d5ee 100644 --- a/setup.c +++ b/setup.c @@ -197,9 +197,6 @@ const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt) } if (*copyfrom == ')') copyfrom++; - } else if (!elt[1]) { - /* Just ':' -- no element! */ - return NULL; } else { /* shorthand */ for (copyfrom = elt + 1; -- cgit v1.2.1 From 2e83b66c32c1d482575fd8caed80680a2f69c5f1 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 10 May 2011 12:02:54 -0700 Subject: fix overslow :/no-such-string-ever-existed diagnostics "git cmd :/no-such-string-ever-existed" runs an extra round of get_sha1() since 009fee4 (Detailed diagnosis when parsing an object name fails., 2009-12-07). Once without error diagnosis to see there is no commit with such a string in the log message (hence "it cannot be a ref"), and after seeing that :/no-such-string-ever-existed is not a filename (hence "it cannot be a path, either"), another time to give "better diagnosis". The thing is, the second time it runs, we already know that traversing the history all the way down to the root will _not_ find any matching commit. Rename misguided "gently" parameter, which is turned off _only_ when the "detailed diagnosis" codepath knows that it cannot be a ref and making the call only for the caller to die with a message. Flip its meaning (and adjust the callers) and call it "only_to_die", which is not a great name, but it describes far more clearly what the codepaths that switches their behaviour based on this variable do. On my box, the command spends ~1.8 seconds without the patch to make the report; with the patch it spends ~1.12 seconds. Signed-off-by: Junio C Hamano --- setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index 84f71d5ee..7fde4fac9 100644 --- a/setup.c +++ b/setup.c @@ -86,7 +86,7 @@ static void NORETURN die_verify_filename(const char *prefix, const char *arg) unsigned char sha1[20]; unsigned mode; /* try a detailed diagnostic ... */ - get_sha1_with_mode_1(arg, sha1, &mode, 0, prefix); + get_sha1_with_mode_1(arg, sha1, &mode, 1, prefix); /* ... or fall back the most general message. */ die("ambiguous argument '%s': unknown revision or path not in the working tree.\n" "Use '--' to separate paths from revisions", arg); -- cgit v1.2.1 From 0e539dca51c298ca2ee102e0ca118797f2da99eb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 10 May 2011 12:05:01 -0700 Subject: rev/path disambiguation: further restrict "misspelled index entry" diag A colon followed by anything !isalnum() (e.g. ":/heh") at this point is known not to be an existing rev. Just give a generic "neither a rev nor a path" error message. Signed-off-by: Junio C Hamano --- setup.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index 7fde4fac9..fd4ce59f2 100644 --- a/setup.c +++ b/setup.c @@ -85,8 +85,17 @@ static void NORETURN die_verify_filename(const char *prefix, const char *arg) { unsigned char sha1[20]; unsigned mode; - /* try a detailed diagnostic ... */ - get_sha1_with_mode_1(arg, sha1, &mode, 1, prefix); + + /* + * Saying "'(icase)foo' does not exist in the index" when the + * user gave us ":(icase)foo" is just stupid. A magic pathspec + * begins with a colon and is followed by a non-alnum; do not + * let get_sha1_with_mode_1(only_to_die=1) to even trigger. + */ + if (!(arg[0] == ':' && !isalnum(arg[1]))) + /* try a detailed diagnostic ... */ + get_sha1_with_mode_1(arg, sha1, &mode, 1, prefix); + /* ... or fall back the most general message. */ die("ambiguous argument '%s': unknown revision or path not in the working tree.\n" "Use '--' to separate paths from revisions", arg); -- cgit v1.2.1 From 488201c87e284ae06323b534c31e354811fb0d51 Mon Sep 17 00:00:00 2001 From: Ramsay Jones Date: Tue, 17 May 2011 18:43:10 +0100 Subject: setup.c: Fix some "symbol not declared" sparse warnings In particular, sparse issues the "symbol 'a_symbol' was not declared. Should it be static?" warnings for the following symbols: setup.c:159:3: 'pathspec_magic' setup.c:176:12: 'prefix_pathspec' These symbols only require file scope, so we add the static modifier to their declarations. Signed-off-by: Ramsay Jones Signed-off-by: Junio C Hamano --- setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index fd4ce59f2..d7d8e3efb 100644 --- a/setup.c +++ b/setup.c @@ -152,7 +152,7 @@ void verify_non_filename(const char *prefix, const char *arg) */ #define PATHSPEC_FROMTOP (1<<0) -struct pathspec_magic { +static struct pathspec_magic { unsigned bit; char mnemonic; /* this cannot be ':'! */ const char *name; @@ -173,7 +173,7 @@ struct pathspec_magic { * the prefix part must always match literally, and a single stupid * string cannot express such a case. */ -const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt) +static const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt) { unsigned magic = 0; const char *copyfrom = elt; -- cgit v1.2.1