houseabsolute / precious

One code quality tool to rule them all
Apache License 2.0
81 stars 4 forks source link

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

Open autarch opened 2 years ago

autarch commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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.