Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Absolute local paths on Windows not recognized #972

Open
Dobatymo opened this issue Mar 1, 2023 · 8 comments
Open

Absolute local paths on Windows not recognized #972

Dobatymo opened this issue Mar 1, 2023 · 8 comments
Labels
bug Something isn't working windows

Comments

@Dobatymo
Copy link

Dobatymo commented Mar 1, 2023

Sorry if I am using it wrong or it's duplicate report.

Running lychee (lychee-v0.11.1-windows-x86_64) on a local absolute path on Windows gives the following error:

>lychee.exe "C:\asd"
Error: Network error

Caused by:
    0: builder error for url (c:\asd): URL scheme is not allowed
    1: URL scheme is not allowed

Running it from that directory with lychee.exe . works!

@Dobatymo Dobatymo changed the title Absolute local paths on windows not recognized Absolute local paths on Windows not recognized Mar 1, 2023
@lebensterben
Copy link
Member

this indeed is a bug.

Windows path is technically URL and the lychee tries to parse the argument as URL before parse it as a file path. And in the case the drive is mistaken as a scheme.

A simple workaround is to change the said order. But it won't work because someone may name a drive as "http". so "http:\foo.bar\xyz" becomes both a legit Windows file path and a legit URL.

The true solution is:

Totally ban positional arguments. If you want to supply a file path, use --file, if you want to supply a glob, use --glob, etc.

@Dobatymo
Copy link
Author

Dobatymo commented Mar 2, 2023

Windows drive names can only be letters a-z, so http:\ would not be a valid Windows path. URLs would have to start with a schema and double slash, which Windows paths don't (ignoring network drives like \\server\path\ for now...

But yeah, I agree that paths/URLs are a mess and it's best to explicitly distinguish them!

@lebensterben
Copy link
Member

in my made-up example http:\foo.bar\xyz:

  • http is the drive name
  • \foo.bar\xyz is an absolute path

URLs would have to start with a schema and double slash

False. It doesn't have to contain double slash. Examples

  • mailto:foo@bar.com
  • foo:bar

@Dobatymo
Copy link
Author

Dobatymo commented Mar 2, 2023

Yes sorry, I was incorrect about the double slashes. However Windows doesn't really have drive names, only drive letters afaik.

@lebensterben
Copy link
Member

okay after some research it turns out you're right.

on windows the drive letter must be one of A-Z.

(for example see https://learn.microsoft.com/en-us/windows-server/storage/disk-management/change-a-drive-letter)

So we can indeed guess whether a positional argument is a absolute path with drive on Windows.

@mre
Copy link
Member

mre commented Mar 2, 2023

Can we collect a few test cases and the expected outcome here?

@lebensterben
Copy link
Member

we really should not abuse positional argument even though we can guess whether it's a file path or a URL.

I think in most cases people invoke lychee against specific files or a glob pattern, so the practical choice is to allow path and glob as the positional argument.

directly providing a URL to lychee is a niche case actually. it should either be done with a keyword argument or we simply remove this feature altogether.

@mre mre added the bug Something isn't working label Mar 9, 2023
@mre mre added the windows label Jun 24, 2024
@Silvenga
Copy link

FWIW, you can workaround this with using drive relative paths - which basically look like standard POSIX paths.

So, assuming the working directory is on the C: drive, you can use /Users/.... Windows is a lot more forgiving with paths than POSIX systems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working windows
Projects
None yet
Development

No branches or pull requests

4 participants