From a810d1faa29803fb9d42a7d645a1ab41fe7a45de Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 15 Feb 2025 14:43:59 -0500 Subject: [PATCH] Remove TODOs about using `path-slash` to handle escapes The `to_unix_separators` and `to_windows_separators` functions in `gix_path::convert` had TODO comments saying they should use the `path-slash` crate "to handle escapes". These comments were added as part of e4f4c4b (#397) but the context there and in the broader related issue #301 does not seem to clarify the basis for this. It is not really clear what handling escapes would entail here, and it seems like there is not a way to do it without substantially changing the interface of these conversion functions in `gix-path`, which currently take a single argument representing a path and return a single string-like value also representing a path. If escape sequences appear in the input to such a path conversion function, it would not have a way to know if they are meant literally or as escape sequences. (An analogous concern applies if a function is to add escape sequences in its return value; it would have no way to know if the caller expects them.) Furthermore, while `path-slash` can convert some `\` paths to use `/` instead, it does not appear to do anything related to handling escape sequences or distinguishing which occurrences of `\` or any other character may be intended as part of an escape sequence. Its documentation (https://docs.rs/path-slash/latest/path_slash/) does prominently mention that `\` in escape sequences should not be converted to `/`: > On Unix-like OS, the path separator is `/`. So any conversion is > not necessary. But on Windows, the file path separator is `\`, > and needs to be replaced with `/` for converting the paths to > "slash paths". Of course, `\`s used for escaping characters > should not be replaced. But it looks like the part about `\` characters used for escaping is meant as advice on how and when to use `path-slash`, rather than meaning that `path-slash` would itself be able to distinguish between `\` characters meant as directory separators and `\` characters that perform quoting/escaping. --- gix-path/src/convert.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/gix-path/src/convert.rs b/gix-path/src/convert.rs index 4f2fe990ee0..8533fa63da2 100644 --- a/gix-path/src/convert.rs +++ b/gix-path/src/convert.rs @@ -232,7 +232,6 @@ pub fn to_unix_separators_on_windows<'a>(path: impl Into>) -> Cow< /// Replaces windows path separators with slashes, unconditionally. /// /// **Note** Do not use these and prefer the conditional versions of this method. -// TODO: use https://lib.rs/crates/path-slash to handle escapes pub fn to_unix_separators<'a>(path: impl Into>) -> Cow<'a, BStr> { replace(path, b'\\', b'/') } @@ -240,7 +239,6 @@ pub fn to_unix_separators<'a>(path: impl Into>) -> Cow<'a, BStr> { /// Find backslashes and replace them with slashes, which typically resembles a unix path, unconditionally. /// /// **Note** Do not use these and prefer the conditional versions of this method. -// TODO: use https://lib.rs/crates/path-slash to handle escapes pub fn to_windows_separators<'a>(path: impl Into>) -> Cow<'a, BStr> { replace(path, b'/', b'\\') }