[PATCH 0/3] Make "git add dir/" notice removed "dir/file"

[PATCH 0/3] Make "git add dir/" notice removed "dir/file"

[Junio C Hamano]

This resurrects an ancient stalled topic from last year, rebased on
top of the recent "git add -u/-A" documentation updates 5cae93566027
(add: Clarify documentation of -A and -u, 2013-03-07) by Greg Price.

The first one is a pure clean-up.  The second one is a preparatory
step that can happen before Git 2.0 to prepare existing users to
form a habit of saying "--all" (or "--no-all") explicitly when
giving a directory name to "git add <pathspec>...".

Then the last one can be done in Git 2.0 to swap the default.

Junio C Hamano (3):
  builtin/add.c: simplify boolean variables
  git add: start preparing for "git add <pathspec>..." to default to "-A"
  git add <pathspec>... defaults to "-A"

 Documentation/git-add.txt | 22 +++++++++++++++++++---
 builtin/add.c             | 36 +++++++++++++++++++++++++-----------
 t/t2200-add-update.sh     |  6 +++---
 3 files changed, 47 insertions(+), 17 deletions(-)

-- 
1.8.2-rc3-203-gc9aaab5

[PATCH 1/3] builtin/add.c: simplify boolean variables

[Junio C Hamano]

Do not to explicitly initialize static variables to 0 and instead
let BSS take care of it.  Also use OPT_BOOL() to let the command
line arguments set these variables to 0 or 1, instead of the
deprecated OPT_BOOLEAN() aka OPT_COUNTUP().

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/add.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 0dd014e..220321b 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -270,23 +270,23 @@ static struct lock_file lock_file;
 static const char ignore_error[] =
 N_("The following paths are ignored by one of your .gitignore files:\n");
 
-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 verbose, show_only, ignored_too, refresh_only;
+static int ignore_add_errors, addremove, intent_to_add, ignore_missing;
 
 static struct option builtin_add_options[] = {
 	OPT__DRY_RUN(&show_only, N_("dry run")),
 	OPT__VERBOSE(&verbose, N_("be verbose")),
 	OPT_GROUP(""),
-	OPT_BOOLEAN('i', "interactive", &add_interactive, N_("interactive picking")),
-	OPT_BOOLEAN('p', "patch", &patch_interactive, N_("select hunks interactively")),
-	OPT_BOOLEAN('e', "edit", &edit_interactive, N_("edit current diff and apply")),
+	OPT_BOOL('i', "interactive", &add_interactive, N_("interactive picking")),
+	OPT_BOOL('p', "patch", &patch_interactive, N_("select hunks interactively")),
+	OPT_BOOL('e', "edit", &edit_interactive, N_("edit current diff and apply")),
 	OPT__FORCE(&ignored_too, N_("allow adding otherwise ignored files")),
-	OPT_BOOLEAN('u', "update", &take_worktree_changes, N_("update tracked files")),
-	OPT_BOOLEAN('N', "intent-to-add", &intent_to_add, N_("record only the fact that the path will be added later")),
-	OPT_BOOLEAN('A', "all", &addremove, N_("add changes from all tracked and untracked files")),
-	OPT_BOOLEAN( 0 , "refresh", &refresh_only, N_("don't add, only refresh the index")),
-	OPT_BOOLEAN( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")),
-	OPT_BOOLEAN( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")),
+	OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")),
+	OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the fact that the path will be added later")),
+	OPT_BOOL('A', "all", &addremove, N_("add changes from all tracked and untracked files")),
+	OPT_BOOL( 0 , "refresh", &refresh_only, N_("don't add, only refresh the index")),
+	OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")),
+	OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")),
 	OPT_END(),
 };
 
-- 
1.8.2-rc3-203-gc9aaab5

[PATCH 2/3] git add: start preparing for "git add <pathspec>..." to default to "-A"

[Junio C Hamano]

When "git add subdir/" is run without "-u" or "-A" option, e.g.

    $ edit subdir/x
    $ create subdir/y
    $ rm subdir/z
    $ git add subdir/

the command does not notice removal of paths (e.g. subdir/z) from
the working tree.  This sometimes confuses new people, as arguably
"git add" is told to record the current state of "subdir/" as a
whole, not the current state of the paths that exist in the working
tree that matches that pathspec (the latter by definition excludes
the state of "subdir/z" because it does not exist in the working
tree).

Plan to eventually make "git add" pretend as if "-A" is given when
there is a pathspec on the command line.  When resolving a conflict
to remove a path, the current code tells you to "git rm $path", but
with such a change, you will be able to say "git add $path" (of
course you can do "git add -A $path" today).  That means that we can
simplify the advice messages given by "git status".  That all will
be in Git 2.0 or later, if we are going to do so.

For that transition to work, people need to learn either to say "git
add --no-all subdir/" when they want to ignore the removed paths
like "subdir/z", or to say "git add -A subdir/" when they want to
take the state of the directory as a whole.

"git add" without any argument will continue to be a no-op.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-add.txt | 14 +++++++++++++-
 builtin/add.c             | 45 +++++++++++++++++++++++++++++++++++++++++++--
 t/t2200-add-update.sh     |  6 +++---
 3 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index b0944e5..5c501a2 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
-	  [--edit | -e] [--all | [--update | -u]] [--intent-to-add | -N]
+	  [--edit | -e] [--[no-]all | [--update | -u]] [--intent-to-add | -N]
 	  [--refresh] [--ignore-errors] [--ignore-missing] [--]
 	  [<pathspec>...]
 
@@ -121,6 +121,18 @@ If no <pathspec> is given, the current version of Git defaults to
 and its subdirectories. This default will change in a future version
 of Git, hence the form without <pathspec> should not be used.
 
+--no-all::
+	Update the index by adding new files that are unknown to the
+	index and files modified in the working tree, but ignore
+	files that have been removed from the working tree.  This
+	option is a no-op when no <pathspec> is used.
++
+This option is primarily to help the current users of Git, whose
+"git add <pathspec>..." ignores removed files.  In future versions
+of Git, "git add <pathspec>..." will be a synonym to "git add -A
+<pathspec>..." and "git add --no-all <pathspec>..." will behave like
+today's "git add <pathspec>...", ignoring removed files.
+
 -N::
 --intent-to-add::
 	Record only the fact that the path will be added later. An entry
diff --git a/builtin/add.c b/builtin/add.c
index 220321b..f8f6c9e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -271,7 +271,11 @@ static const char ignore_error[] =
 N_("The following paths are ignored by one of your .gitignore files:\n");
 
 static int verbose, show_only, ignored_too, refresh_only;
-static int ignore_add_errors, addremove, intent_to_add, ignore_missing;
+static int ignore_add_errors, intent_to_add, ignore_missing;
+
+#define ADDREMOVE_DEFAULT 0 /* Change to 1 in Git 2.0 */
+static int addremove = ADDREMOVE_DEFAULT;
+static int addremove_explicit = -1; /* unspecified */
 
 static struct option builtin_add_options[] = {
 	OPT__DRY_RUN(&show_only, N_("dry run")),
@@ -283,7 +287,7 @@ static struct option builtin_add_options[] = {
 	OPT__FORCE(&ignored_too, N_("allow adding otherwise ignored files")),
 	OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")),
 	OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the fact that the path will be added later")),
-	OPT_BOOL('A', "all", &addremove, N_("add changes from all tracked and untracked files")),
+	OPT_BOOL('A', "all", &addremove_explicit, N_("add changes from all tracked and untracked files")),
 	OPT_BOOL( 0 , "refresh", &refresh_only, N_("don't add, only refresh the index")),
 	OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files which cannot be added because of errors")),
 	OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even missing - files are ignored in dry run")),
@@ -350,6 +354,18 @@ static void warn_pathless_add(const char *option_name, const char *short_name) {
 		option_name, short_name);
 }
 
+static int directory_given(int argc, const char **argv)
+{
+	struct stat st;
+
+	while (argc--) {
+		if (!lstat(*argv, &st) && S_ISDIR(st.st_mode))
+			return 1;
+		argv++;
+	}
+	return 0;
+}
+
 int cmd_add(int argc, const char **argv, const char *prefix)
 {
 	int exit_status = 0;
@@ -377,8 +393,33 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	argc--;
 	argv++;
 
+	if (0 <= addremove_explicit)
+		addremove = addremove_explicit;
+	else if (take_worktree_changes && ADDREMOVE_DEFAULT)
+		addremove = 0; /* "-u" was given but not "-A" */
+
 	if (addremove && take_worktree_changes)
 		die(_("-A and -u are mutually incompatible"));
+
+	/*
+	 * Warn when "git add pathspec..." was given without "-u" or "-A"
+	 * and pathspec... contains a directory name.
+	 */
+	if (!take_worktree_changes && addremove_explicit < 0 &&
+	    directory_given(argc, argv))
+		warning(_("In Git 2.0, 'git add <pathspec>...' will also update the\n"
+			  "index for paths removed from the working tree that match\n"
+			  "the given pathspec. If you want to 'add' only changed\n"
+			  "or newly created paths, say 'git add --no-all <pathspec>...'"
+			  " instead."));
+
+	if (!take_worktree_changes && addremove_explicit < 0 && argc)
+		/*
+		 * Turn "git add pathspec..." to "git add -A pathspec..."
+		 * in Git 2.0 but not yet
+		 */
+		; /* addremove = 1; */
+
 	if (!show_only && ignore_missing)
 		die(_("Option --ignore-missing can only be used together with --dry-run"));
 	if (addremove) {
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index 4cdebda..32f932a 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -150,9 +150,9 @@ test_expect_success 'add -u resolves unmerged paths' '
 	echo 2 >path3 &&
 	echo 2 >path5 &&
 
-	# Explicit resolving by adding removed paths should fail
-	test_must_fail git add path4 &&
-	test_must_fail git add path6 &&
+	# Fail to explicitly resolve removed paths with "git add"
+	test_must_fail git add --no-all path4 &&
+	test_must_fail git add --no-all path6 &&
 
 	# "add -u" should notice removals no matter what stages
 	# the index entries are in.
-- 
1.8.2-rc3-203-gc9aaab5

[PATCH 3/3] git add <pathspec>... defaults to "-A"

[Junio C Hamano]

Make "git add <pathspec>..." notice paths that have been removed
from the working tree, i.e. a synonym to "git add -A <pathspec>...".

Given that "git add <pathspec>" is to update the index with the
state of the named part of the working tree as a whole, it makes it
more intuitive, and also makes it possible to simplify the advice we
give while marking the paths the user finished resolving conflicts
with.  We used to say "to record removal as a resolution, remove the
path from the working tree and say 'git rm'; for all other cases,
edit the path in the working tree and say 'git add'", but we can now
say "update the path in the working tree and say 'git add'" instead.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-add.txt | 18 +++++++++++-------
 builtin/add.c             | 33 +++------------------------------
 2 files changed, 14 insertions(+), 37 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 5c501a2..e96fa84 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -53,8 +53,14 @@ OPTIONS
 	Files to add content from.  Fileglobs (e.g. `*.c`) can
 	be given to add all matching files.  Also a
 	leading directory name (e.g. `dir` to add `dir/file1`
-	and `dir/file2`) can be given to add all files in the
-	directory, recursively.
+	and `dir/file2`) can be given to update the index to
+	match the current state of the directory as a whole (e.g.
+	specifying `dir` will record not just a file `dir/file1`
+	modified in the working tree, a file `dir/file2` added to
+	the working tree, but also a file `dir/file3` removed from
+	the working tree.  Note that older versions of 	Git used
+	to ignore removed files; use `--no-all` option if you want
+	to add modified or new files but ignore removed	ones.
 
 -n::
 --dry-run::
@@ -127,11 +133,9 @@ of Git, hence the form without <pathspec> should not be used.
 	files that have been removed from the working tree.  This
 	option is a no-op when no <pathspec> is used.
 +
-This option is primarily to help the current users of Git, whose
-"git add <pathspec>..." ignores removed files.  In future versions
-of Git, "git add <pathspec>..." will be a synonym to "git add -A
-<pathspec>..." and "git add --no-all <pathspec>..." will behave like
-today's "git add <pathspec>...", ignoring removed files.
+This option is primarily to help users who are used to older
+versions of Git, whose "git add <pathspec>..." was a synonym
+to "git add --no-all <pathspec...", i.e. ignored removed files.
 
 -N::
 --intent-to-add::
diff --git a/builtin/add.c b/builtin/add.c
index f8f6c9e..21c685f 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -273,7 +273,7 @@ N_("The following paths are ignored by one of your .gitignore files:\n");
 static int verbose, show_only, ignored_too, refresh_only;
 static int ignore_add_errors, intent_to_add, ignore_missing;
 
-#define ADDREMOVE_DEFAULT 0 /* Change to 1 in Git 2.0 */
+#define ADDREMOVE_DEFAULT 1
 static int addremove = ADDREMOVE_DEFAULT;
 static int addremove_explicit = -1; /* unspecified */
 
@@ -354,18 +354,6 @@ static void warn_pathless_add(const char *option_name, const char *short_name) {
 		option_name, short_name);
 }
 
-static int directory_given(int argc, const char **argv)
-{
-	struct stat st;
-
-	while (argc--) {
-		if (!lstat(*argv, &st) && S_ISDIR(st.st_mode))
-			return 1;
-		argv++;
-	}
-	return 0;
-}
-
 int cmd_add(int argc, const char **argv, const char *prefix)
 {
 	int exit_status = 0;
@@ -401,24 +389,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	if (addremove && take_worktree_changes)
 		die(_("-A and -u are mutually incompatible"));
 
-	/*
-	 * Warn when "git add pathspec..." was given without "-u" or "-A"
-	 * and pathspec... contains a directory name.
-	 */
-	if (!take_worktree_changes && addremove_explicit < 0 &&
-	    directory_given(argc, argv))
-		warning(_("In Git 2.0, 'git add <pathspec>...' will also update the\n"
-			  "index for paths removed from the working tree that match\n"
-			  "the given pathspec. If you want to 'add' only changed\n"
-			  "or newly created paths, say 'git add --no-all <pathspec>...'"
-			  " instead."));
-
 	if (!take_worktree_changes && addremove_explicit < 0 && argc)
-		/*
-		 * Turn "git add pathspec..." to "git add -A pathspec..."
-		 * in Git 2.0 but not yet
-		 */
-		; /* addremove = 1; */
+		/* Turn "git add pathspec..." to "git add -A pathspec..." */
+		addremove = 1;
 
 	if (!show_only && ignore_missing)
 		die(_("Option --ignore-missing can only be used together with --dry-run"));
-- 
1.8.2-rc3-203-gc9aaab5