[PATCH 2/3] dir.c: Omit non-excluded directories with dir->show_ignored

[PATCH 2/3] dir.c: Omit non-excluded directories with dir->show_ignored

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

Signed-off-by: Michael Spang <mspang@uwaterloo.ca>
---
 dir.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/dir.c b/dir.c
index d306352..adb3e62 100644
--- a/dir.c
+++ b/dir.c
@@ -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

Re: [PATCH 2/3] dir.c: Omit non-excluded directories with dir->show_ignored

[Linus Torvalds]

On Sun, 6 May 2007, Michael Spang wrote:
> @@ -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) {

Style issue: please write this as

			exclude = excluded(dir, fullname);
			if (exclude != dir->show_ignored) {

instead. 

Yes, both are valid C, and mean the same thing, but one is much more 
readable than the other.

Combining multiple things inside an if-statement is convenient if:

 - the things inside are _really_ trivial.

 - it's done as part of macro expansion etc (ie it's not visible as such, 
   and the code is readable in its pre-preprocessor format)

but it's not good form otherwise.

		Linus

Re: [PATCH 2/3] dir.c: Omit non-excluded directories with dir->show_ignored

[Michael Spang]

Linus Torvalds wrote:
> 
> On Sun, 6 May 2007, Michael Spang wrote:
>> @@ -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) {
> 
> Style issue: please write this as
> 
> 			exclude = excluded(dir, fullname);
> 			if (exclude != dir->show_ignored) {
> 
> instead. 

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

Thanks,
Michael Spang

Re: [PATCH 2/3] dir.c: Omit non-excluded directories with dir->show_ignored

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

Signed-off-by: Michael Spang <mspang@uwaterloo.ca>
---

This fixes the style issue noted by Linus.


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

diff --git a/dir.c b/dir.c
index d306352..11fab7f 100644
--- a/dir.c
+++ b/dir.c
@@ -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,

[PATCH 3/3] Fix minor documentation errors

[Michael Spang]

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

Signed-off-by: Michael Spang <mspang@uwaterloo.ca>
---
 Documentation/git-clean.txt    |    2 +-
 Documentation/git-ls-files.txt |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 5aff026..e3252d5 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -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::
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 79e0b7b..076cebc 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -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

[PATCH/RFD 4/3] t7300: Tests for git-clean using filenames with spaces/punctuation

[Michael Spang]

Signed-off-by: Michael Spang <mspang@uwaterloo.ca>
---

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(-)

diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index 1fb3850..3792221 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -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 &&

Re: [PATCH/RFD 4/3] t7300: Tests for git-clean using filenames with spaces/punctuation

[Junio C Hamano]

Michael Spang <mspang@uwaterloo.ca> writes:

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

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.

Re: [PATCH/RFD 4/3] t7300: Tests for git-clean using filenames with spaces/punctuation

[Michael Spang]

Junio C Hamano wrote:
> Michael Spang <mspang@uwaterloo.ca> writes:
> 
>> Is this even properly solvable without making clean a builtin or
>> writing git-unescape?
> 
> If you know how to use "xargs -0" and are willing to depend on
> the -0 GNU extension, then the answer is yes.

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.

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

That seems like the cleanest solution to me as well.

Cheers,
Michael Spang