* [PATCH 0/4] Redoing the "add -u" migration plan @ 2011-04-07 1:16 Junio C Hamano 2011-04-07 1:16 ` [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support Junio C Hamano ` (3 more replies) 0 siblings, 4 replies; 35+ messages in thread From: Junio C Hamano @ 2011-04-07 1:16 UTC (permalink / raw) To: git So here is the new migration plan to make "add -u" without pathspec to default to the tree wide operation at 1.8.0 boundary. The first patch is more or less the same as the "heads-up" version I sent earlier to implement the magic ":/" pathspec at a wrong level as a hack, but with some documentation updates. It should apply on top of a91df69 (the parent of the first commit in the "jc/add-u-migration" series). Then merge the 33c33ca (the first commit in the "jc/add-u-migration" series) to the result, and then apply the remainder of this series. The second patch gets rid of treewideupdate configuration variable, as we no longer rely on user preference for this migration plan, and rewords the warning message. I did it this way only because the first commit in the old series is already in 'next'; I will redo the series after 1.7.5 ships so that we do not have to have this patch, nor "a configuration appears and then disappears". The third (step 2) patch is what should happen at 1.8.0 boundary by flipping the default, but still keeps the warning for people who missed the late 1.7.X series. The last (step 3) patch is to remove the warning long after 1.8.0 boundary when everybody got used to the new behaviour. Junio C Hamano (4): magic pathspec: add tentative ":/path/from/top/level" pathspec support add -u: get rid of "treewideupdate" configuration add: make "add -u/-A" update full tree without pathspec (step 2) add: make "add -u/-A" update full tree without pathspec (step 3) ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-07 1:16 [PATCH 0/4] Redoing the "add -u" migration plan Junio C Hamano @ 2011-04-07 1:16 ` Junio C Hamano 2011-04-07 1:40 ` Junio C Hamano 2011-04-07 13:23 ` Nguyen Thai Ngoc Duy 2011-04-07 1:16 ` [PATCH 2/4] add -u: get rid of "treewideupdate" configuration Junio C Hamano ` (2 subsequent siblings) 3 siblings, 2 replies; 35+ messages in thread From: Junio C Hamano @ 2011-04-07 1:16 UTC (permalink / raw) To: git 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 is the only kind of magic that can be hacked into the system without such a refactoring. Other types of magic that are envisioned (e.g. "icase") needs to be able to express more than what a simple string can encode and needs to wait. 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 <gitster@pobox.com> --- Documentation/glossary-content.txt | 31 +++++++++++- setup.c | 98 +++++++++++++++++++++++++++++++++++- 2 files changed, 126 insertions(+), 3 deletions(-) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index 33716a3..e51d7e6 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -277,7 +277,8 @@ This commit is referred to as a "merge commit", or sometimes just a Pattern used to specify paths. + Pathspecs are used on the command line of "git ls-files", "git -ls-tree", "git grep", "git checkout", and many other commands to +ls-tree", "git add", "git grep", "git diff", "git checkout", +and many other commands to limit the scope of operations to some subset of the tree or worktree. See the documentation of each command for whether paths are relative to the current directory or toplevel. The @@ -296,6 +297,34 @@ For example, Documentation/*.jpg will match all .jpg files in the Documentation subtree, including Documentation/chapter_1/figure_1.jpg. ++ +A pathspec that begins with a colon `:` has special meaning. In the +short form, the leading colon `:` is followed by zero or more "magic +signature" letters (which optionally is terminated by another colon `:`), +and the remainder is the pattern to match against the path. The optional +colon that terminates the "magic signature" can be omitted if the pattern +begins with a character that cannot be a "magic signature" and is not a +colon. ++ +In the long form, the leading colon `:` is followed by a open +parenthesis `(`, a comma-separated list of zero or more "magic words", +and a close parentheses `)`, and the remainder is the pattern to match +against the path. ++ +The "magic signature" consists of an ASCII symbol that is not +alphanumeric. ++ +-- +top `/`;; + The magic word `top` (mnemonic: `/`) makes the pattern match + from the root of the working tree, even when you are running + the command from inside a subdirectory. +-- ++ +Currently only the slash `/` is recognized as the "magic signature", +but it is envisioned that we will support more types of magic in later +versions of git. + [[def_parent]]parent:: A <<def_commit_object,commit object>> contains a (possibly empty) list of the logical predecessor(s) in the line of development, i.e. its diff --git a/setup.c b/setup.c index 03cd84f..820ed05 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; -- 1.7.5.rc1 ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-07 1:16 ` [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support Junio C Hamano @ 2011-04-07 1:40 ` Junio C Hamano 2011-04-07 13:09 ` Nguyen Thai Ngoc Duy 2011-04-07 13:23 ` Nguyen Thai Ngoc Duy 1 sibling, 1 reply; 35+ messages in thread From: Junio C Hamano @ 2011-04-07 1:40 UTC (permalink / raw) To: git; +Cc: Nguyễn Thái Ngọc Duy Junio C Hamano <gitster@pobox.com> writes: > This patch implements only "top" magic because it is the only kind of > magic that can be hacked into the system without such a refactoring. > Other types of magic that are envisioned (e.g. "icase") needs to be able > to express more than what a simple string can encode and needs to wait. Actually "icase" could be implemented inside get_pathspec() by doing something like the attached patch. But "noglob" needs support from the "struct pathspec_item" infrastructure. It is insufficient to parse the magic signature at get_pathspec() level, as I do not see a way to encode these magic in the match string. I suspect that Duy's favourite "negate" cannot be done if we discard the magic information at the get_pathspec() level, either. --- Documentation/glossary-content.txt | 7 +++++-- setup.c | 31 +++++++++++++++++++++++++++---- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index e51d7e6..0ca029b 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -319,10 +319,13 @@ top `/`;; The magic word `top` (mnemonic: `/`) makes the pattern match from the root of the working tree, even when you are running the command from inside a subdirectory. +icase;; + The magic word `icase` (there is no mnemonic for it) makes the + pattern match case insensitively. E.g. `:(icase)makefile` matches + both `Makefile` and `makefile`. -- + -Currently only the slash `/` is recognized as the "magic signature", -but it is envisioned that we will support more types of magic in later +It is envisioned that we will support more types of magic in later versions of git. [[def_parent]]parent:: diff --git a/setup.c b/setup.c index 820ed05..e66ffbe 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 */ @@ -215,10 +217,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) ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-07 1:40 ` Junio C Hamano @ 2011-04-07 13:09 ` Nguyen Thai Ngoc Duy 2011-04-07 18:28 ` Junio C Hamano 0 siblings, 1 reply; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-04-07 13:09 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 2011/4/7 Junio C Hamano <gitster@pobox.com>: > + if (('a' <= ch && ch <= 'z') || > + ('A' <= ch && ch <= 'Z')) { > + strbuf_addf(&sb, "[%c%c]", > + tolower(ch), toupper(ch)); Nice try. You know you are going to pay a high performance price for that, don't you ;) Maybe also worth mentioning in document that this applies to ASCII charset only (as opposed to Unicode). -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-07 13:09 ` Nguyen Thai Ngoc Duy @ 2011-04-07 18:28 ` Junio C Hamano 2011-04-08 11:39 ` Nguyen Thai Ngoc Duy 0 siblings, 1 reply; 35+ messages in thread From: Junio C Hamano @ 2011-04-07 18:28 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: git Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > 2011/4/7 Junio C Hamano <gitster@pobox.com>: >> + if (('a' <= ch && ch <= 'z') || >> + ('A' <= ch && ch <= 'Z')) { >> + strbuf_addf(&sb, "[%c%c]", >> + tolower(ch), toupper(ch)); > > Nice try. You know you are going to pay a high performance price for > that, don't you ;) Maybe also worth mentioning in document that this > applies to ASCII charset only (as opposed to Unicode). You know this is a throw-away patch, just to illustrate that some things are doable with a hack to add more code to get_pathspec(), while others would need a bigger restructuring, don't you? Besides, _if_ the user wants to do something costly, as long as the implementation does not harm common cases, it _still_ is better to make the code do the work, no? ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-07 18:28 ` Junio C Hamano @ 2011-04-08 11:39 ` Nguyen Thai Ngoc Duy 0 siblings, 0 replies; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-04-08 11:39 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Fri, Apr 8, 2011 at 1:28 AM, Junio C Hamano <gitster@pobox.com> wrote: > Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > >> 2011/4/7 Junio C Hamano <gitster@pobox.com>: >>> + if (('a' <= ch && ch <= 'z') || >>> + ('A' <= ch && ch <= 'Z')) { >>> + strbuf_addf(&sb, "[%c%c]", >>> + tolower(ch), toupper(ch)); >> >> Nice try. You know you are going to pay a high performance price for >> that, don't you ;) Maybe also worth mentioning in document that this >> applies to ASCII charset only (as opposed to Unicode). > > You know this is a throw-away patch, just to illustrate that some things > are doable with a hack to add more code to get_pathspec(), while others > would need a bigger restructuring, don't you? > > Besides, _if_ the user wants to do something costly, as long as the > implementation does not harm common cases, it _still_ is better to make > the code do the work, no? I was thinking of the latter rather than the former. And I agree. Just wondering how slow it (fnmatch in general) can be compared to plain prefix. I guess we will know soon if this patch gets in and users start to use it. -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-07 1:16 ` [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support Junio C Hamano 2011-04-07 1:40 ` Junio C Hamano @ 2011-04-07 13:23 ` Nguyen Thai Ngoc Duy 2011-04-07 16:18 ` Junio C Hamano 1 sibling, 1 reply; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-04-07 13:23 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Thu, Apr 7, 2011 at 8:16 AM, Junio C Hamano <gitster@pobox.com> wrote: > +The "magic signature" consists of an ASCII symbol that is not > +alphanumeric. Except dot, as Michael pointed out in another email, so we can write ":/.foo" instead of ":/:.foo". I'm tempted to rule out wildcards as well (star, backslash, question mark and square brackets) Also the patch does not catch this (ie. not die() on unrecognized signature). -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-07 13:23 ` Nguyen Thai Ngoc Duy @ 2011-04-07 16:18 ` Junio C Hamano 2011-04-08 12:00 ` Nguyen Thai Ngoc Duy 0 siblings, 1 reply; 35+ messages in thread From: Junio C Hamano @ 2011-04-07 16:18 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: git Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > Also the patch does not catch this (ie. not die() on unrecognized signature). Of course it doesn't. I didn't say "everything that is not alphanumeric is magic". I only said "all the magic are not alphanumeric". Notice the difference. This is so that you can say ":/.gitignore" and do not have to say ":/:.gitignore". I am also tempted to special case a ':' followed by a zero magic as if it specifies the top magic, e.g. ":Makefile" is the same as ":(top)Makefile", aka ":/Makefile". It is not in the published code, but the short-cut would help ":.gitignore". You don't require, nor allowed to have, a colon after ')' when writing a long hand, by the way. There is no reason to require one for disambiguation. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-07 16:18 ` Junio C Hamano @ 2011-04-08 12:00 ` Nguyen Thai Ngoc Duy 2011-04-08 15:05 ` Junio C Hamano 0 siblings, 1 reply; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-04-08 12:00 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Thu, Apr 7, 2011 at 11:18 PM, Junio C Hamano <gitster@pobox.com> wrote: > Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > >> Also the patch does not catch this (ie. not die() on unrecognized signature). > > Of course it doesn't. > > I didn't say "everything that is not alphanumeric is magic". I only said > "all the magic are not alphanumeric". Notice the difference. > > > This is so that you can say ":/.gitignore" and do not have to say > ":/:.gitignore". But then, say people have a file named @foo at top dir. They can write ":/@foo" to address the file. Some time later we decide to use '@' as magic, how can we re-train user's fingers? > I am also tempted to special case a ':' followed by a zero magic as if it > specifies the top magic, e.g. ":Makefile" is the same as ":(top)Makefile", > aka ":/Makefile". It is not in the published code, but the short-cut > would help ":.gitignore". If you're not too tempted to do it, then reserve the case (ie. die()). Although I quite like it, one less keystroke for me. -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-08 12:00 ` Nguyen Thai Ngoc Duy @ 2011-04-08 15:05 ` Junio C Hamano 2011-04-08 15:39 ` Nguyen Thai Ngoc Duy 2011-04-08 16:37 ` Junio C Hamano 0 siblings, 2 replies; 35+ messages in thread From: Junio C Hamano @ 2011-04-08 15:05 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: git Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > On Thu, Apr 7, 2011 at 11:18 PM, Junio C Hamano <gitster@pobox.com> wrote: > >> This is so that you can say ":/.gitignore" and do not have to say >> ":/:.gitignore". > > But then, say people have a file named @foo at top dir. They can write > ":/@foo" to address the file. Some time later we decide to use '@' as > magic, how can we re-train user's fingers? You don't. The primary goal of short form is to be short to type from the command line, and if you are in doubt, you can always disambiguate by saying ":/:@foo", and you can use the terminating colon even if you are sure "@" is not a magic in your version of git. Scripts can and should use the long form for readability and compatibility. > Although I quite like it, one less keystroke for me. Exactly. It is Ok if the short form is a bit more complex to explain than the long form (which should be very easy to explain). The goal of the short form is to make the end result is short to type, once the user understands the rules. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-08 15:05 ` Junio C Hamano @ 2011-04-08 15:39 ` Nguyen Thai Ngoc Duy 2011-04-08 16:37 ` Junio C Hamano 1 sibling, 0 replies; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-04-08 15:39 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Fri, Apr 8, 2011 at 10:05 PM, Junio C Hamano <gitster@pobox.com> wrote: > Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > >> On Thu, Apr 7, 2011 at 11:18 PM, Junio C Hamano <gitster@pobox.com> wrote: >> >>> This is so that you can say ":/.gitignore" and do not have to say >>> ":/:.gitignore". >> >> But then, say people have a file named @foo at top dir. They can write >> ":/@foo" to address the file. Some time later we decide to use '@' as >> magic, how can we re-train user's fingers? > > You don't. The primary goal of short form is to be short to type from the > command line, and if you are in doubt, you can always disambiguate by > saying ":/:@foo", and you can use the terminating colon even if you are > sure "@" is not a magic in your version of git. If you allow me to use ":/@foo", I would (because it's convenient). And over time it will be carved in my muscle memory. Doubts and surprises after that are not good. Suppose I usually do "git co :/@foo", then '@' in later versions means many files, not just '@foo' at top. The '@' magic surprise would upset (enrage in the first few minutes maybe) me. My argument would be "it used to work fine" (against "you should use 'git co :/:@foo'" because it's less convenient that way). -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-08 15:05 ` Junio C Hamano 2011-04-08 15:39 ` Nguyen Thai Ngoc Duy @ 2011-04-08 16:37 ` Junio C Hamano 2011-04-08 17:02 ` Nguyen Thai Ngoc Duy 1 sibling, 1 reply; 35+ messages in thread From: Junio C Hamano @ 2011-04-08 16:37 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: git Junio C Hamano <gitster@pobox.com> writes: > Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > >> On Thu, Apr 7, 2011 at 11:18 PM, Junio C Hamano <gitster@pobox.com> wrote: >> >>> This is so that you can say ":/.gitignore" and do not have to say >>> ":/:.gitignore". >> >> But then, say people have a file named @foo at top dir. They can write >> ":/@foo" to address the file. Some time later we decide to use '@' as >> magic, how can we re-train user's fingers? > > You don't. The primary goal of short form is to be short to type from the > command line, and if you are in doubt, you can always disambiguate by > saying ":/:@foo", and you can use the terminating colon even if you are > sure "@" is not a magic in your version of git. Actually, after thinking a bit more about it, I changed my mind. It is not such a big deal to require the terminating colon for a pathname that begins with a nonalnum (except for '.' and we might find some others perhaps '_'), as they are rare. I agree that we should reserve most of the nonalnum ASCII as potential magic, and require the terminating colon if the user wants to start the pattern part with a potential magic signature letter and error out if we see a magic that we do not support yet. The reason I said "most of" is that we should exclude some non-alnum letters that often begin a filename. Off the top of my head, we should exclude "." (period -- dot-files are common), "_" (underscore), and possibly "+" (plus) and "=" (equal), as I saw these used to start filenames, but the latter two are rather rare and I don't mind requiring the terminating colon after the magic signature. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support 2011-04-08 16:37 ` Junio C Hamano @ 2011-04-08 17:02 ` Nguyen Thai Ngoc Duy 0 siblings, 0 replies; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-04-08 17:02 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Fri, Apr 8, 2011 at 11:37 PM, Junio C Hamano <gitster@pobox.com> wrote: > Off the top of my head, we should exclude "." (period -- dot-files are > common), "_" (underscore), and possibly "+" (plus) and "=" (equal), as I > saw these used to start filenames, but the latter two are rather rare and > I don't mind requiring the terminating colon after the magic signature. Totally agree on dot and underscore. I don't really care the other two. While '*' is not actually part of path name, '*.[ch]' is commonly used, and I usually have to quote once to avoid bash expansion already. So put asterisk in exclude list too, to avoid another quote? -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-07 1:16 [PATCH 0/4] Redoing the "add -u" migration plan Junio C Hamano 2011-04-07 1:16 ` [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support Junio C Hamano @ 2011-04-07 1:16 ` Junio C Hamano 2011-04-08 17:54 ` Jeff King 2011-04-07 1:16 ` [PATCH 3/4] add: make "add -u/-A" update full tree without pathspec (step 2) Junio C Hamano 2011-04-07 1:16 ` [PATCH 4/4] add: make "add -u/-A" update full tree without pathspec (step 3) Junio C Hamano 3 siblings, 1 reply; 35+ messages in thread From: Junio C Hamano @ 2011-04-07 1:16 UTC (permalink / raw) To: git Thanks to the magic ":/" pathspec, it is much easier to invoke both tree-wide operation and limited-to-cwd operation on demand from the command line. What remains is the downside of the configuration variable, namely, that it makes git behave differently depending on who you are and in which repository you are using it, hence making it harder to help and/or teach others. Remove the configuration variable, and adjust the warning message. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/add.c | 26 ++++++-------------------- t/t2200-add-update.sh | 29 +++++++++++++++++++---------- 2 files changed, 25 insertions(+), 30 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 595f5cc..f58d1cf 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -310,7 +310,6 @@ static const char ignore_error[] = static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0; static int ignore_add_errors, addremove, intent_to_add, ignore_missing = 0; -static int default_tree_wide_update = -1; static struct option builtin_add_options[] = { OPT__DRY_RUN(&show_only, "dry run"), @@ -336,10 +335,6 @@ static int add_config(const char *var, const char *value, void *cb) ignore_add_errors = git_config_bool(var, value); return 0; } - if (!strcasecmp(var, "add.treewideupdate")) { - default_tree_wide_update = git_config_bool(var, value); - return 0; - } return git_default_config(var, value, cb); } @@ -368,15 +363,10 @@ static const char *warn_add_uA_180_migration_msg[] = { "In release 1.8.0, running 'git add -u' (or 'git add -A') from", "a subdirectory without giving any pathspec WILL take effect", "on the whole working tree, not just the part under the current", - "directory. You can set add.treewideupdate configuration variable", - "to 'false' to keep the current behaviour.", - "You can set the configuration variable to 'true' to make the", - "'git add -u/-A' command without pathspec take effect on the whole", - "working tree now. If you do so, you can use '.' at the end of", - "the command, e.g. 'git add -u .' when you want to limit the", - "operation to the current directory.", - "This warning will be issued until you set the configuration variable", - "to either 'true' or 'false'." + "directory. Please make it a habit to add '.' when you want to", + "limit the operation to the current directory and below.", + "You can use ':/' at the end of the command to affect the operation", + "on the whole working tree.", }; static int warn_180_migration(void) @@ -419,12 +409,8 @@ int cmd_add(int argc, const char **argv, const char *prefix) die("Option --ignore-missing can only be used together with --dry-run"); if ((addremove || take_worktree_changes) && !argc) { whole_tree_add = 1; - if (prefix) { - if (default_tree_wide_update < 0) - default_tree_wide_update = warn_180_migration(); - if (!default_tree_wide_update) - whole_tree_add = 0; - } + if (prefix) + whole_tree_add = warn_180_migration(); } add_new_files = !take_worktree_changes && !refresh_only; diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index 7ac8b70..f7711ba 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -80,10 +80,9 @@ test_expect_success 'change gets noticed' ' ' -test_expect_success 'update from a subdirectory without pathspec (no config)' ' +test_expect_success 'update from a subdirectory without pathspec' ' # This test needs to be updated to expect the whole tree # update after 1.8.0 migration. - test_might_fail git config --remove add.treewideupdate && test_might_fail git reset check dir1 && echo changed >check && ( @@ -97,15 +96,13 @@ test_expect_success 'update from a subdirectory without pathspec (no config)' ' grep warning expect.warning ' -test_expect_success 'update from a subdirectory without pathspec (local)' ' - test_when_finished "git config --remove add.treewideupdate; :" && - git config add.treewideupdate false && +test_expect_success 'update from a subdirectory with local pathspec' ' test_might_fail git reset check dir1 && echo changed >check && ( cd dir1 && echo even more >sub2 && - git add -u 2>../expect.warning + git add -u . 2>../expect.warning ) && git diff-files --name-only dir1 check >actual && echo check >expect && @@ -113,15 +110,27 @@ test_expect_success 'update from a subdirectory without pathspec (local)' ' ! grep warning expect.warning ' -test_expect_success 'update from a subdirectory without pathspec (global)' ' - test_when_finished "git config --remove add.treewideupdate; :" && - git config add.treewideupdate true && +test_expect_success 'update from a subdirectory with magic pathspec (mnemonic)' ' test_might_fail git reset check dir1 && echo changed >check && ( cd dir1 && echo even more >sub2 && - git add -u 2>../expect.warning + git add -u :/ 2>../expect.warning + ) && + git diff-files --name-only dir1 check >actual && + : >expect && + test_cmp expect actual && + ! grep warning expect.warning +' + +test_expect_success 'update from a subdirectory with magic pathspec (longhand)' ' + test_might_fail git reset check dir1 && + echo changed >check && + ( + cd dir1 && + echo even more >sub2 && + git add -u ":(top)" 2>../expect.warning ) && git diff-files --name-only dir1 check >actual && : >expect && -- 1.7.5.rc1 ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-07 1:16 ` [PATCH 2/4] add -u: get rid of "treewideupdate" configuration Junio C Hamano @ 2011-04-08 17:54 ` Jeff King 2011-04-08 19:27 ` Junio C Hamano 0 siblings, 1 reply; 35+ messages in thread From: Jeff King @ 2011-04-08 17:54 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Wed, Apr 06, 2011 at 06:16:34PM -0700, Junio C Hamano wrote: > Thanks to the magic ":/" pathspec, it is much easier to invoke both > tree-wide operation and limited-to-cwd operation on demand from the > command line. I am mildly negative on this patch. Having the config variable helps two types of users: 1. Ones who see the warning for new behavior, say "great, I've been informed and am ready to use it", and don't want to see the message again. They are stuck typing "./" or ":/" every time, or end up getting spammed by the migration message. 2. Users who prefer the current behavior and would rather keep it. We give them no out except to type "./" every time. Changing the default is one thing; an irate user can see the change and fix it. But to give them no way of changing the default back seems unnecessarily limiting. > What remains is the downside of the configuration variable, > namely, that it makes git behave differently depending on who you are and > in which repository you are using it, hence making it harder to help > and/or teach others. I have never been a fan of this reasoning. Sure, it is slightly harder to help people when the system is configurable. But dropping configurability comes at the cost of people who are using the system day-to-day. And isn't making it pleasant to use every day more important than the minority of times you are telling somebody else how to use it? Besides which, if you are helping somebody remotely or sitting at an unfamiliar git installation, it still wouldn't be safe to recommend pathspec-less "git add -u" without first checking which version of git the person is running (though to be fair, in 2 or 3 years it will be reasonable to assume a certain behavior, and a config option would still exist). -Peff ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-08 17:54 ` Jeff King @ 2011-04-08 19:27 ` Junio C Hamano 2011-04-08 20:24 ` Jeff King 0 siblings, 1 reply; 35+ messages in thread From: Junio C Hamano @ 2011-04-08 19:27 UTC (permalink / raw) To: Jeff King; +Cc: git Jeff King <peff@peff.net> writes: >> What remains is the downside of the configuration variable, >> namely, that it makes git behave differently depending on who you are and >> in which repository you are using it, hence making it harder to help >> and/or teach others. > > I have never been a fan of this reasoning. Sure, it is slightly harder > to help people when the system is configurable. But dropping > configurability comes at the cost of people who are using the system > day-to-day. And isn't making it pleasant to use every day more important > than the minority of times you are telling somebody else how to use it? I probably should stated it differently. I dropped it during this round because they are _not_ needed to help the transition, but it is a possible additional feature. Let me try to explain the train of throught a bit better. People seem to expect "add -u" without any pathspec to work on the whole working tree structure even when you are in a subdirectory, similar to "git commit -a". We will be changing "add -u" in the longer term to do so. After the transition happens, people can easily say "add -u ." to limit it to the current subdirectory, and people can say "add -u ." even today to be explicit (which would help them transitioning). Before the transition, however, there is no pleasant way to cause "add -u" to add everything when you work in a subdirectory, short of saying: (cd $(git rev-parse --show-cdup)/. && git add -u) With ":/", we gain an easy way, "git add -u :/", to say "whole tree". That is the only thing this series does. Up to this part, I think we both agree are good thing. Now, imagine you were born in a world where we had the :/ magic from day one of git. Different commands "git add -u", "git grep", "git ls-files" without pathspec operate differently, and for some reason (e.g. usability, similarity to other people's corresponding command, or historical reasons) some of them operate only within the current subdirectory while others operate on the whole tree. The default behaviour might even be different between versions of git or user configuration. Because we assume that you already have both "." and ":/" to easily be explicit in that world, "teachers and helpers may have to scratch there head wasting their time" is no longer an issue. If you are teaching others, you ought to know about "." and ":/", and whether we add the configuration mechanism or not, you ought to know that you should be explicit to protect yourself from the differences between 1.7.X and 1.8.0 versions in the first place. So I agree that "costs teachers and helpers" is much less of an issue. You can certainly introduce configuration variables e.g. "addu.treewide", "grep.treewide", "lsfiles.treewide" (or even "core.treewide" to cover them all) to change the default behaviour for each user or each repository. The real question would become: if it is worth the maintenance cost of additional code to help users who want to avoid typing "." (or ":/") all the time in the environment they control. I don't know the answer to this question yet. A good new is that we had to conflate the discussion with "but there is no pleasant way to tell 'default to local' commands to work on the whole tree" justification for configuration variables before ":/". With ":/", that excuse will be gone and the discussion can be much more simplified. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-08 19:27 ` Junio C Hamano @ 2011-04-08 20:24 ` Jeff King 2011-04-08 22:22 ` Junio C Hamano 0 siblings, 1 reply; 35+ messages in thread From: Jeff King @ 2011-04-08 20:24 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Fri, Apr 08, 2011 at 12:27:05PM -0700, Junio C Hamano wrote: > > I have never been a fan of this reasoning. Sure, it is slightly harder > > to help people when the system is configurable. But dropping > > configurability comes at the cost of people who are using the system > > day-to-day. And isn't making it pleasant to use every day more important > > than the minority of times you are telling somebody else how to use it? > > I probably should stated it differently. I dropped it during this round > because they are _not_ needed to help the transition, but it is a possible > additional feature. But in my earlier email, one of the users who is helped by this is one who wants to silence the migration warning. So it is somewhat related to the transition. If we were in a world where "." and ":/" had always worked and there was no variable, would I write a patch for the variable? Probably not, especially because I think the full-tree behavior is what I would set it to (and that is going to become the default). But we don't live in that world. We are making a transition, and so it may be worth it to help: 1. People who want the new behavior _now_ without extra typing. 2. Placate people who say "...but I liked the old behavior better". I am in group (1). I don't know if people in group (2) need placating or not, but I have grown to assume there will always be people to complain about a change. :) -Peff ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-08 20:24 ` Jeff King @ 2011-04-08 22:22 ` Junio C Hamano 2011-04-08 22:32 ` Jeff King 0 siblings, 1 reply; 35+ messages in thread From: Junio C Hamano @ 2011-04-08 22:22 UTC (permalink / raw) To: Jeff King; +Cc: git Jeff King <peff@peff.net> writes: > But we don't live in that world. We are making a transition, and so it > may be worth it to help: > > 1. People who want the new behavior _now_ without extra typing. > > 2. Placate people who say "...but I liked the old behavior better". > > I am in group (1). I don't know if people in group (2) need placating or > not, but I have grown to assume there will always be people to complain > about a change. :) Ok, first things first. Any new feature we add we would need to keep forever. I would rather not to see us having to maintain the default behaviour configuration variable forever into the future for these commands (not just add-u, but also grep, ls-files, ...). Now, having said that, we could vastly simplify things. I think the real cause of this confusion is because we have been dead-set assuming that we _have_ to change the default. The logic goes like this: - We will change the default, but we do not want to harm existing users; therefore - We need to introduce the variable to keep the old default not to harm existing users, but it would add to the uncertainty of the default when you do not know which version of git you are using. You now have to also worry about unknown configurations, so we need to train people to explicitly say '.' or ':/'; therefore - We need to issue warnings to train the user, but we do not want to show warnings to users who already learned the change is coming; therefore - We do need the configuration variable, and everybody needs to set it to squelch the warning. Otherwise "add -u" without any pathspec is too noisy to be usable. But I am not happy with the last step of the above deduction for two reasons. The configuration will only help people who have total control of the version of git they have on their boxes. They can say "Ok, I know what I want, and I know which default the version of git I installed on this box, so I set the variable and can forget about it forever, and keep typing 'git add -u' without any pathspec." But that would not work for people who need to work on multiple boxes, some of the boxes still running late 1.7.X while others running 1.8.0 or later, and the version of git they will be running is under sysadmin's control (your "a site that runs unknown version" example). These people need to train their fingers to be explicit when working in a subdirectory and running the command without a pathspec during the transition period anyway. It is a _good_ thing not to have any way to squelch the warning without being explicit in that sense, _if_ we are going to change the default. Another reason, which is worse, is that it would make it _harder_ to help migrating scripts, if the presense of the configuration variable squelched the warning. "Warn when run without pathspec from a subdirectory" would issue the warning every time until the invocation is changed to an explicit one, and your script that runs 'add -u' without pathspec will keep emitting the message until all such invocations are fixed. So let's step back a bit. How about we'd just add ':/' to make it equally easy to switch between "local only" vs "tree-wide" in 1.7.6 release, and be done with it. We don't change the default for any of the commands at all. No need to issue warnings because there won't be any backward imcompatible change. The users fingers do not need any re-training. There is no need to rewrite scripts, either. One good attribute of the combination of short-and-sweet ':/' and existing short-and-sweet '.' is that they make the default immaterial. Since more than a year ago, I've been saying that the ideal is to make the default not matter: http://thread.gmane.org/gmane.comp.version-control.git/133570/focus=133683 If the default does not matter, why change it? It just causes us more headaches for dubious gain, no? IIRC, I think the two reasons why we started discussing of "add -u" and friends were that (1) some commands default to whole-tree while others limit to $cwd --- inconsistency is bad; and (2) when the user wants to do a full tree "add -u", there is no way other than counting the current depth and typing "../" that many times. But when we looked at the current set of commands that limit them to the $cwd, we found that "add -u" was the only one that may make sense to switch the default, meaning that the "consistency" was not something we would even want to shoot for. For example, we want our "git grep -e pattern" to mimic "grep -r -e pattern .". And the second reason is going away, thanks to Michael and Duy's ':/'. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-08 22:22 ` Junio C Hamano @ 2011-04-08 22:32 ` Jeff King 2011-04-08 22:37 ` Junio C Hamano 0 siblings, 1 reply; 35+ messages in thread From: Jeff King @ 2011-04-08 22:32 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Fri, Apr 08, 2011 at 03:22:20PM -0700, Junio C Hamano wrote: > So let's step back a bit. > > How about we'd just add ':/' to make it equally easy to switch between > "local only" vs "tree-wide" in 1.7.6 release, and be done with it. We > don't change the default for any of the commands at all. Yeah, I am beginning to think that is a sensible route. And it commits us to nothing, so if we decide much later that a change of default is sensible, that is still open to us. > Since more than a year ago, I've been saying that the ideal is to make the > default not matter: > > http://thread.gmane.org/gmane.comp.version-control.git/133570/focus=133683 > > If the default does not matter, why change it? It just causes us more > headaches for dubious gain, no? I'm not sure how much you can achieve the "make it not matter". The shorthands go a long way, but I still want git to read my mind about which I wanted to use (and the closest approximation of that, from my experience, would be a per-repo variable). However, having the shorthands mean that we can try them out in the real world and revisit the topic in a year if people still care. > IIRC, I think the two reasons why we started discussing of "add -u" and > friends were that (1) some commands default to whole-tree while others > limit to $cwd --- inconsistency is bad; and (2) when the user wants to do > a full tree "add -u", there is no way other than counting the current > depth and typing "../" that many times. > > But when we looked at the current set of commands that limit them to the $cwd, > we found that "add -u" was the only one that may make sense to switch the > default, meaning that the "consistency" was not something we would even > want to shoot for. For example, we want our "git grep -e pattern" to > mimic "grep -r -e pattern .". I am not sure of that. I thought there was interest in full-tree grep (OK, _I_ had some interst in it). But the same transition pain arguments apply there, and we should be able to do "git grep pattern :/" soon, right? -Peff ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-08 22:32 ` Jeff King @ 2011-04-08 22:37 ` Junio C Hamano 2011-04-08 23:18 ` Junio C Hamano 0 siblings, 1 reply; 35+ messages in thread From: Junio C Hamano @ 2011-04-08 22:37 UTC (permalink / raw) To: Jeff King; +Cc: git Jeff King <peff@peff.net> writes: > I am not sure of that. I thought there was interest in full-tree grep > (OK, _I_ had some interst in it). But the same transition pain > arguments apply there, and we should be able to do "git grep pattern :/" > soon, right? I never tested it myself, but the earlier "support :/ at a wrong level get_pathspec()" patch should take care of "git grep" as well. It is equivalent to the "alternative approach" Michael posted as RFC as a follow-up to his earlier "grep --full-tree" thread. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-08 22:37 ` Junio C Hamano @ 2011-04-08 23:18 ` Junio C Hamano 2011-04-09 4:38 ` Nguyen Thai Ngoc Duy ` (2 more replies) 0 siblings, 3 replies; 35+ messages in thread From: Junio C Hamano @ 2011-04-08 23:18 UTC (permalink / raw) To: Jeff King; +Cc: git, Nguyen Thai Ngoc Duy, Michael J Gruber Junio C Hamano <gitster@pobox.com> writes: > Jeff King <peff@peff.net> writes: > >> ... I thought there was interest in full-tree grep >> (OK, _I_ had some interst in it). But the same transition pain >> arguments apply there, and we should be able to do "git grep pattern :/" >> soon, right? > > I never tested it myself, but the earlier "support :/ at a wrong level > get_pathspec()" patch should take care of "git grep" as well. It is > equivalent to the "alternative approach" Michael posted as RFC as a > follow-up to his earlier "grep --full-tree" thread. It appears that we might want to further tweak the code that tries to disambiguate between revs and paths (we error out when argv[i] does not name a rev and lstat(argv[i]) fails), but other than that, this command sequence $ cd Documentation $ git grep -e purple -- : seems to hit ../contrib/emacs/git.el and ../gitk-git/gitk correctly. Of course, from the same directory: $ git grep -e purple -- :/*.el hits ../contrib/emacs/git.el as expected. The following patch will apply on top of 8a42c98 (magic pathspec: add tentative ":/path/from/top/level" pathspec support, 2011-04-06). Per our discussion, I think 'add -u' migration topics should be scrapped for now, and rethought after giving time for people to get familiar with the new :/ facility. Thanks. -- >8 -- Subject: [PATCH] magic pathspec: futureproof shorthand form 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 <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- ctype.c | 15 ++++++++------- git-compat-util.h | 2 ++ setup.c | 9 ++++++++- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/ctype.c b/ctype.c index de60027..b5d856f 100644 --- a/ctype.c +++ b/ctype.c @@ -10,17 +10,18 @@ enum { A = GIT_ALPHA, D = GIT_DIGIT, G = GIT_GLOB_SPECIAL, /* *, ?, [, \\ */ - R = GIT_REGEX_SPECIAL /* $, (, ), +, ., ^, {, | */ + R = GIT_REGEX_SPECIAL, /* $, (, ), +, ., ^, {, | */ + P = GIT_PATHSPEC_MAGIC /* other non-alnum, except for ] and } */ }; unsigned char sane_ctype[256] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, S, S, 0, 0, S, 0, 0, /* 0.. 15 */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 16.. 31 */ - S, 0, 0, 0, R, 0, 0, 0, R, R, G, R, 0, 0, R, 0, /* 32.. 47 */ - D, D, D, D, D, D, D, D, D, D, 0, 0, 0, 0, 0, G, /* 48.. 63 */ - 0, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A, /* 64.. 79 */ - A, A, A, A, A, A, A, A, A, A, A, G, G, 0, R, 0, /* 80.. 95 */ - 0, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A, /* 96..111 */ - A, A, A, A, A, A, A, A, A, A, A, R, R, 0, 0, 0, /* 112..127 */ + S, P, P, P, R, P, P, P, R, R, G, R, P, P, R, P, /* 32.. 47 */ + D, D, D, D, D, D, D, D, D, D, P, P, P, P, P, G, /* 48.. 63 */ + P, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A, /* 64.. 79 */ + A, A, A, A, A, A, A, A, A, A, A, G, G, 0, R, P, /* 80.. 95 */ + P, A, A, A, A, A, A, A, A, A, A, A, A, A, A, A, /* 96..111 */ + A, A, A, A, A, A, A, A, A, A, A, R, R, 0, P, 0, /* 112..127 */ /* Nothing in the 128.. range */ }; diff --git a/git-compat-util.h b/git-compat-util.h index 49b50ee..d88cf8a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -462,6 +462,7 @@ extern unsigned char sane_ctype[256]; #define GIT_ALPHA 0x04 #define GIT_GLOB_SPECIAL 0x08 #define GIT_REGEX_SPECIAL 0x10 +#define GIT_PATHSPEC_MAGIC 0x20 #define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0) #define isascii(x) (((x) & ~0x7f) == 0) #define isspace(x) sane_istest(x,GIT_SPACE) @@ -472,6 +473,7 @@ extern unsigned char sane_ctype[256]; #define is_regex_special(x) sane_istest(x,GIT_GLOB_SPECIAL | GIT_REGEX_SPECIAL) #define tolower(x) sane_case((unsigned char)(x), 0x20) #define toupper(x) sane_case((unsigned char)(x), 0) +#define is_pathspec_magic(x) sane_istest(x,GIT_PATHSPEC_MAGIC) static inline int sane_case(int x, int high) { diff --git a/setup.c b/setup.c index 820ed05..5048252 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++; -- 1.7.5.rc1 ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-08 23:18 ` Junio C Hamano @ 2011-04-09 4:38 ` Nguyen Thai Ngoc Duy 2011-04-09 4:56 ` Junio C Hamano 2011-04-09 4:58 ` Nguyen Thai Ngoc Duy 2011-05-03 7:52 ` Nguyen Thai Ngoc Duy 2 siblings, 1 reply; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-04-09 4:38 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git, Michael J Gruber On Fri, Apr 08, 2011 at 04:18:46PM -0700, Junio C Hamano wrote: > It appears that we might want to further tweak the code that tries to > disambiguate between revs and paths (we error out when argv[i] does not > name a rev and lstat(argv[i]) fails) Something like below? The additional goodness is, instead of writing git grep foo -- '*.a' I can now write a shorter version git grep foo :./*.a Perhaps we can use the first pathspec with magic as a mark of pathspec arguments, equivalent to "--" --8<-- diff --git a/setup.c b/setup.c index 03cd84f..a00a23f 100644 --- a/setup.c +++ b/setup.c @@ -73,6 +73,8 @@ int check_filename(const char *prefix, const char *arg) const char *name; struct stat st; + if (*arg == ':') /* pathspec magic */ + return 1; name = prefix ? prefix_filename(prefix, strlen(prefix), arg) : arg; if (!lstat(name, &st)) return 1; /* file exists */ --8<-- -- Duy ^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-09 4:38 ` Nguyen Thai Ngoc Duy @ 2011-04-09 4:56 ` Junio C Hamano 2011-04-09 5:05 ` Nguyen Thai Ngoc Duy 0 siblings, 1 reply; 35+ messages in thread From: Junio C Hamano @ 2011-04-09 4:56 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: Jeff King, git, Michael J Gruber Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > On Fri, Apr 08, 2011 at 04:18:46PM -0700, Junio C Hamano wrote: >> It appears that we might want to further tweak the code that tries to >> disambiguate between revs and paths (we error out when argv[i] does not >> name a rev and lstat(argv[i]) fails) > > Something like below? That's too loose for my taste. At that point in the codepath, don't we sometimes expect the argv[i] might name a blob in the index? By "we might want to further...", I meant "when we properly redesign the get_pathspec vs init_pathspec combination". There are places in the current code that call verify_filename() to make sure that argv[i] is a pathspec after making sure argv[i] cannot be an object name. Currently it just does lstat() on it to see if it names a path. Instead, when we refactor get/init-pathspec API, we could expose an interface to turn one element from argv[] into a "struct pathspec_item". Then we can try to feed argv[i] to that string-to-pathspec_item function, and consider that argv[i] _is_ a proper pathspec only if it parses correctly *and* if it matches either an item in the current working tree. That would be a moral equivalent of the current verify_filename() check but is far more precise one; e.g. the current code rejects git grep -e foo '*.c' ;# bad because '*.c' is not an object name, but lstat("*.c") fails, and you need to disambiguate with '--'. If you rewrite the verify_filename() in the way I outlined above, you wouldn't have to. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-09 4:56 ` Junio C Hamano @ 2011-04-09 5:05 ` Nguyen Thai Ngoc Duy 2011-04-09 21:34 ` Junio C Hamano 0 siblings, 1 reply; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-04-09 5:05 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git, Michael J Gruber On Sat, Apr 9, 2011 at 11:56 AM, Junio C Hamano <gitster@pobox.com> wrote: > Instead, when we refactor get/init-pathspec API, we could expose an > interface to turn one element from argv[] into a "struct pathspec_item". > Then we can try to feed argv[i] to that string-to-pathspec_item function, > and consider that argv[i] _is_ a proper pathspec only if it parses > correctly *and* if it matches either an item in the current working tree. > > That would be a moral equivalent of the current verify_filename() check > but is far more precise one; e.g. the current code rejects > > git grep -e foo '*.c' ;# bad > > because '*.c' is not an object name, but lstat("*.c") fails, and you need > to disambiguate with '--'. If you rewrite the verify_filename() in the > way I outlined above, you wouldn't have to. I considered that, but we may need to go through the whole worktree just to verify "*.c" matches something. The worst case scenario can be expensive (eg. doing that on a-forest-with-no-c-file gentoo-x86.git). Alternative approach is recognize "*.c" has wildcard and let it pass without actually matching. -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-09 5:05 ` Nguyen Thai Ngoc Duy @ 2011-04-09 21:34 ` Junio C Hamano 0 siblings, 0 replies; 35+ messages in thread From: Junio C Hamano @ 2011-04-09 21:34 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: Jeff King, git, Michael J Gruber Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > On Sat, Apr 9, 2011 at 11:56 AM, Junio C Hamano <gitster@pobox.com> wrote: > >> ... If you rewrite the verify_filename() in the >> way I outlined above, you wouldn't have to. > > I considered that, but we may need to go through the whole worktree > just to verify "*.c" matches something. You are absolutely right. After all, this is only meant to be a quick sanity check to ensure that we got the user's intention absolutely right when the user did not disambiguate with '--'. When we see all earlier ones are object names (and cannot be filenames) and all later ones have working tree entries (and cannot be object names), we are sure that we got the missing '--' boundary right. And we error out when there is any doubt, and that is a good feature. Even when there is no glob involved, e.g. "git log builtin-add.c", we fail the parsing, because there is no builtin-add.c in the filesystem in today's checkou. That behaviour is coming exactly from that reasoning: we may be sure that "builtin-add.c" cannot be an object name, but we don't know if it is a typo of builtin/add.c (or builtin-add.o, etc.), and we don't want to make the user wait while we are guessing. So let's forget about the suggestion. Thanks for injecting sanity to the discussion and stopping me from going overboard. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-08 23:18 ` Junio C Hamano 2011-04-09 4:38 ` Nguyen Thai Ngoc Duy @ 2011-04-09 4:58 ` Nguyen Thai Ngoc Duy 2011-04-09 5:20 ` Junio C Hamano 2011-04-09 11:24 ` Nguyen Thai Ngoc Duy 2011-05-03 7:52 ` Nguyen Thai Ngoc Duy 2 siblings, 2 replies; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-04-09 4:58 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git, Michael J Gruber On Sat, Apr 9, 2011 at 6:18 AM, Junio C Hamano <gitster@pobox.com> wrote: > 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 :". The intention is good, but reality may need more work. I assume that "git add -u :" is equivalent to "git add -u" (or "git add -u ." to be precise). Unfortunately, cmd_add() checks argc for no arguments to turn "add -u <nothing>" to "add -u .", not the result from get_pathspec(). It can be fixed. Just heads up as there can be similar traps elsewhere. -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-09 4:58 ` Nguyen Thai Ngoc Duy @ 2011-04-09 5:20 ` Junio C Hamano 2011-04-09 10:15 ` Nguyen Thai Ngoc Duy 2011-04-09 11:24 ` Nguyen Thai Ngoc Duy 1 sibling, 1 reply; 35+ messages in thread From: Junio C Hamano @ 2011-04-09 5:20 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: Jeff King, git, Michael J Gruber Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > The intention is good, but reality may need more work. Yes, I knew about "add -u", as I was touching in its neighbourhood recently. The way "git grep" does this may be more appropriate as a short term solution. The updated init_pathspec() should at least take (prefix, argc, argv[]), and in addition a hint as to what to do when there is no pathspec from the command line (i.e. argc == 0), so that it can behave differently when the user gave only ":". By the way, the field in "struct pathspec_item" would need to be updated, and the matcher would need to be changed, so that each item knows up to which part of the "match" string came from the prefix (and remainder is a user supplied pattern). Then from a subdirectory a?a/bbb, - "c" should parse into prefix "a?a/bbb/" plus pattern "c" - ":../c" should become prefix "a?a/" plus pattern "c" and the matcher should match the prefix part _literally_ without fnmatch(3), while using whatever magic (e.g. use_wildcard) to match the pattern part. I think we currently match the whole thing with fnmatch(3), which in practice may be OK only because not many people use glob characters in their directory names, but what the current matcher does logically is wrong. Of course, both of the above are tasks after 1.7.5 ships, but I thought I should mention them now, as you seem to be already thinking about the future. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-09 5:20 ` Junio C Hamano @ 2011-04-09 10:15 ` Nguyen Thai Ngoc Duy 0 siblings, 0 replies; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-04-09 10:15 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git, Michael J Gruber On Sat, Apr 9, 2011 at 12:20 PM, Junio C Hamano <gitster@pobox.com> wrote: > By the way, the field in "struct pathspec_item" would need to be updated, > and the matcher would need to be changed, so that each item knows up to > which part of the "match" string came from the prefix (and remainder is a > user supplied pattern). Then from a subdirectory a?a/bbb, > > - "c" should parse into prefix "a?a/bbb/" plus pattern "c" > > - ":../c" should become prefix "a?a/" plus pattern "c" > > and the matcher should match the prefix part _literally_ without > fnmatch(3), while using whatever magic (e.g. use_wildcard) to match the > pattern part. I think we currently match the whole thing with fnmatch(3), > which in practice may be OK only because not many people use glob > characters in their directory names, but what the current matcher does > logically is wrong. OK. Let's add nomagic_len (or plain_len) to pathspec_item for that. I was thinking of noglob_len but changed my mind because the same can also be applied for icase magic. We don't want to do strcasecmp on prefix. > Of course, both of the above are tasks after 1.7.5 ships, but I thought I > should mention them now, as you seem to be already thinking about the > future. -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-09 4:58 ` Nguyen Thai Ngoc Duy 2011-04-09 5:20 ` Junio C Hamano @ 2011-04-09 11:24 ` Nguyen Thai Ngoc Duy 2011-04-09 21:38 ` Junio C Hamano 1 sibling, 1 reply; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-04-09 11:24 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git, Michael J Gruber On Sat, Apr 9, 2011 at 11:58 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote: > On Sat, Apr 9, 2011 at 6:18 AM, Junio C Hamano <gitster@pobox.com> wrote: >> 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 :". > > The intention is good, but reality may need more work. Wait, what if I say "git grep -- : foo : bar"? I take it we should reject on this case? -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-09 11:24 ` Nguyen Thai Ngoc Duy @ 2011-04-09 21:38 ` Junio C Hamano 0 siblings, 0 replies; 35+ messages in thread From: Junio C Hamano @ 2011-04-09 21:38 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: Jeff King, git, Michael J Gruber Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > On Sat, Apr 9, 2011 at 11:58 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote: >> On Sat, Apr 9, 2011 at 6:18 AM, Junio C Hamano <gitster@pobox.com> wrote: >>> 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 :". >> >> The intention is good, but reality may need more work. > > Wait, what if I say "git grep -- : foo : bar"? I take it we should > reject on this case? The patch probably would stuff NULL, "foo", NULL, "bar" in the array, and the first NULL would make the caller turn the array itself into NULL; it is a user error I didn't have to bother while the topic is still in a PoC stage. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-04-08 23:18 ` Junio C Hamano 2011-04-09 4:38 ` Nguyen Thai Ngoc Duy 2011-04-09 4:58 ` Nguyen Thai Ngoc Duy @ 2011-05-03 7:52 ` Nguyen Thai Ngoc Duy 2011-05-03 15:01 ` Junio C Hamano 2 siblings, 1 reply; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-05-03 7:52 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git, Michael J Gruber On Sat, Apr 9, 2011 at 6:18 AM, Junio C Hamano <gitster@pobox.com> wrote: > Subject: [PATCH] magic pathspec: futureproof shorthand form > > ... > > 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 :". I need someone to enlighten me again. Why do we need ":" for "no pathspec" when we can simply specify no pathspec for the same effect? "git log" and "git log ." at top worktree are not different because any changes in the tree will make top tree object different, hence no pruning (unless someone commits the same tree, which is really rare). So - "git log" in subdirectory is exactly the same as "git log" at top. - "git log :/" in subdir can do whatever "git log ." at top can. - "git log ." in subdir will prune commits that does not change subdir (current behavior) I don't (or no longer) see the point of reserving ":" for "no pathspec". -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-05-03 7:52 ` Nguyen Thai Ngoc Duy @ 2011-05-03 15:01 ` Junio C Hamano 2011-05-03 16:17 ` Nguyen Thai Ngoc Duy 0 siblings, 1 reply; 35+ messages in thread From: Junio C Hamano @ 2011-05-03 15:01 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: Jeff King, git, Michael J Gruber Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > I need someone to enlighten me again. Why do we need ":" for "no > pathspec" when we can simply specify no pathspec for the same effect? Futureproofing. Currently "log" family defaults to "tree-wide" when there is no pathspec, but imagine what happens when a command like "log" that knows how to simplify history defaults to "current directory". You cannot override it by "git that-cmd :/", which would give it a single tree-wide pathspec, not "no pathspec". It will still cull empty commits. ^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 2/4] add -u: get rid of "treewideupdate" configuration 2011-05-03 15:01 ` Junio C Hamano @ 2011-05-03 16:17 ` Nguyen Thai Ngoc Duy 0 siblings, 0 replies; 35+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-05-03 16:17 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git, Michael J Gruber On Tue, May 3, 2011 at 10:01 PM, Junio C Hamano <gitster@pobox.com> wrote: > Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes: > >> I need someone to enlighten me again. Why do we need ":" for "no >> pathspec" when we can simply specify no pathspec for the same effect? > > Futureproofing. Currently "log" family defaults to "tree-wide" when there > is no pathspec, but imagine what happens when a command like "log" that > knows how to simplify history defaults to "current directory". You cannot > override it by "git that-cmd :/", which would give it a single tree-wide > pathspec, not "no pathspec". It will still cull empty commits. OK, empty commits. I see. Will test it that way. -- Duy ^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 3/4] add: make "add -u/-A" update full tree without pathspec (step 2) 2011-04-07 1:16 [PATCH 0/4] Redoing the "add -u" migration plan Junio C Hamano 2011-04-07 1:16 ` [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support Junio C Hamano 2011-04-07 1:16 ` [PATCH 2/4] add -u: get rid of "treewideupdate" configuration Junio C Hamano @ 2011-04-07 1:16 ` Junio C Hamano 2011-04-07 1:16 ` [PATCH 4/4] add: make "add -u/-A" update full tree without pathspec (step 3) Junio C Hamano 3 siblings, 0 replies; 35+ messages in thread From: Junio C Hamano @ 2011-04-07 1:16 UTC (permalink / raw) To: git Flip the default behaviour when "git add -u/-A" is run without a pathspec from a subdirectory to tree-wide, and reword the advice message. We will need to keep the advice message for a while to help people who skipped the 1.8.0 boundary. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/add.c | 8 ++++---- t/t2200-add-update.sh | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index f58d1cf..6e6cdc0 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -360,13 +360,13 @@ static int add_files(struct dir_struct *dir, int flags) } static const char *warn_add_uA_180_migration_msg[] = { - "In release 1.8.0, running 'git add -u' (or 'git add -A') from", - "a subdirectory without giving any pathspec WILL take effect", + "Since release 1.8.0, running 'git add -u' (or 'git add -A')", + "from a subdirectory without giving any pathspec takes effect", "on the whole working tree, not just the part under the current", "directory. Please make it a habit to add '.' when you want to", "limit the operation to the current directory and below.", "You can use ':/' at the end of the command to affect the operation", - "on the whole working tree.", + "on the whole working tree, if you want to be explicit.", }; static int warn_180_migration(void) @@ -374,7 +374,7 @@ static int warn_180_migration(void) int i; for (i = 0; i < ARRAY_SIZE(warn_add_uA_180_migration_msg); i++) warning("%s", warn_add_uA_180_migration_msg[i]); - return 0; /* default to "no" (not tree-wide, i.e. local) */ + return 1; /* default to "true" (tree-wide, i.e. not local) */ } int cmd_add(int argc, const char **argv, const char *prefix) diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index f7711ba..a601394 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -91,7 +91,7 @@ test_expect_success 'update from a subdirectory without pathspec' ' git add -u 2>../expect.warning ) && git diff-files --name-only dir1 check >actual && - echo check >expect && + : >expect && test_cmp expect actual && grep warning expect.warning ' -- 1.7.5.rc1 ^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 4/4] add: make "add -u/-A" update full tree without pathspec (step 3) 2011-04-07 1:16 [PATCH 0/4] Redoing the "add -u" migration plan Junio C Hamano ` (2 preceding siblings ...) 2011-04-07 1:16 ` [PATCH 3/4] add: make "add -u/-A" update full tree without pathspec (step 2) Junio C Hamano @ 2011-04-07 1:16 ` Junio C Hamano 3 siblings, 0 replies; 35+ messages in thread From: Junio C Hamano @ 2011-04-07 1:16 UTC (permalink / raw) To: git Now long after 1.8.0 happened, people should have got used to the new default behaviour and it is no longer necessary to give the migration advice anymore. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/add.c | 23 +---------------------- t/t2200-add-update.sh | 4 +--- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 6e6cdc0..3564d7e 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -359,24 +359,6 @@ static int add_files(struct dir_struct *dir, int flags) return exit_status; } -static const char *warn_add_uA_180_migration_msg[] = { - "Since release 1.8.0, running 'git add -u' (or 'git add -A')", - "from a subdirectory without giving any pathspec takes effect", - "on the whole working tree, not just the part under the current", - "directory. Please make it a habit to add '.' when you want to", - "limit the operation to the current directory and below.", - "You can use ':/' at the end of the command to affect the operation", - "on the whole working tree, if you want to be explicit.", -}; - -static int warn_180_migration(void) -{ - int i; - for (i = 0; i < ARRAY_SIZE(warn_add_uA_180_migration_msg); i++) - warning("%s", warn_add_uA_180_migration_msg[i]); - return 1; /* default to "true" (tree-wide, i.e. not local) */ -} - int cmd_add(int argc, const char **argv, const char *prefix) { int exit_status = 0; @@ -407,11 +389,8 @@ int cmd_add(int argc, const char **argv, const char *prefix) die("-A and -u are mutually incompatible"); if (!show_only && ignore_missing) die("Option --ignore-missing can only be used together with --dry-run"); - if ((addremove || take_worktree_changes) && !argc) { + if ((addremove || take_worktree_changes) && !argc) whole_tree_add = 1; - if (prefix) - whole_tree_add = warn_180_migration(); - } add_new_files = !take_worktree_changes && !refresh_only; require_pathspec = !take_worktree_changes; diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index a601394..3ad6bff 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -81,8 +81,6 @@ test_expect_success 'change gets noticed' ' ' test_expect_success 'update from a subdirectory without pathspec' ' - # This test needs to be updated to expect the whole tree - # update after 1.8.0 migration. test_might_fail git reset check dir1 && echo changed >check && ( @@ -93,7 +91,7 @@ test_expect_success 'update from a subdirectory without pathspec' ' git diff-files --name-only dir1 check >actual && : >expect && test_cmp expect actual && - grep warning expect.warning + ! grep warning expect.warning ' test_expect_success 'update from a subdirectory with local pathspec' ' -- 1.7.5.rc1 ^ permalink raw reply related [flat|nested] 35+ messages in thread
end of thread, other threads:[~2011-05-03 16:17 UTC | newest] Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-04-07 1:16 [PATCH 0/4] Redoing the "add -u" migration plan Junio C Hamano 2011-04-07 1:16 ` [PATCH 1/4] magic pathspec: add tentative ":/path/from/top/level" pathspec support Junio C Hamano 2011-04-07 1:40 ` Junio C Hamano 2011-04-07 13:09 ` Nguyen Thai Ngoc Duy 2011-04-07 18:28 ` Junio C Hamano 2011-04-08 11:39 ` Nguyen Thai Ngoc Duy 2011-04-07 13:23 ` Nguyen Thai Ngoc Duy 2011-04-07 16:18 ` Junio C Hamano 2011-04-08 12:00 ` Nguyen Thai Ngoc Duy 2011-04-08 15:05 ` Junio C Hamano 2011-04-08 15:39 ` Nguyen Thai Ngoc Duy 2011-04-08 16:37 ` Junio C Hamano 2011-04-08 17:02 ` Nguyen Thai Ngoc Duy 2011-04-07 1:16 ` [PATCH 2/4] add -u: get rid of "treewideupdate" configuration Junio C Hamano 2011-04-08 17:54 ` Jeff King 2011-04-08 19:27 ` Junio C Hamano 2011-04-08 20:24 ` Jeff King 2011-04-08 22:22 ` Junio C Hamano 2011-04-08 22:32 ` Jeff King 2011-04-08 22:37 ` Junio C Hamano 2011-04-08 23:18 ` Junio C Hamano 2011-04-09 4:38 ` Nguyen Thai Ngoc Duy 2011-04-09 4:56 ` Junio C Hamano 2011-04-09 5:05 ` Nguyen Thai Ngoc Duy 2011-04-09 21:34 ` Junio C Hamano 2011-04-09 4:58 ` Nguyen Thai Ngoc Duy 2011-04-09 5:20 ` Junio C Hamano 2011-04-09 10:15 ` Nguyen Thai Ngoc Duy 2011-04-09 11:24 ` Nguyen Thai Ngoc Duy 2011-04-09 21:38 ` Junio C Hamano 2011-05-03 7:52 ` Nguyen Thai Ngoc Duy 2011-05-03 15:01 ` Junio C Hamano 2011-05-03 16:17 ` Nguyen Thai Ngoc Duy 2011-04-07 1:16 ` [PATCH 3/4] add: make "add -u/-A" update full tree without pathspec (step 2) Junio C Hamano 2011-04-07 1:16 ` [PATCH 4/4] add: make "add -u/-A" update full tree without pathspec (step 3) Junio C Hamano
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.