1Password / sys-locale

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

Add get_locales() method for preference-order list of locales #22

Closed Dinnerbone closed 1 year ago

Dinnerbone commented 1 year ago

It's much better to get and respect a list of preferred locales, rather than a single locale or nothing. For example, someone may prefer Dutch and then French, but if your app only supports English and French, you're likely going to give them a not-preferred English despite supporting something the user actually wants.

This Implements the list behaviour on windows and wasm, but I'm not able to test the other providers atm to be able to convert those over - they'll just return either an empty vec or a vec with a single element. Apple should be easy to change for someone who can test it, but I've no idea about linux or android.

This implements #14.

Dinnerbone commented 1 year ago

Thanks a lot for this PR! For the most part what's implemented so far looks good. My biggest suggestion would be that I think it may be better to have get_locales() return impl Iterator<Item = String> instead.

Good idea, I've made that change now. In doing so, unix, android and apple return to using Option<String> internally and .into_iter() for the public API. I think we can just leave android and unix as-is, and if you fix up Apple when you have time then that'd be appreciated!

Dinnerbone commented 1 year ago

Fixed comments, thanks for the review. I've pushed fixes as their own commits for ease of reviewing but I can squash it down (or you can during merge) if you'd prefer, it's a bit messy history now :D