Skip to content

Treat complete Windows UNC prefixes as absolute paths#62

Merged
chipsenkbeil merged 1 commit into
chipsenkbeil:mainfrom
dlenski:main
Apr 4, 2026
Merged

Treat complete Windows UNC prefixes as absolute paths#62
chipsenkbeil merged 1 commit into
chipsenkbeil:mainfrom
dlenski:main

Conversation

@dlenski
Copy link
Copy Markdown
Contributor

@dlenski dlenski commented Apr 1, 2026

I justify this in #61, specifically:

\\some.server\share should be considered equivalent to
\\some.server\share\, with .is_absolute() being true regardless of
whether or not the final \ is included. In the same way that
C:\whatever and C:\whatever\ are equivalent when .joining or
.pushing further path components.

@chipsenkbeil
Copy link
Copy Markdown
Owner

@dlenski good catch! Seems like there's a minor nitpick in clippy about using !is_empty() over len() > 0.

Outside of that, wondering out loud if we need to also add more tests for \\?\UNC\server\share (VerbatimUNC) and \\server (server-only), and maybe update our absolutize() test to have a case for this as well.

I justify this in chipsenkbeil#61,
specifically:

> `\\some.server\share` should be considered equivalent to
> `\\some.server\share\`, with `.is_absolute()` being `true` regardless of
> whether or not the final `\` is included.  In the same way that
> `C:\whatever` and `C:\whatever\` are equivalent when `.join`ing or
> `.push`ing further path components.
@dlenski
Copy link
Copy Markdown
Contributor Author

dlenski commented Apr 2, 2026

Seems like there's a minor nitpick in clippy about using !is_empty() over len() > 0.

Fixed, and cargo fmt too.

Outside of that, wondering out loud if we need to also add more tests for \\?\UNC\server\share (VerbatimUNC) and \\server (server-only), and maybe update our absolutize() test to have a case for this as well.

Yeah. As you can see, I added a couple of relevant doctests, but I do think that more Windows-specific tests for path-joining and absolutizing would be useful, given the many strange aspects of Windows paths.

@chipsenkbeil chipsenkbeil merged commit f355a0e into chipsenkbeil:main Apr 4, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants