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

Replace all uses of Path with Utf8Path #1299

Open
willemneal opened this issue Apr 29, 2024 · 3 comments
Open

Replace all uses of Path with Utf8Path #1299

willemneal opened this issue Apr 29, 2024 · 3 comments

Comments

@willemneal
Copy link
Member

What problem does your feature solve?

https://docs.rs/camino/latest/camino/ ensures that the file path is utf8 and works on windows. Path uses OsSr under the hood, which allows non-utf8 strings. This is why there is pain when trying to go from a Path back to a &str. You either get an Option or a lossy string with invalid characters replaced with invalid character.

Enforcing utf-8 upfront means that .unwrap_unchecked can safely be used.

What would you like to see?

All instances of Path and PathBuf replaced with Utf8Path and Utf8PathBuf.

What alternatives are there?

@leighmcculloch
Copy link
Member

This is why there is pain when trying to go from a Path back to a &str. You either get an Option or a lossy string with invalid characters replaced with invalid character.

Where is this causing problems in the CLI?

@willemneal
Copy link
Member Author

Here are 24 examples of .to_string_lossy. We don't have any tests that cover this, but it definitely wouldn't be a problem for Utf8Path.
https://sourcegraph.com/search?q=context:global+repo:%5Egithub%5C.com/stellar/soroban-cli%24+.to_string_lossy&patternType=keyword&sm=0

@willemneal
Copy link
Member Author

Also if we ever have paths as strings in config files we would likely encode with utf8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

2 participants