Re: [RFC] git rm -u

Matthieu Moy [off-list ref] writes:

Implementing "git rm -u" as a tree-wide command would create a
discrepancy with "git add -u". Implementing it as a "current directory"
command would make the migration harder if we eventually try to change
"git add -u". Perhaps "git rm -u" should be forbidden from a
subdirectory (with an error message pointing to "git rm -u :/" and "git
rm -u ."), waiting for a possible "git add -u" change.

Yeah, that sounds sensible.  Start with a "'git rm -u' is forbidden
without arguments", give advise to use either "." or ":/".  And stop
there.

The first step of "git add -u" migration plan would be to warn when
no argument is given and update all the existing index entries, and
give the same advise to use either "." or ":/".  Keep this for three
cycles: 3 * (8 to 10 weeks per cycle) = 27 weeks ~ 1/2 year.

The second step would be to forbid "git add -u", and keep the
advise.  That will make it in-line with "git rm -u".

On 01/20/2013 01:53 PM, Junio C Hamano wrote:
 > Matthieu Moy [off-list ref] writes:
 >
 >> Implementing "git rm -u" as a tree-wide command would create a
 >> discrepancy with "git add -u". Implementing it as a "current
 >> directory" command would make the migration harder if we eventually
 >> try to change "git add -u". Perhaps "git rm -u" should be forbidden
 >> from a subdirectory (with an error message pointing to "git rm -u
 >> :/" and "git rm -u ."), waiting for a possible "git add -u" change.
 >
 > Yeah, that sounds sensible.  Start with a "'git rm -u' is forbidden
 > without arguments", give advise to use either "." or ":/".  And stop
 > there.

I was unaware of any plan to change `git add -u`, but the above makes
sense to me.  I will use those suggestions as guidelines for the
initial implementation of `git rm -u`.  In particular, it will require
an argument like `.` or `:/`.  It sounds like the future direction of
`git add -u` will play a role in how `git rm -u` should behave so that
there is consistency between the two, so I will try to take a
conservative approach in my implementation.  Thank you both for the
advice and insight.

--
ejmr
南無妙法蓮華經

On Sun, Jan 20, 2013 at 7:53 PM, Junio C Hamano [off-list ref] wrote:

Matthieu Moy [off-list ref] writes:

quoted

Implementing "git rm -u" as a tree-wide command would create a
discrepancy with "git add -u". Implementing it as a "current directory"
command would make the migration harder if we eventually try to change
"git add -u". Perhaps "git rm -u" should be forbidden from a
subdirectory (with an error message pointing to "git rm -u :/" and "git
rm -u ."), waiting for a possible "git add -u" change.

Yeah, that sounds sensible.  Start with a "'git rm -u' is forbidden
without arguments", give advise to use either "." or ":/".  And stop
there.

The first step of "git add -u" migration plan would be to warn when
no argument is given and update all the existing index entries, and
give the same advise to use either "." or ":/".  Keep this for three
cycles: 3 * (8 to 10 weeks per cycle) = 27 weeks ~ 1/2 year.

The second step would be to forbid "git add -u", and keep the
advise.  That will make it in-line with "git rm -u".

Do you mean "git add" will be disallowed without "." or ":/" argument?
Or will this change in future and "git add" without argument will me
"whole tree", same as ":/" ?

--
Piotr Krukowiecki

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'.

The inconsistancy 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.

Signed-off-by: Matthieu Moy <redacted>
---
Junio C Hamano [off-list ref] writes:

The first step of "git add -u" migration plan would be to warn when
no argument is given and update all the existing index entries, and
give the same advise to use either "." or ":/".  Keep this for three
cycles: 3 * (8 to 10 weeks per cycle) = 27 weeks ~ 1/2 year.

The first step should look like this patch. The message remains vague
about the next steps ("change in a future Git version", no mention of
the exact change nor of the exact version in which it will happen),
but I'm fine with refining it (perhaps this could be a 2.0 change,
like the change to push.default?).

 Documentation/git-add.txt |  7 ++++---
 builtin/add.c             | 30 +++++++++++++++++++++++++++++-
 2 files changed, 33 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 e664100..e6eb829 100644
--- a/builtin/add.c
+++ b/builtin/add.c

@@ -373,6 +373,7 @@ 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;
 
 	git_config(add_config, NULL);
 

@@ -392,7 +393,34 @@ 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)
+		option_with_implicit_dot = "--all";
+	if (take_worktree_changes)
+		option_with_implicit_dot = "--update";
+	if (option_with_implicit_dot && !argc) {
+		/*
+		 * To be consistant 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' with no path argument will change in a future\n"
+			  "Git version and shouldn't be used anymore.  To add content for the whole tree, run:\n"
+			  "\n"
+			  "  git add %s :/\n"
+			  "\n"
+			  "To restrict the command to the current directory, run:\n"
+			  "\n"
+			  "  git add %s .\n"
+			  "\n"
+			  "With the current Git version, the command is restricted to the current directory."),
+			option_with_implicit_dot,
+			option_with_implicit_dot,
+			option_with_implicit_dot);
 		static const char *here[2] = { ".", NULL };
 		argc = 1;
 		argv = here;

-- 
1.8.0.319.g8abfee4

----- Ursprungligt meddelande -----

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'.

The inconsistancy 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)

git add -u without filepattern is, I believe very common, so no noisy
output there please.

git diff
#looks good
git add -u

-- robin

Hi,

Matthieu Moy wrote:

The inconsistancy 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:

Thanks for tackling this.

[...]

+	if (option_with_implicit_dot && !argc) {
+		/*
+		 * To be consistant 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' with no path argument will change in a future\n"

Would it be possible to make this conditional on cwd not being at the
toplevel (the case where "git add -u :/" and "git add -u ." have
different behavior)?  E.g.,

		static const char *here[2] = { ".", NULL };
		if (prefix)
			warning(...);

Thanks,
Jonathan

On Mon, Jan 21, 2013 at 7:00 PM, Matthieu Moy [off-list ref] wrote:

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'.

The inconsistancy 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.

What about 'grep' and 'clean'? I think at least 'clean' should go
tree-wide default too. I don't mind grep go the same way either but I
think people voiced preference in current behavior..
-- 
Duy