Re: [PATCH 1/4] config --show-origin: report paths with forward slashes
Am 29.03.2016 um 22:05 schrieb Junio C Hamano:
Show 26 quoted lines
> Johannes Sixt <j6t@kdbg.org> 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 <j6t@kdbg.org>
---
abspath.c | 5 +----
compat/mingw.c | 9 ++-------
2 files changed, 3 insertions(+), 11 deletions(-)
Show changes to 2 files +3 −11
abspath.c, compat/mingw.c
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
Johannes Schindelin
