[PATCH alt-v1] Improve function dir.c:trim_trailing_spaces()
- From
- Pasha Bolokhov <pasha.bolokhov@gmail.com>
- Date
- May 30, 2014, 20:20 UTC
- Message-ID
- <1401481233-14500-1-git-send-email-pasha.bolokhov@gmail.com>
Discard unnecessary 'nr_spaces' variable and improve the 'if' structure. Switch to pointers instead of integers
Slightly more rare occurrences of 'text \ ' with a backslash in between spaces are handled correctly. Namely, the code in 8ba87adad6 does not reset 'last_space' when a backslash is encountered and the above line stays intact as a result. Add a test at the end of t/t0008-ignores.sh to exhibit this behavior
Signed-off-by: Pasha Bolokhov <pasha.bolokhov@gmail.com> --- Instead of the "optimized" version, which had complaints about reasonability, I am attaching the "correction" of the original version. 'last_space' was not reset after encountering a backslash, and 'nr_spaces' is an unnecessary variable.
I myself am still leaning towards the optimized version (which scans backwards instead of forward), since it completely ignores strings which do not have spaces at the end, while these forward implementations (both the one being attached and the original one) always scan all strings through. Again, I'm not attaching the optimized one because its readability is indeed less
dir.c | 29 ++++++++++++++--------------- t/t0008-ignores.sh | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/dir.c b/dir.c index eb6f581..e67dcc2 100644 --- a/dir.c +++ b/dir.c @@ -508,21 +508,20 @@ void clear_exclude_list(struct exclude_list *el) static void trim_trailing_spaces(char *buf) { - int i, last_space = -1, nr_spaces, len = strlen(buf); - for (i = 0; i < len; i++) - if (buf[i] == '\\') - i++; - else if (buf[i] == ' ') { - if (last_space == -1) { - last_space = i; - nr_spaces = 1; - } else - nr_spaces++; - } else - last_space = -1; - - if (last_space != -1 && last_space + nr_spaces == len) - buf[last_space] = '\0'; + char *p, *last_space = NULL; + + for (p = buf; *p; p++) + if (*p == ' ') { + if (last_space == NULL) + last_space = p; + } else { + if (*p == '\\') + p++; + last_space = NULL; + } + + if (last_space != NULL) + *last_space = '\0'; } int add_excludes_from_file_to_list(const char *fname, diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh index 63beb99..7becf96 100755 --- a/t/t0008-ignores.sh +++ b/t/t0008-ignores.sh @@ -806,4 +806,25 @@ test_expect_success !MINGW 'quoting allows trailing whitespace' ' test_cmp err.expect err ' +test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' ' + rm -rf whitespace && + mkdir whitespace && + >"whitespace/trailing 1 " && + >"whitespace/trailing 2 \\\\" && + >"whitespace/trailing 3 \\\\" && + >"whitespace/trailing 4 \\ " && + >"whitespace/trailing 5 \\ \\ " && + >whitespace/untracked && + echo "whitespace/trailing 1 \\ " >ignore && + echo "whitespace/trailing 2 \\\\\\\\\\\\\\\\" >>ignore && + echo "whitespace/trailing 3 \\\\\\\\\\\\\\\\ " >>ignore && + echo "whitespace/trailing 4 \\\\\\\\\\\\ " >>ignore && + echo "whitespace/trailing 5 \\\\\\\\ \\\\\\\\\\\\ " >>ignore && + echo whitespace/untracked >expect && + : >err.expect && + git ls-files -o -X ignore whitespace >actual 2>err && + test_cmp expect actual && + test_cmp err.expect err +' + test_done
-- 1.9.1