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

nls: rename command that extract localizations #10324

Merged
merged 1 commit into from
Oct 26, 2021
Merged

Conversation

paul-marechal
Copy link
Member

Rename the @theia/cli command to extract localizations from sources to
something that would make it explicit enough. extract alone is a bit
too generic.

Change --pattern/-p into an array of --files/-f.

Minor changes.

How to test

Same as #10247

  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 nls-extract --files "**/*.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

Rename the `@theia/cli` command to extract localizations from sources to
something that would make it explicit enough. `extract` alone is a bit
too generic.

Change `--pattern/-p` into an array of `--files/-f`.

Minor changes.
@paul-marechal paul-marechal requested a review from msujew October 25, 2021 22:42
@paul-marechal
Copy link
Member Author

@msujew I forgot to submit the small review I made for your PR that adds the localization extractor :S

Here are some suggested changes (just few renames really) that I think would be better. Since those are not published yet I figured now is the time to nitpick.

Tell me what you think :)

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

LGTM! The names fit better and everything continues to work as expected 👍

@paul-marechal paul-marechal merged commit 2c63c96 into master Oct 26, 2021
@paul-marechal paul-marechal deleted the mp/nls-extract branch October 26, 2021 17:24
@github-actions github-actions bot added this to the 1.19.0 milestone Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants