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

Add localization extraction to Theia CLI #10247

Merged
merged 1 commit into from
Oct 25, 2021

Conversation

msujew
Copy link
Member

@msujew msujew commented Oct 8, 2021

What it does

Adds an additional CLI command to the Theia-CLI that allows users to extract their localization keys and default values into a json file. This file can later be used to create translations for Theia using different services (e.g. the default mechanism proposed in #10187).

How to test

  1. Create a dummy TypeScript file like this:
import { nls } from '@theia/core/lib/browser/nls';
import { Command } from '@theia/core/lib/common';

const v1 = nls.localize('key1', 'value1');
const v2 = nls.localize('nested/key', 'value2');

const categoryKey = 'category-key';
const command = Command.toLocalizedCommand({
	id: 'id',
	label: 'command-label',
        category: 'category'
}, 'nested/command', categoryKey); // The CLI is able to resolve local references to constants
  1. Call the extract command on it using the following CLI call (tune the glob pattern accordingly to your setup):
yarn theia extract -p "**/*.ts" -o out.json
  1. Assert that all keys/default values have been correctly extracted from the input file.
  2. Try this with different command parameters
  3. Create an error by modifying the dummy file:
const test = { key: 'key' }; 
const v1 = nls.localize(test.key, 'value1');
  1. Assert that the cli finishes successfully and that any errors are printed into the console along with the source information (file path, line, character)

Review checklist

Reminder for reviewers

@msujew msujew added theia-cli issues related to the theia-cli localization issues related to localization/internalization/nls labels Oct 8, 2021
@msujew msujew mentioned this pull request Oct 8, 2021
8 tasks
@msujew msujew force-pushed the msujew/localization-extraction-cli branch from 720edfb to c195d41 Compare October 8, 2021 16:23
Copy link

@silvanocerza silvanocerza left a comment

Choose a reason for hiding this comment

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

I don't know enough knowledge in Typescript to review the code I think but all in all looks good.

In any case I tested the new command with arduino/arduino-ide and it works great. The json is created as expected and in case of failures the errors are clear and meaningful.

For what it's worth am approving this. 👍
Thanks @msujew! 🙏

Copy link
Contributor

@jbicker jbicker left a comment

Choose a reason for hiding this comment

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

Works exactly as described.

@msujew msujew force-pushed the msujew/localization-extraction-cli branch from c278672 to eb09f31 Compare October 18, 2021 18:23
@msujew msujew mentioned this pull request Oct 21, 2021
2 tasks
@msujew msujew force-pushed the msujew/localization-extraction-cli branch from eb09f31 to 42cba45 Compare October 25, 2021 09:19
@msujew
Copy link
Member Author

msujew commented Oct 25, 2021

@vince-fugnitto Just to make sure, there's nothing that stops me from merging this PR, right?

@vince-fugnitto
Copy link
Member

@vince-fugnitto Just to make sure, there's nothing that stops me from merging this PR, right?

@msujew I'll give it a quick review if that's alright? :)

@msujew
Copy link
Member Author

msujew commented Oct 25, 2021

@vince-fugnitto Please do so :)

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@msujew I confirmed that:

  • the source code changes look good
  • the different steps in 'how to reproduce' work well

LGTM! 👍

@msujew msujew merged commit de70cd3 into master Oct 25, 2021
@msujew msujew deleted the msujew/localization-extraction-cli branch October 25, 2021 12:40
@github-actions github-actions bot added this to the 1.19.0 milestone Oct 25, 2021
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 theia-cli issues related to the theia-cli
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants