# [PATCH 2/3] dir.c: Omit non-excluded directories with dir->show_ignored 8 messages from 2007-05-06 to 2007-05-07. Participants: Michael Spang, Junio C Hamano, Linus Torvalds. Thread: https://gitlist.dev/t/8002 ## Michael Spang, 2007-05-06 18:09 Subject: [PATCH 2/3] dir.c: Omit non-excluded directories with dir->show_ignored Message-ID: <463E19D4.4030400@uwaterloo.ca> URL: https://gitlist.dev/e/463E19D4.4030400%40uwaterloo.ca In-Reply-To: <463E1705.2090201@gmail.com> ``` 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 --- 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 ``` ## Michael Spang, 2007-05-06 18:09 Subject: [PATCH 3/3] Fix minor documentation errors Message-ID: <463E19DE.8070305@uwaterloo.ca> URL: https://gitlist.dev/e/463E19DE.8070305%40uwaterloo.ca In-Reply-To: <463E1705.2090201@gmail.com> ``` - git-ls-files.txt: typo in description of --ignored - git-clean.txt: s/forceRequire/requireForce/ Signed-off-by: Michael Spang --- 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 ``` ## Michael Spang, 2007-05-06 18:09 Subject: [PATCH/RFD 4/3] t7300: Tests for git-clean using filenames with spaces/punctuation Message-ID: <463E19F6.9000906@uwaterloo.ca> URL: https://gitlist.dev/e/463E19F6.9000906%40uwaterloo.ca In-Reply-To: <463E1705.2090201@gmail.com> ``` Signed-off-by: Michael Spang --- 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 && ``` ## Junio C Hamano, 2007-05-06 18:54 Subject: Re: [PATCH/RFD 4/3] t7300: Tests for git-clean using filenames with spaces/punctuation Message-ID: <7vr6ptbx5g.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vr6ptbx5g.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <463E19F6.9000906@uwaterloo.ca> ``` Michael Spang 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. ``` ## Michael Spang, 2007-05-06 19:14 Subject: Re: [PATCH/RFD 4/3] t7300: Tests for git-clean using filenames with spaces/punctuation Message-ID: <463E28FC.9080209@uwaterloo.ca> URL: https://gitlist.dev/e/463E28FC.9080209%40uwaterloo.ca In-Reply-To: <7vr6ptbx5g.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano wrote: > Michael Spang 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 ``` ## Linus Torvalds, 2007-05-06 19:42 Subject: Re: [PATCH 2/3] dir.c: Omit non-excluded directories with dir->show_ignored Message-ID: URL: https://gitlist.dev/e/alpine.LFD.0.98.0705061239460.25245%40woody.linux-foundation.org In-Reply-To: <463E19D4.4030400@uwaterloo.ca> ``` 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 ``` ## Michael Spang, 2007-05-06 20:18 Subject: Re: [PATCH 2/3] dir.c: Omit non-excluded directories with dir->show_ignored Message-ID: <463E382F.8080708@uwaterloo.ca> URL: https://gitlist.dev/e/463E382F.8080708%40uwaterloo.ca In-Reply-To: ``` 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 ``` ## Michael Spang, 2007-05-07 02:35 Subject: Re: [PATCH 2/3] dir.c: Omit non-excluded directories with dir->show_ignored Message-ID: <463E9058.5080907@uwaterloo.ca> URL: https://gitlist.dev/e/463E9058.5080907%40uwaterloo.ca In-Reply-To: ``` 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 --- 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, ```