Subject: Re: [PATCH 1/4] config --show-origin: report paths with forward slashes

From: Johannes Sixt <[email-masked]>
Date: Sat, 2 Apr 2016 21:03:14 +0200

Am 29.03.2016 um 22:05 schrieb Junio C Hamano:
> Johannes Sixt <[email-masked]> writes:
> 
>> This part of your 45bf3297 (t1300: fix the new --show-origin tests on
>> Windows)
>>
>> @@ -1205,6 +1205,9 @@ test_expect_success POSIXPERM,PERL 'preserves existing per
>>            "die q(badrename) if ((stat(q(.git/config)))[2] & 07777) != 0600"
>>   '
>>   
>> +! test_have_prereq MINGW ||
>> +HOME="$(pwd)" # convert to Windows path
>> +
>>   test_expect_success 'set up --show-origin tests' '
>>          INCLUDE_DIR="$HOME/include" &&
>>          mkdir -p "$INCLUDE_DIR" &&
>>
>> is actually a much more concise version of my proposed patch,
>> although the result still misuses $HOME where it does not have
>> to. In fact, if I revert 5ca6b7bb (config --show-origin: report
>> paths with forward slashes), the tests still pass. But since it
>> does not make a difference save for a few microseconds more or
>> less during startup, it is not worth the churn at this point.
> 
> Well, from the point of view of codebase cleanliness, if we can do
> without 5ca6b7bb4, we would be much better off in the longer term,
> so I would say it would be wonderful if we can safely revert it.

Although I am convinced that the change is not necessary for
correctness, I can buy the justification that we should produce forward
slashes for consistency. There are a number of occasions where we
present paths to the user, and we do show forward-slashes in all cases
that I found. We should keep the commit.

But then let's do this:

---- 8< ----
Subject: [PATCH] Windows: shorten code by re-using convert_slashes()

Make a few more spots more readable by using the recently introduced,
Windows-specific helper.

Signed-off-by: Johannes Sixt <[email-masked]>
---
 abspath.c      | 5 +----
 compat/mingw.c | 9 ++-------
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/abspath.c b/abspath.c
index 5edb4e7..2825de8 100644
--- a/abspath.c
+++ b/abspath.c
@@ -167,7 +167,6 @@ const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
 	strbuf_add(&path, pfx, pfx_len);
 	strbuf_addstr(&path, arg);
 #else
-	char *p;
 	/* don't add prefix to absolute paths, but still replace '\' by '/' */
 	strbuf_reset(&path);
 	if (is_absolute_path(arg))
@@ -175,9 +174,7 @@ const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
 	else if (pfx_len)
 		strbuf_add(&path, pfx, pfx_len);
 	strbuf_addstr(&path, arg);
-	for (p = path.buf + pfx_len; *p; p++)
-		if (*p == '\\')
-			*p = '/';
+	convert_slashes(path.buf + pfx_len);
 #endif
 	return path.buf;
 }
diff --git a/compat/mingw.c b/compat/mingw.c
index 54c82ec..0413d5c 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -763,15 +763,12 @@ struct tm *localtime_r(const time_t *timep, struct tm *result)
 
 char *mingw_getcwd(char *pointer, int len)
 {
-	int i;
 	wchar_t wpointer[MAX_PATH];
 	if (!_wgetcwd(wpointer, ARRAY_SIZE(wpointer)))
 		return NULL;
 	if (xwcstoutf(pointer, wpointer, len) < 0)
 		return NULL;
-	for (i = 0; pointer[i]; i++)
-		if (pointer[i] == '\\')
-			pointer[i] = '/';
+	convert_slashes(pointer);
 	return pointer;
 }
 
@@ -2112,9 +2109,7 @@ static void setup_windows_environment()
 		 * executable (by not mistaking the dir separators
 		 * for escape characters).
 		 */
-		for (; *tmp; tmp++)
-			if (*tmp == '\\')
-				*tmp = '/';
+		convert_slashes(tmp);
 	}
 
 	/* simulate TERM to enable auto-color (see color.c) */
-- 
2.7.0.118.g90056ae