search

1Password / sys-locale

A small and lightweight Rust library to obtain the active locale on the system.
Apache License 2.0
81 stars 15 forks source link

Why would the library not be able to retrieve the os current locale ? #13

Closed antoninhrlt closed 1 year ago

antoninhrlt commented 1 year ago

We know that :

pub fn get_locale() -> Option<String>

In theory, it's possible to get a None value.

I did not look at the code in depth, so I don't know if it's really possible to get a None value. My question is : why would it be possible to not get the locale ? Why would the library not be able to retrieve the os current locale ? Besides, can I do the following code without being afraid of getting an error at runtime :

get_locale().unwrap();

README.md

Here is the code example I found in README.md :

let locale = get_locale().unwrap_or_else(|| String::from("en-US"));

I've raised questions from here.

complexspaces commented 1 year ago

Hi there @antoninhrlt,

I wouldn't consider the get_locale function infallible if you plan to distribute software to end users. If you plan to only run it on systems you control/own, this might be reasonable but I would still recommend falling back to a default language instead. If this is user-facing software, you could log a warning (or something like it) that lets someone know why their expected locale isn't being respected. This is exactly 1Password 8 does today, for example (with a fallback to en-US).

UNIX platforms are a good example to demonstrate the failure case with. This library is reliant on either the desktop environment or terminal to set the various locale environment variables. If this code is ran in a very different way, it could return None since the setup may not exist. Moving on to others: In theory get_locale shouldn't fail on Windows and macOS because they get data directly from OS APIs, but its non-ideal to rely on systems behaving perfectly in the real world.

In the end, it comes down to real-world systems being very diverse, which increases the chance of non-documented edge cases. If sys-locale were to panic in these cases, it would make the whole app unusable for users instead of just having a degraded localization experience.

antoninhrlt commented 1 year ago

@complexspaces Thanks for your reply, I appreciate it. It's complete and with many details. Have a nice day !