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

Fix template list parsing on non-English systems #826

Merged
merged 1 commit into from Aug 24, 2021

Conversation

jmiven
Copy link
Contributor

@jmiven jmiven commented Aug 23, 2021

To list the available templates, FSAC parses directly the output of dotnet new --list, looking for predefined English words. But if the configured display language / locale of the system is another language (e.g. French or German), this parsing fails and generates an out-of-bounds exception.

This commit takes the simplest route to fix the problem: forcing dotnet to print its output in English by setting a documented environment variable[1].

Fixes ionide/ionide-vscode-fsharp#1569

[1] https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-environment-variables#dotnet_cli_ui_language

@baronfel
Copy link
Contributor

Thanks! Could you try adding a test to ensure we don't regress here? The tests for templates are here, and I think a good test would:

  • get the current locale (should be English, but worth checking here)
  • set the locale to something non-English
  • call the list function
  • ensure it parses
  • re-set the old locale

the tests can be run with dotnet run or dotnet test.

@jmiven
Copy link
Contributor Author

jmiven commented Aug 23, 2021

I agree that adding a regression test is important. But I'm hitting a roadblock, as I don't know .NET well for the time being. I've tried setting Thread.CurrentThread.CurrentCulture and CultureInfo.DefaultThreadCurrentCulture before calling the list function but neither of them modify the locale/culture for the new process.

Do you know how to do this?

@baronfel
Copy link
Contributor

Since new processes inherit their environment from their parents, I was thinking that you could use System.Environment's GetEnvironmentVariable and SetEnvironmentVariable methods to implement the steps above.

it might look something like

// save previous env
let priorEnv = System.Environment.GetEnvironmentVariable "DOTNET_CLI_UI_LANGUAGE"
// set env for this call
System.Environment.SetEnvironmentVariable("DOTNET_CLI_UI_LANGUAGE", "en-US")
// perform the test call
let templates = ....... // actual code goes here
// whatever the assertions are would go here
......
// put the old state back for the remainder of the tests
System.Environment.SetEnvironmentVariable("DOTNET_CLI_UI_LANGUAGE", priorEnv)

there might be some null-checking required there, but that's about what I was thinking.

To list the available templates, FSAC parses directly the output of `dotnet new --list`, looking for predefined English words. But if the configured display language / locale of the system is another language (e.g. French or German), this parsing fails and generates an out-of-bounds exception.

This commit takes the simplest route to fix the problem: forcing `dotnet` to print its output in English by setting a documented environment variable[1].

Fixes ionide/ionide-vscode-fsharp#1569

[1] https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-environment-variables#dotnet_cli_ui_language
@jmiven jmiven force-pushed the fix-ionide-vscode-fsharp-1569 branch from a2d7483 to ebed694 Compare August 24, 2021 15:46
@jmiven
Copy link
Contributor Author

jmiven commented Aug 24, 2021

I wasn't thinking of the DOTNET_CLI_UI_LANGUAGE, good idea, thanks!

Testing definitely was a good idea as my initial fix was wrong! :-)

@baronfel
Copy link
Contributor

This changeset looks great, I especially appreciate how you made the save and restore after use pattern work using a higher-order-function.

@baronfel
Copy link
Contributor

Your new tests look like they're passing just fine. I'll go ahead and merge this and we'll start working on a release with it soon. Thanks again for being willing to make this fix!

@baronfel baronfel merged commit a9a2416 into ionide:master Aug 24, 2021
@jmiven
Copy link
Contributor Author

jmiven commented Aug 24, 2021

Thanks for guiding me!

@jmiven jmiven deleted the fix-ionide-vscode-fsharp-1569 branch August 24, 2021 17:06
@baronfel
Copy link
Contributor

baronfel commented Sep 9, 2021

This has been released in FSAC 0.47.2

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

Successfully merging this pull request may close these issues.

Creation of a new project fails on Windows with a non-English setup
2 participants