cleanup_path: force forward slashes on Windows #1972
Conversation
Git prefers forward slashes as directory separators across all platforms. On Windows, the backslash is the native directory separator, but all Windows versions supported by Git also accept the forward slash in all but rare circumstances. Our tests expect forward slashes. Git generates relative paths using forward slashes. Forward slashes are more convenient to use in shell scripts. For these reasons, we enforced forward slashes in `interpolate_path()` in 5ca6b7b (config --show-origin: report paths with forward slashes, 2016-03-23). However, other code paths may generate paths containing backslashes. For example, `config --show-origin` prints the XDG config path with backslashes on Windows: $ git config --list --show-origin file:C:/Program Files/Git/etc/gitconfig system.foo=bar file:"C:\\Users\\delilah/.config/git/config" xdg.foo=bar file:C:/Users/delilah/.gitconfig home.foo=bar file:.git/config local.foo=bar Let's enforce forward slashes in all code paths that directly or indirectly call `cleanup_path()` by modifying it to call `convert_slashes()` on Windows. Since `convert_slashes()` modifies the path in-place, change the argument and return type of `cleanup_path()` from `const char *` to `char *`. All existing callers of `cleanup_path()` pass `char *` anyways, so this change is compatible. Suggested-by: Johannes Schindelin <[email protected]> Signed-off-by: Delilah Ashley Wu <[email protected]>
delilahw
force-pushed
the
lilah/fix-xdg-path-slashes-windows/patchset
branch
from
89b6531 to
bdd6252
Compare
delilahw
changed the title
|
Feedback addressed in the new commit message, thanks @dscho! 😊 I should probably add some tests to check that the XDG path uses the correct slashes, in either My other patch series, #1938, will exercise a similar scenario: Lines 2427 to 2432 in 5b253d6 |
|
@delilahw how about inserting the current patch as a first commit into #1938, mentioning in the commit message that the next patch adds a test case that validates the newly-adjusted logic anyway, therefore that first commit refrains from adding a redundant test case? Then you could go ahead and submit #1938. |
|
Ohhhh yep that makes sense. Thanks so much for your guidance @dscho !! |
|
Superseded by #1938. |
cc: Delilah Ashley Wu [email protected], Delilah Ashley Wu [email protected]
cc: Derrick Stolee [email protected]
cc: Johannes Schindelin [email protected]