git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	git@vger.kernel.org, Lars Schneider <larsxschneider@gmail.com>,
	Kazutoshi SATODA <k_satoda@f2.dion.ne.jp>,
	Eric Wong <normalperson@yhbt.net>
Subject: Re: [PATCH 1/4] config --show-origin: report paths with forward slashes
Date: Sat, 2 Apr 2016 21:03:14 +0200	[thread overview]
Message-ID: <57001772.7090007@kdbg.org> (raw)
In-Reply-To: <xmqqwpolvyml.fsf@gitster.mtv.corp.google.com>

Am 29.03.2016 um 22:05 schrieb Junio C Hamano:
> 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

  reply	other threads:[~2016-04-02 19:03 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-22 17:42 [PATCH 0/4] Git for Windows fixes in preparation for 2.8.0 Johannes Schindelin
2016-03-22 17:42 ` [PATCH 1/4] config --show-origin: report paths with forward slashes Johannes Schindelin
2016-03-22 17:58   ` Junio C Hamano
2016-03-23  8:20     ` Johannes Schindelin
2016-03-23 16:34       ` Junio C Hamano
2016-03-28  7:58   ` Johannes Sixt
2016-03-28 15:14     ` Johannes Schindelin
2016-03-29 19:18       ` Johannes Sixt
2016-03-29 20:05         ` Junio C Hamano
2016-04-02 19:03           ` Johannes Sixt [this message]
2016-04-04 15:51             ` Junio C Hamano
2016-04-04 20:42               ` Johannes Schindelin
2016-03-30  5:52         ` Johannes Sixt
2016-04-02 18:51           ` Johannes Sixt
2016-03-22 17:42 ` [PATCH 2/4] Make t1300-repo-config resilient to being run via 'sh -x' Johannes Schindelin
2016-03-22 17:59   ` Jonathan Nieder
2016-03-22 20:34     ` Junio C Hamano
2016-03-22 23:45       ` Jonathan Nieder
2016-03-23  7:21         ` Johannes Schindelin
2016-03-22 18:01   ` Junio C Hamano
2016-03-23  8:22     ` Johannes Schindelin
2016-03-22 17:42 ` [PATCH 3/4] t1300: fix the new --show-origin tests on Windows Johannes Schindelin
2016-03-22 18:13   ` Junio C Hamano
2016-03-23 10:42     ` Johannes Schindelin
2016-03-22 17:43 ` [PATCH 4/4] mingw: skip some tests in t9115 due to file name issues Johannes Schindelin
2016-03-22 18:03   ` Jonathan Nieder
2016-03-22 18:30   ` Torsten Bögershausen
2016-03-22 20:44     ` Junio C Hamano
2016-03-22 22:44       ` Torsten Bögershausen
2016-03-22 22:57         ` Junio C Hamano
2016-03-23  5:54           ` Torsten Bögershausen
2016-03-23 10:49             ` Johannes Schindelin
2016-03-23 15:56               ` Junio C Hamano
2016-03-23 19:08                 ` Torsten Bögershausen
2016-03-23 22:44                 ` Torsten Bögershausen
2016-03-22 17:50 ` [PATCH 0/4] Git for Windows fixes in preparation for 2.8.0 Junio C Hamano
2016-03-23 10:54 ` [PATCH v2 " Johannes Schindelin
2016-03-23 10:55   ` [PATCH v2 1/4] config --show-origin: report paths with forward slashes Johannes Schindelin
2016-03-23 10:55   ` [PATCH v2 2/4] Make t1300-repo-config resilient to being run via 'sh -x' Johannes Schindelin
2016-03-23 10:55   ` [PATCH v2 3/4] t1300: fix the new --show-origin tests on Windows Johannes Schindelin
2016-03-23 11:08     ` Lars Schneider
2016-03-23 10:55   ` [PATCH v2 4/4] mingw: skip some tests in t9115 due to file name issues Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57001772.7090007@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=k_satoda@f2.dion.ne.jp \
    --cc=larsxschneider@gmail.com \
    --cc=normalperson@yhbt.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).