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

Not localized #29098

Closed
EbXpJ6bp opened this issue Jun 20, 2017 · 14 comments
Closed

Not localized #29098

EbXpJ6bp opened this issue Jun 20, 2017 · 14 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug l10n-platform Localization platform issues (not wrong translations) verified Verification succeeded
Milestone

Comments

@EbXpJ6bp
Copy link
Contributor

EbXpJ6bp commented Jun 20, 2017

  • VSCode Version: Code 1.13.1 (379d2ef, 2017-06-14T18:21:47.485Z)
  • OS Version: Windows_NT ia32 10.0.15063
  • Extensions: the listing length exceeds browsers' URL characters limit

Steps to Reproduce:

  1. Change display language to "ja"(any)
  2. Click a git sync button in the status bar

localize points (secondary modules, in /extensions) do not work.

I guess they cannot get nls.config(process.env.VSCODE_NLS_CONFIG)().

Example:

extensions/git/src/main.ts (has nls.config)
inked863f6ddc36157e20498dbc62dbe523b3_li

extensions/git/src/commands.ts (not work)
inkedd147eccc09f515f06e7a69b9542b93e2_li

And, It seems that some Extensions(such, merge-conflict) have no nls.config.

@bpasero
Copy link
Member

bpasero commented Jun 21, 2017

Maybe the translations have not made it in yet?

@EbXpJ6bp
Copy link
Contributor Author

EbXpJ6bp commented Jun 21, 2017

It is translated.
In fact, changing localize = nls.loadMessageBundle(__filename); to localize = nls.config(process.env.VSCODE_NLS_CONFIG)(__filename); in Microsoft VS Code\resources\app\extensions\git\out\commands.js will work.
inkedfc1a7cfdbe3729fd19673aa2f53c9dce_li

I do not know secondary modules defined by vscode-nls, but I think it is the cause of the issue.
The issue is happening not only in the git extension but also in other built-in extensions.

@joaomoreno joaomoreno assigned dbaeumer and unassigned joaomoreno Jun 22, 2017
@dbaeumer
Copy link
Member

Strangely this works for TS and other extensions. I don't know what Git does different here. Could be a problem with the tool that extracts the strings.

@dbaeumer dbaeumer added l10n-platform Localization platform issues (not wrong translations) bug Issue identified by VS Code Team member as probable bug labels Jun 23, 2017
@dbaeumer dbaeumer added this to the June 2017 milestone Jun 23, 2017
@EbXpJ6bp
Copy link
Contributor Author

EbXpJ6bp commented Jun 23, 2017

As far as I confirmed, the following is also no translated.(It's already in i18n folder)
Would you debug it?

  • php (a portion)
  • markdown (a portion)
  • merge-confilict (insiders)

Also, if you don’t mind, could you tell me how to debug the Code-OSS including the display language function?

@dbaeumer
Copy link
Member

@EbXpJ6bp you can't since nls is a build time step to make dev time expierence easy. You can only debug this in a build version which you can produce using the gulp script vscode-win32-x64

@EbXpJ6bp
Copy link
Contributor Author

@dbaeumer Thanks!

@michelkaporin
Copy link
Contributor

michelkaporin commented Jun 28, 2017

Apparently the issue was that message bundles were loaded before nls configuration was happening, thus defaulting to English locale.

https://github.com/Microsoft/vscode-nls indicates that

The config call configures the nls module and should only be called once in the applications entry point.

I fixed it for git, php, markdown and merge-conflict extensions.

FYI @joaomoreno @chrmarti @mjbvz

@michelkaporin
Copy link
Contributor

michelkaporin commented Jun 28, 2017

@Microsoft/vscode if you own any extension that is localized, please ensure that nls.config() is called before any other code in your extension, otherwise localization won't be working for you.

See ab3c945 for reference 🙂

@joaomoreno
Copy link
Member

joaomoreno commented Jun 28, 2017

@michelkaporin We're lucky extensions are built to CommonJS... AMD wouldn't make this easy.

@michelkaporin
Copy link
Contributor

michelkaporin commented Jun 28, 2017

@joaomoreno yeah, we would have to redesign all our localization logic...

@chrmarti
Copy link
Contributor

@michelkaporin Should we replicate that change across all extensions or only for specific cases?

@michelkaporin
Copy link
Contributor

@chrmarti across all that use nls, but only for extension entrypoints.

@dbaeumer
Copy link
Member

@michelkaporin Thanks for looking into this.

@dbaeumer dbaeumer added the verified Verification succeeded label Jun 29, 2017
@dbaeumer
Copy link
Member

Verified:

capture

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug l10n-platform Localization platform issues (not wrong translations) verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants