dir.c: Omit non-excluded directories with dir->show_ignored

This makes "git-ls-files --others --directory --ignored" behave as documented and consequently also fixes "git-clean -d -X". Previously, git-clean would remove non-excluded directories even when using the -X option.

---
 dir.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

index d306352..adb3e62 100644
@@ -448,6 +448,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 
 		while ((de = readdir(fdir)) != NULL) {
 			int len;
+			int exclude;
 
 			if ((de->d_name[0] == '.') &&
 			    (de->d_name[1] == 0 ||

@@ -461,7 +462,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			memcpy(fullname + baselen, de->d_name, len+1);
 			if (simplify_away(fullname, baselen + len, simplify))
 				continue;
-			if (excluded(dir, fullname) != dir->show_ignored) {
+			if ((exclude = excluded(dir, fullname)) != dir->show_ignored) {
 				if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
 					continue;
 				}

@@ -484,6 +485,8 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 				len++;
 				switch (treat_directory(dir, fullname, baselen + len, simplify)) {
 				case show_directory:
+					if (exclude != dir->show_ignored)
+						continue;
 					break;
 				case recurse_into_directory:
 					contents += read_directory_recursive(dir,

1.5.2.rc1.4.g47e1

Okay. Will fixup after waiting a bit for more comments.

Thanks,
Michael Spang

This makes "git-ls-files --others --directory --ignored" behave as documented and consequently also fixes "git-clean -d -X". Previously, git-clean would remove non-excluded directories even when using the -X option.

---

This fixes the style issue noted by Linus.

 dir.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

index d306352..11fab7f 100644
@@ -448,6 +448,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 
 		while ((de = readdir(fdir)) != NULL) {
 			int len;
+			int exclude;
 
 			if ((de->d_name[0] == '.') &&
 			    (de->d_name[1] == 0 ||

@@ -461,7 +462,9 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			memcpy(fullname + baselen, de->d_name, len+1);
 			if (simplify_away(fullname, baselen + len, simplify))
 				continue;
-			if (excluded(dir, fullname) != dir->show_ignored) {
+
+			exclude = excluded(dir, fullname);
+			if (exclude != dir->show_ignored) {
 				if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
 					continue;
 				}

@@ -484,6 +487,8 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 				len++;
 				switch (treat_directory(dir, fullname, baselen + len, simplify)) {
 				case show_directory:
+					if (exclude != dir->show_ignored)
+						continue;
 					break;
 				case recurse_into_directory:
 					contents += read_directory_recursive(dir,

- git-ls-files.txt: typo in description of --ignored
- git-clean.txt: s/forceRequire/requireForce/

---
 Documentation/git-clean.txt    |    2 +-
 Documentation/git-ls-files.txt |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

index 5aff026..e3252d5 100644
@@ -26,7 +26,7 @@ OPTIONS
 	Remove untracked directories in addition to untracked files.
 
 -f::
-	If the git configuration specifies clean.forceRequire as true,
+	If the git configuration specifies clean.requireForce as true,
 	git-clean will refuse to run unless given -f or -n.
 
 -n::

index 79e0b7b..076cebc 100644
@@ -42,8 +42,8 @@ OPTIONS
 	Show other files in the output
 
 -i|--ignored::
-	Show ignored files in the output
-	Note the this also reverses any exclude list present.
+	Show ignored files in the output.
+	Note that this also reverses any exclude list present.
 
 -s|--stage::
 	Show stage files in the output

---

This isn't meant for applying, at least until this flaw is fixed.

These tests are failing because ls-files does escaping and clean does not unescape. Does anyone know of a solution to this?

sed could be used to remove the double quotes and replace escaped characters, but doing many replacements happens in multiple passes and hence does not always work as desired.

Is this even properly solvable without making clean a builtin or writing git-unescape?

 t/t7300-clean.sh |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

index 1fb3850..3792221 100755
@@ -7,6 +7,8 @@ test_description='git-clean basic tests'
 
 . ./test-lib.sh
 
+TAB="	"
+
 test_expect_success \
     'setup' \
     "mkdir -p src &&

@@ -123,6 +125,27 @@ test_expect_success \
     test ! -e obj.o &&
     test ! -e build"
 
+test_expect_success \
+    'filenames with spaces' \
+    'touch abc\ def &&
+    touch 123 123\  &&
+    git-add 123 &&
+    git-clean &&
+    test ! -e abc\ def &&
+    test ! -e 123\  &&
+    test -e 123'
+
+test_expect_success \
+    'filenames with escaped characters' \
+    'touch "'"$TAB"'" " " \\ \" \\\\\" \\t &&
+    git-clean &&
+    test ! -e "'"$TAB"'" &&
+    test ! -e " " &&
+    test ! -e \\ &&
+    test ! -e \" &&
+    test ! -e \\\\\" &&
+    test ! -e \\t'
+
 test_expect_failure \
     'clean.requireForce' \
     "mkdir -p build docs &&

If you know how to use "xargs -0" and are willing to depend on the -0 GNU extension, then the answer is yes.

I do not use git-clean myself, as I do not see what (I think) it tries to solve as a problem to begin with, so obviously I do not care too deeply about the command's implementation --- I just let it be there because there seem to be others who want it --- but if I were asked an advice on the right direction to proceed, I would probably suggest rewriting it in C.

Right. But I do not think xargs can call a shell function. git-clean does not just call git-rm directly. I guess git-clean could call itself through xargs, but that might become confusing.

That seems like the cleanest solution to me as well.

Cheers,
Michael Spang