hotfix for os-locale on some windows platforms #240
Conversation
| @@ -531,6 +531,14 @@ function Argv (processArgs, cwd) { | |||
| return argv | |||
| } | |||
|
|
|||
| function guessLocale () { | |||
| try { | |||
| return osLocale.sync() | |||
There was a problem hiding this comment.
This is still a bit worrisome for Atom's case where app startup time is critical and this call execs processes synchronously.
There was a problem hiding this comment.
two options I see:
- is there an environment variable I can detect and flip this feature off specifically for Atom.
- what if I add an option
.detectLocale(false)?
Shall I release this in the interim to fix the exception?
There was a problem hiding this comment.
Shall I release this in the interim to fix the exception?
I think Atom will just pin to version 3.19.0 for now and cut a new release.
Just a heads up, earlier this week we moved apm to use 3.19.0 as well because of issues with long paths on Windows introduced by the os-locale addition. I know this is a Windows issues, but I just wanted to mention it in case you came across it on that repo.
what if I add an option .detectLocale(false)?
👍 That seems good
There was a problem hiding this comment.
It only spawns when neither of the following environment variables are defined:
env.LC_ALL || env.LANGUAGE || env.LANG || env.LC_MESSAGES
Might be a good idea to add an option to os-locale to prevent spawning {spawn: false}?
|
Fixed in I guess this PR still makes sense though, just in case. |
|
@kevinsawicki @sindresorhus I'm going to get this hot fix out the door so that we're not breaking anything on Windows, we can discuss in this thread: |
hotfix for os-locale on some windows platforms
If os-locale throws, we will now default to en when detecting locale.
CC: @sindresorhus, @kevinsawicki, @MatthewNewland