[PATCH alt-v1] Improve function dir.c:trim_trailing_spaces()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
---
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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]