make sure OOP inherit same culture as VS otherwise culture sensitive …#24332
make sure OOP inherit same culture as VS otherwise culture sensitive …#24332heejaechang merged 2 commits intodotnet:dev15.6.xfrom
Conversation
…resource such as xml doc comment will not work properly
|
@dotnet/roslyn-ide @jinujoseph @Pilchie can I get code review and approval for 15.6? quite simple change. |
|
@NTaylorMullen I am fixing it in 15.6 branch. is that okay? |
|
@jinujoseph @NTaylorMullen which release should this go? right now I targeted it to dev15.6.x which I believe is for next preview? |
15.6 is totally fine for us. |
| var uiCultureLCIDE = 0; | ||
| var cultureLCID = 0; | ||
|
|
||
| var host = await instance._rpc.InvokeAsync<string>(nameof(IRemoteHostService.Connect), current, uiCultureLCIDE, cultureLCID, telemetrySession).ConfigureAwait(false); |
There was a problem hiding this comment.
Can't you change the culture within a VS session? How does that get propagated to the remote host?
There was a problem hiding this comment.
this is a mock for testing. this code path just ignore culture.
There was a problem hiding this comment.
ah. mis-understood your question. no you can't change culture within VS session. you need to re-open VS.
| // make sure connection is done right | ||
| var host = await client._rpc.InvokeWithCancellationAsync<string>( | ||
| nameof(IRemoteHostService.Connect), new object[] { current, TelemetryService.DefaultSession.SerializeSettings() }, cancellationToken).ConfigureAwait(false); | ||
| nameof(IRemoteHostService.Connect), new object[] { current, uiCultureLCID, cultureLCID, TelemetryService.DefaultSession.SerializeSettings() }, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
this is real one. and RemoteHostService set culture of OOP same as VS one which we got here.
There was a problem hiding this comment.
well. I could change CultureInfo.CurrentUICuture to CultureInfo.DefaultCurrentUICuture.
There was a problem hiding this comment.
How would that help? If the VS culture changes, that's not going to update the CultureInfo.DefaultCurrentUICuture.
There was a problem hiding this comment.
I was just saying changing CultureInfo.CurrentUICuture to CultureInfo.DefaultCurrentUICuture since someone could have changed current threads culture.
not sure what you mean by
How would that help?
about
If the VS culture changes, that's not going to update the CultureInfo.DefaultCurrentUICuture.
you can't change VS culture without close and re-open VS, so I don't need to monitor VS culture setting change.
There was a problem hiding this comment.
Got it. For some reason I didn't realize it required a restart.
|
Tagging @tmeschter as a "World Ready" issue. |
| // report exception | ||
| WatsonReporter.Report(ex); | ||
|
|
||
| // ignore expected exception |
There was a problem hiding this comment.
If they are expected, why are we reporting them to Watson?
There was a problem hiding this comment.
I still want to see how often VS uses wrong LCID and what LCID it uses.
| // report exception | ||
| WatsonReporter.Report(ex); | ||
|
|
||
| // ignore expected exception |
There was a problem hiding this comment.
If they are expected, why are we reporting them to Watson?
|
@Pilchie let me know if there is something you want me to change. |
|
Approved - this will show up in 15.6 Preview 4. |
…resource such as xml doc comment will not work properly
Customer scenario
User sets VS language to something else than system language (culture), but in some cases, still get descriptions such as xml documment or diagnostic description in system language in some features
Bugs this fixes
#24317
Workarounds, if any
there is no workaround
Risk
the fix is quite straight forward. and low risk
Performance impact
I don't see any impact on perf
Is this a regression from a previous update?
No
Root cause analysis
Roslyn out of process didn't set default culture for its app domain and use default system culture when VS supports setting different culture than system culture.
How was the bug found?
Dogfooding, customer report