Skip to content

Conversation

@Zebsterpasha
Copy link
Contributor

What it does

Fixes switching the localization language back to the default #15444

How to test

  1. Launch IDE
  2. Press F1
  3. Configure Display Language to something other than default
  4. IDE will reboot
  5. Make sure getNlsConfig() returns the correct language
    env['VSCODE_NLS_CONFIG'] = JSON.stringify(this.localizationService.getNlsConfig());
  6. Switch the language to the default language (e.g. English)
  7. getNlsConfig() will return the default language (breakpoint from point 5). Before the fix, it returned the previous language instead of the default language.

Follow-ups

Breaking changes

  • This PR introduces breaking changes and requires careful review. If yes, the breaking changes section in the changelog has been updated.

Attribution

Review checklist

Reminder for reviewers

@github-project-automation github-project-automation bot moved this to Waiting on reviewers in PR Backlog Apr 11, 2025
Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Zebsterpasha,

Thank you for your PR! It seems that this change was introduced as part of #13134 as an optimization:

Update I18nPreloadContribution to ensure that we only load the localizations from the back end if the current locale is not equal to the default locale. This fixes an issue where we unnecessarily queried the localization server in the preload step if the locale is explicitly set to "en". (This happens for instance if you change the display language and then switch back to english).

However, it seems that due to that we are missing the setCurrentLanguage call that the server is making in the backend when we load the localizations:

this.localizationProvider.setCurrentLanguage(languageId);

@tortmayr Do you remember if there was any issue with loading localizations for the default language in particular? It seems to me that loading them should be quite fast anyways and I don't see why we would never want to load for the default locale as other localizations may have been added. Unless there is a good reason for this, I'm inclined to accept this PR (as soon as the CI is happy as well).

@msujew msujew added the localization issues related to localization/internalization/nls label Apr 14, 2025
@tortmayr
Copy link
Contributor

@tortmayr Do you remember if there was any issue with loading localizations for the default language in particular? It seems to me that loading them should be quite fast anyways and I don't see why we would never want to load for the default locale as other localizations may have been added. Unless there is a good reason for this, I'm inclined to accept this PR (as soon as the CI is happy as well).

I don't recall exactly. I think it gave a slight performance boost when using the default locale but nothing to significant.
Anyways, since there are issues with it we should accept this PR.
If we encounter significant performance degradation (witch I don't expect) we can handle this in a follow-up.

Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, then I'm accepting this PR. However, there seems to be an issue in the CI that seems unrelated to me: https://github.com/eclipse-theia/theia/actions/runs/14401863140/job/40497162158?pr=15445#step:7:1007. @JonasHelming Do you know whether there is a known issue in the CI?

@github-project-automation github-project-automation bot moved this from Waiting on reviewers to Needs merge in PR Backlog Apr 14, 2025
@JonasHelming
Copy link
Contributor

@martin-fleck-at Yes it is known

@martin-fleck-at martin-fleck-at merged commit 56d8304 into eclipse-theia:master Apr 15, 2025
10 of 11 checks passed
@github-project-automation github-project-automation bot moved this from Needs merge to Done in PR Backlog Apr 15, 2025
@github-actions github-actions bot added this to the 1.61.0 milestone Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

localization issues related to localization/internalization/nls

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants