Re: [PATCH 1/4] config --show-origin: report paths with forward slashes
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Apr 2, 2016, 19:03 UTC
- Message-ID
- <57001772.7090007@kdbg.org>
- In-Reply-To
- <xmqqwpolvyml.fsf@gitster.mtv.corp.google.com>
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(-)
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