git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Delilah Ashley Wu <delilahwu@linux.microsoft.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Nils Fahldieck <nils@fahldieck.de>,
	 Patrick Steinhardt <ps@pks.im>,
	Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>,
	 Delilah Ashley Wu <delilahwu@microsoft.com>,
	Derrick Stolee <stolee@gmail.com>,
	 Ben Knoble <ben.knoble@gmail.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v2 1/3] path: use forward slashes in XDG config on Windows
Date: Thu, 10 Sep 2026 14:48:54 +1000	[thread overview]
Message-ID: <aqIvJhLLcCSnyaL4-delilahwu@linux.microsoft.com> (raw)
In-Reply-To: <xmqqecfkhify.fsf@gitster.g>

Thanks for the reviews! I'm still working through the feedback for v3.


On Wed, Aug 26, 2026 at 10:58:57AM +1000, Junio C Hamano wrote:
> Is this "force forwared slashes to Windows users" a required part of
> XDG/HOME global fix?  If not, please leave it out of the topic. [...]
> Again, I do not see it explained why this change has to be part of
> this series in the proposed log message, so...?

Sorry, I forgot to explain in the log message that this change is
supposed to supplement the `--show-origin` tests added in patch 3
(config: read global scope via config_sequence). Without it, the
`--show-origin` would output a path with mixed slashes on Windows:

    file:"C:\\Users\\delilah/.config/git/config"    xdg.foo=bar

The tests expect paths containing only forward slashes. So patch 3
modifies `t1300-config.sh` assertions to look like this:

    echo "file:$HOME/.config/git/config    xdg.config=xdg" >expect
    git config list --global --show-origin >actual
    test_cmp expect actual

where `$HOME` has been normalised to contain forward slashes only, as
seen at `t1300-config.sh:2179`, which was introduced in 45bf329 (t1300:
fix the new --show-origin tests on Windows):

    HOME="$(pwd)" # convert to Windows path


> Even if it is a good idea to always force forward slashes to Windows
> users (I have no strong opinions on the topic), and if it is very
> unlikely to break existing Windows users (I do not have any clue if
> that would be the case or not, as I do not do Windows), we would
> want to make sure if we can get the same effect without sprinkling
> "#ifdef" in the platform agnostic part of the codebase like "path.c"
> file.

I followed an existing usage of `#ifdef GIT_WINDOWS_NATIVE` and
`convert_slashes()` in `path.c`, but if it's no longer allowed in the
platform agnostic part, we could do the slash conversion elsewhere. This
assumes we want to keep converting the slashes, but we should reconsider
from your points raised below.


> Where would the slash in "ret" that is passed to convert_slashes()
> function come from?  If they come from environment variables like
> XDG_CONFIG_HOME and HOME, that is end-user's preference and we have
> no business forcing them which forms of slashes to use.

The slash in `ret` would come from the environment variables, so perhaps
we should not modify the slashes in them at all. Instead, I could drop
this patch and change the tests in patch 3 to export a `XDG_CONFIG_HOME`
value containing only forward slashes. This would satisfy the assumption
that paths in the tests will contain only forward slashes. What do you
think?


Thanks!
Delilah =)

  reply	other threads:[~2026-09-10  4:49 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-10  1:14 [PATCH/RFC 0/4] config: read both home and xdg files for --global Delilah Ashley Wu via GitGitGadget
2025-10-10  1:14 ` [PATCH/RFC 1/4] cleanup_path: force forward slashes on Windows Delilah Ashley Wu via GitGitGadget
2025-11-19 17:47   ` Junio C Hamano
2025-10-10  1:14 ` [PATCH/RFC 2/4] config: test home and xdg files in `list --global` Delilah Ashley Wu via GitGitGadget
2025-11-19 18:29   ` Junio C Hamano
2025-10-10  1:14 ` [PATCH/RFC 3/4] config: read global scope via config_sequence Delilah Ashley Wu via GitGitGadget
2025-11-19 18:39   ` Junio C Hamano
2025-10-10  1:14 ` [PATCH/RFC 4/4] config: keep bailing on unreadable global files Delilah Ashley Wu via GitGitGadget
2025-10-10  1:27 ` [PATCH/RFC 0/4] config: read both home and xdg files for --global Kristoffer Haugsbakk
2025-11-22  1:36   ` Delilah Ashley Wu
2026-01-20 20:41     ` Junio C Hamano
2025-11-17 13:29 ` Johannes Schindelin
2025-11-18  0:28   ` Junio C Hamano
2025-11-19 14:44 ` Junio C Hamano
2025-11-22  2:00   ` Delilah Ashley Wu
2026-08-23 10:28 ` [PATCH v2 0/3] " Delilah Ashley Wu
2026-08-23 10:28   ` [PATCH v2 1/3] path: use forward slashes in XDG config on Windows Delilah Ashley Wu
2026-08-26 17:58     ` Junio C Hamano
2026-09-10  4:48       ` Delilah Ashley Wu [this message]
2026-08-23 10:28   ` [PATCH v2 2/3] config: let sequence require a successful file Delilah Ashley Wu
2026-08-26 18:20     ` Junio C Hamano
2026-08-23 10:28   ` [PATCH v2 3/3] config: read global scope via config_sequence Delilah Ashley Wu
2026-08-26 18:38     ` Junio C Hamano
2026-08-23 12:36   ` [PATCH v2 0/3] config: read both home and xdg files for --global Chris Torek
2026-08-24  1:32     ` Junio C Hamano

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

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

  git send-email \
    --in-reply-to=aqIvJhLLcCSnyaL4-delilahwu@linux.microsoft.com \
    --to=delilahwu@linux.microsoft.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=ben.knoble@gmail.com \
    --cc=delilahwu@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kristofferhaugsbakk@fastmail.com \
    --cc=nils@fahldieck.de \
    --cc=ps@pks.im \
    --cc=stolee@gmail.com \
    /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.
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 NNTP newsgroup(s).