Re: [PATCH 1/2] require pathspec for "git add -u/-A"
On Tue, Mar 12, 2013 at 02:58:44PM +0100, Matthieu Moy wrote: > > I guess we already rejected the idea of being able to shut off the > > warning and just get the new behavior, in favor of having people > > specify it manually each time? > > Somehow, but we may find a way to do so, as long as it temporary (i.e. > something that will have no effect after the transition period), and > that is is crystal clear that it's temporary. Yeah, I think this is easy as long as it is "enable the new behavior now" and not "toggle between new and old behavior". That is, a boolean rather than a selector, with a note that it will go away at the behavior switch. The only downside I see is that a user may switch it on now, saying "Yes, I understand and prefer the new behavior", but some script they run might not expect it. We can warn against that in the documentation, but that may or may not be enough. Here's a series which does that; if it's the direction we want to go, I think we'd want to rebase Junio's "now add -u is full-tree" patch on top. [1/2]: t2200: check that "add -u" limits itself to subdirectory [2/2]: add: respect add.updateroot config option -Peff -- 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 1/2] require pathspec for "git add -u/-A"
Jeff King writes: > PS I wonder how others are finding the warning? I'm finding it slightly >annoying. Perhaps I just haven't retrained my fingers yet. But one >problem with that retraining is that I type "git add -u" from the >root _most_ of the time, and it just works. But occasionally, I get >the "hey, do not do that" warning, because I'm in a subdir, even >though I would be happy to have the full-tree behavior. Same here. Not terribly disturbing, but I did get the warning occasionally, even though I'm the author of the patch introducing it ;-). > I guess we already rejected the idea of being able to shut off the > warning and just get the new behavior, in favor of having people > specify it manually each time? Somehow, but we may find a way to do so, as long as it temporary (i.e. something that will have no effect after the transition period), and that is is crystal clear that it's temporary. All proposals up to now were rejected because of the risk of confusing users (either shutting the warning blindly, or letting people think they could keep the current behavior after Git 2.0). -- 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 1/2] require pathspec for "git add -u/-A"
On Sun, Mar 10, 2013 at 04:49:09PM +0100, Matthieu Moy wrote: > Junio C Hamano writes: > > > As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without > > pathspec, 2013-01-28), "git add -u/-A" that is run without pathspec > > in a subdirectory will stop working sometime before Git 2.0, to wean > > users off of the old default, in preparation for adopting the new > > default in Git 2.0. > > I originally thought this step was necessary, but I changed my mind. The > warning is big enough and doesn't need to be turned into an error. > > If this step is applied, it should be applied at 2.0, not before, as > this is the big incompatible change. Re-introducing a new behavior won't > harm users OTOH. FWIW, I agree with this. The warning is enough without an error period (unless the intent was to switch to the error and stay there forever, but I do not think that is the proposal). -Peff PS I wonder how others are finding the warning? I'm finding it slightly annoying. Perhaps I just haven't retrained my fingers yet. But one problem with that retraining is that I type "git add -u" from the root _most_ of the time, and it just works. But occasionally, I get the "hey, do not do that" warning, because I'm in a subdir, even though I would be happy to have the full-tree behavior. I guess we already rejected the idea of being able to shut off the warning and just get the new behavior, in favor of having people specify it manually each time? -- 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 1/2] require pathspec for "git add -u/-A"
Junio C Hamano writes: > So let's squash these two steps into one and keep that in 'next' > until 2.0 ships. OK, then we may update the comment describing the plan (for people digging in the code to find out what the plan is). Small patch serie follows with this (will probably give conflict with your patch, feel free to drop if resolving them is too painful given the benefit) and another minor improvement. -- 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 1/2] require pathspec for "git add -u/-A"
Matthieu Moy writes: > Junio C Hamano writes: > >> As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without >> pathspec, 2013-01-28), "git add -u/-A" that is run without pathspec >> in a subdirectory will stop working sometime before Git 2.0, to wean >> users off of the old default, in preparation for adopting the new >> default in Git 2.0. > > I originally thought this step was necessary, but I changed my mind. The > warning is big enough and doesn't need to be turned into an error. I tend to agree. The plan requires the warning to be big enough and warning period to be long enough so that by the time Git 2.0 is released, no existing users will run "git add -u/-A" without pathspec expecting it to limit the operation to the current directory, so an extra step to error out such a command invocation is simply redundant. If it is not redundant, that would only mean that the warning period was not long enough. The only effect the extra "error it out" step would have is to hurt the people who jump on Git bandwagon after such release ships, as they do not have any reason to retrain their fingers---they instead can just get used to the new behaviour right away. So let's squash these two steps into one and keep that in 'next' until 2.0 ships. Thanks for an injection of sanity. -- 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 1/2] require pathspec for "git add -u/-A"
Junio C Hamano writes: > As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without > pathspec, 2013-01-28), "git add -u/-A" that is run without pathspec > in a subdirectory will stop working sometime before Git 2.0, to wean > users off of the old default, in preparation for adopting the new > default in Git 2.0. I originally thought this step was necessary, but I changed my mind. The warning is big enough and doesn't need to be turned into an error. If this step is applied, it should be applied at 2.0, not before, as this is the big incompatible change. Re-introducing a new behavior won't harm users OTOH. -- 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
[PATCH 1/2] require pathspec for "git add -u/-A"
As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without
pathspec, 2013-01-28), "git add -u/-A" that is run without pathspec
in a subdirectory will stop working sometime before Git 2.0, to wean
users off of the old default, in preparation for adopting the new
default in Git 2.0.
Signed-off-by: Junio C Hamano
---
Documentation/git-add.txt | 12
builtin/add.c | 38 ++
2 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 388a225..1ea1d39 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -107,10 +107,14 @@ apply to the index. See EDITING PATCHES below.
from the index if the corresponding files in the working tree
have been removed.
+
-If no 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 should not be used.
+If no is given when `-u` or `-A` option is used, Git used
+to update all tracked files in the current directory and its
+subdirectories. We would eventually want to change this to operate
+on the entire working tree, not limiting it to the current
+directory, to make it consistent with `git commit -a` and other
+commands. In order to avoid harming users who are used to the old
+default, Git *errors out* when no is given to train their
+fingers to explicitly type `git add -u .` when they mean it.
-A::
--all::
diff --git a/builtin/add.c b/builtin/add.c
index 0dd014e..4b9d57c 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -321,30 +321,28 @@ 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)
{
+static void die_on_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.
+* "git add -u" or "git add -A". In the previous release,
+* we kept the old behavior but gave a big warning.
*/
- 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."),
+ die(_("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 .)"),
option_name, short_name,
option_name, short_name,
option_name, short_name);
@@ -392,8 +390,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (option_with_implicit_dot && !argc) {
static const char *here[2] = { ".", NULL };
if (prefix)
- warn_pathless_add(option_with_implicit_dot,
- short_option_with_implicit_dot);
+ die_on_pathless_add(option_with_implicit_dot,
+ short_option_with_implicit_dot);
argc = 1;
argv = here;
}
--
1.8.2-rc3-196-gfcda97c
--
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
