Re: [PATCH v3] add: warn when -u or -A is used without filepattern
Junio C Hamano gits...@pobox.com writes: Junio C Hamano gits...@pobox.com writes: warning: The behavior of 'git add --update (or -u)' with no path argument from a subdirectory of the tree will change in Git 2.0 and should not be used anymore. There is a logic gap between will change and should not be used that is not filled like the text in the manual page does. I guess it is not so bad after all, if you read the entire message, not just the first two lines. Also, the warning is meant to be read by a user who just typed git add -u, so it is expected that the user knows what it does in current (or past) versions of Git. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] add: warn when -u or -A is used without filepattern
Junio C Hamano gits...@pobox.com writes: We should probably update the documentation/help for git add, but that is entirely a separate topic. The documentation update in 0fa2eb530fb7 (add: warn when -u or -A is used without pathspec, 2013-01-28) says: If no pathspec is given, the current version of Git defaults to .; in other words, update all tracked files in the current directory and its subdirectories. This default will change in a future version of Git, hence the form without filepattern should not be used. (oops, I just spotted a stray filepattern here, which came from a semantic mismerge---I'll fix it locally). The above text says that we currently add what you have in your current directory and below, before it says this default will change. That makes it easier to connect the default will change and form without pathspec should not be used in readers' mind. It does not take that much imagination and intelligence to infer it will change and will not limit to my current directory, so in the future I will have to be explicit when I want to do what I just told git to do. But the warning text does not sound quite right. This is what I get: warning: The behavior of 'git add --update (or -u)' with no path argument from a subdirectory of the tree will change in Git 2.0 and should not be used anymore. There is a logic gap between will change and should not be used that is not filled like the text in the manual page does. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] add: warn when -u or -A is used without filepattern
Junio C Hamano gits...@pobox.com writes: warning: The behavior of 'git add --update (or -u)' with no path argument from a subdirectory of the tree will change in Git 2.0 and should not be used anymore. There is a logic gap between will change and should not be used that is not filled like the text in the manual page does. I guess it is not so bad after all, if you read the entire message, not just the first two lines. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] add: warn when -u or -A is used without filepattern
Matthieu Moy wrote: Signed-off-by: Matthieu Moy matthieu@imag.fr Looks good to me. At some point we'll want to have tests for this case, but that's not particularly urgent until it's time for the warning() to turn into a die(). Thanks. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] add: warn when -u or -A is used without filepattern
Matthieu Moy venit, vidit, dixit 28.01.2013 10:16:
Most git commands that can be used with our without a filepattern are
tree-wide by default, the filepattern being used to restrict their scope.
A few exceptions are: 'git grep', 'git clean', 'git add -u' and 'git add -A'.
Since I didn't follow this thread, my first reaction was: Huh? Aren't
they treewide? (for the relative tree)
So, for someone reading just the commit message, it would be helpful to
say what the others do, i.e. default to the relative tree at pwd (rather
than defaulting to an empty tree, or all files whether tracked or not,
or...).
Otherwise, I'd rather switch sooner than later; it's so easy to take
git add -u git commit == git commit -a for granted and to miss
staging some hunks. But 2.0 is around the corner anyway, isn't it ;)
The inconsistency of 'git add -u' and 'git add -A' are particularly
problematic since other 'git add' subcommands (namely 'git add -p' and
'git add -e') are tree-wide by default.
Flipping the default now is unacceptable, so this patch starts training
users to type explicitely 'git add -u|-A :/' or 'git add -u|-A .', to prepare
for the next steps:
* forbid 'git add -u|-A' without filepattern (like 'git add' without
option)
* much later, maybe, re-allow 'git add -u|-A' without filepattern, with a
tree-wide scope.
A nice side effect of this patch is that it makes the :/ special
filepattern easier to discover for users.
When the command is called from the root of the tree, there is no
ambiguity and no need to change the behavior, hence no need to warn.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
Changes since v2:
* Typo consistant - consistent
* Mention both short and long option names (Thanks Junio). I went for
a two-lines display which I find a bit nicer to read than Junio's
version, but I'm fine with both.
Documentation/git-add.txt | 7 ---
builtin/add.c | 44 +++-
2 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index fd9e36b..5333559 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -107,9 +107,10 @@ apply to the index. See EDITING PATCHES below.
from the index if the corresponding files in the working tree
have been removed.
+
-If no filepattern is given, default to .; in other words,
-update all tracked files in the current directory and its
-subdirectories.
+If no filepattern is given, the current version of Git defaults to
+.; in other words, update all tracked files in the current directory
+and its subdirectories. This default will change in a future version
+of Git, hence the form without filepattern should not be used.
-A::
--all::
diff --git a/builtin/add.c b/builtin/add.c
index 7cb6cca..7738025 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -321,6 +321,35 @@ static int add_files(struct dir_struct *dir, int flags)
return exit_status;
}
+static void warn_pathless_add(const char *option_name, const char
*short_name) {
+ /*
+ * To be consistent with git add -p and most Git
+ * commands, we should default to being tree-wide, but
+ * this is not the original behavior and can't be
+ * changed until users trained themselves not to type
+ * git add -u or git add -A. For now, we warn and
+ * keep the old behavior. Later, this warning can be
+ * turned into a die(...), and eventually we may
+ * reallow the command with a new behavior.
+ */
+ warning(_(The behavior of 'git add %s (or %s)' with no path argument
from a\n
+ subdirectory of the tree will change in Git 2.0 and should
not be used anymore.\n
+ To add content for the whole tree, run:\n
+ \n
+ git add %s :/\n
+ (or git add %s :/)\n
+ \n
+ To restrict the command to the current directory, run:\n
+ \n
+ git add %s .\n
+ (or git add %s .)\n
+ \n
+ With the current Git version, the command is restricted to
the current directory.),
+ option_name, short_name,
+ option_name, short_name,
+ option_name, short_name);
+}
+
int cmd_add(int argc, const char **argv, const char *prefix)
{
int exit_status = 0;
@@ -331,6 +360,8 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
int add_new_files;
int require_pathspec;
char *seen = NULL;
+ const char *option_with_implicit_dot = NULL;
+ const char *short_option_with_implicit_dot = NULL;
git_config(add_config, NULL);
@@ -350,8 +381,19 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
die(_(-A and -u are mutually incompatible));
if (!show_only
Re: [PATCH v3] add: warn when -u or -A is used without filepattern
Michael J Gruber g...@drmicha.warpmail.net writes: Matthieu Moy venit, vidit, dixit 28.01.2013 10:16: Most git commands that can be used with our without a filepattern are tree-wide by default, the filepattern being used to restrict their scope. A few exceptions are: 'git grep', 'git clean', 'git add -u' and 'git add -A'. Since I didn't follow this thread, my first reaction was: Huh? Aren't they treewide? (for the relative tree) So, for someone reading just the commit message, it would be helpful to say what the others do, i.e. default to the relative tree at pwd (rather than defaulting to an empty tree, or all files whether tracked or not, or...). I think add -u commit vs commit -a you brought up is a good thing to mention, so let's do this. Another tweak is that I did s/filepattern/pathspec/ here. I know that both the documentation and the help text for git add say filepattern, but we say pathspec starting from glossary fairly consistently everywhere in the rest of the system. We should probably update the documentation/help for git add, but that is entirely a separate topic. add: warn when -u or -A is used without pathspec Most Git commands that can be used with or without pathspec operate tree-wide by default, the pathspec being used to restrict their scope. A few exceptions are: 'git grep', 'git clean', 'git add -u' and 'git add -A'. When run in a subdirectory without pathspec, they operate only on paths in the current directory. The inconsistency of 'git add -u' and 'git add -A' are particularly problematic since other 'git add' subcommands (namely 'git add -p' and 'git add -e') are tree-wide by default. It also means that git add -u git commit will record a state that is different from what is recorded with git commit -a. Flipping the default now is unacceptable, so let's start training users to type 'git add -u|-A :/' or 'git add -u|-A .' explicitly, to prepare for the next steps: * forbid 'git add -u|-A' without pathspec (like 'git add' without option) * much later, maybe, re-allow 'git add -u|-A' without pathspec, that will add all tracked and modified files, or all files, tree-wide. A nice side effect of this patch is that it makes the :/ magic pathspec easier to discover for users. When the command is called from the root of the tree, there is no ambiguity and no need to change the behavior, hence no need to warn. Signed-off-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Junio C Hamano gits...@pobox.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] add: warn when -u or -A is used without filepattern
Junio C Hamano gits...@pobox.com writes: I think add -u commit vs commit -a you brought up is a good thing to mention, so let's do this. I'm OK with your proposal. Let me know if you want me to resend. The inconsistency of 'git add -u' and 'git add -A' are particularly Nitpick: this should be inconsistencies (or is particularly). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] add: warn when -u or -A is used without filepattern
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: I think add -u commit vs commit -a you brought up is a good thing to mention, so let's do this. I'm OK with your proposal. Let me know if you want me to resend. Thanks for a quick response. As you may have guessed, I am sending these after running commit --amend, so ... The inconsistency of 'git add -u' and 'git add -A' are particularly Nitpick: this should be inconsistencies (or is particularly). ... it is much easier for me to fix these locally instead of getting a reroll. Will amend with s/are particularly/is particularly/; thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
