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

Paths should be treated as OsStr, not regular strings #27

Open
autarch opened this issue Aug 27, 2022 · 2 comments
Open

Paths should be treated as OsStr, not regular strings #27

autarch opened this issue Aug 27, 2022 · 2 comments
Labels

Comments

@autarch
Copy link
Member

autarch commented Aug 27, 2022

There are places where we call to_string_lossy on paths while we're doing things like building up a command as a slice of strings. But there's no good reason to force these things to be UTF-8. We should use OsStr for this stuff instead.

@autarch
Copy link
Member Author

autarch commented Aug 27, 2022

I did some work on this in the os-str branch but I realized it's probably not worth continuing. Unless all the commands that Precious invokes also work with non-UTF-8 files, there's no point in supporting them in Precious. I suspect many commands will not work with them. I couldn't even get rustfmt to work with a non-UTF-8 filename.

@plugwash
Copy link

I would argue that in the context of code quality tools, it is not unreasonable to regard such filenames as an error.

On windows, filenames are nominally stored in UTF-16, and the only time that conversion of UTF-16 to UTF-8 can fail is if the UTF-16 was not valid in the first place.

On Unix-like systems filenames were historically a sequence of bytes in a locale-dependent encoding, but utf-8 locales are now by far the dominant choice, a filename in a non utf-8 encoding will only be meaningful on a small minority of systems.

However I would argue that using to_string_lossy to build up a command is tantamount to ignoring errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants