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 method to Command #1801

Open
gylove1994 opened this issue Sep 15, 2022 · 30 comments
Open

Add localization method to Command #1801

gylove1994 opened this issue Sep 15, 2022 · 30 comments

Comments

@gylove1994
Copy link

gylove1994 commented Sep 15, 2022

Even though we can use .configureOutput() to configure the output to change the help header and error, but it is not convenient for user to change it entirely ( include error message and help header and etc. ) for localization.

I want to add a method to Command like program.localization('zh_CN') to change the output template words and error message could be Simplified Chinese entirely.

If it is passable to add , I will try to make a PR then.

Related Issues: #128 #774 #1608

Edit from @shadowspawn to readers: please 👍 this comment if you would like localisation support or support for customising strings added to Commander. You don't need to read all the comments!

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 15, 2022

PR related to #128 is #131

More related issues: #486 #1498

I think there were multiple requests to set version and help description for localisation, but could only find one comment:

FYI: @haochuan9421

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 15, 2022

I am interested in localisation, but I think this is a hard problem that will require a lot of code changes and make the code more difficult to write and maintain. So only a maybe, and worth discussing before investing a lot of time in a PR. Somewhat to my surprise, #128 did not get many likes or comments despite being open for six years. (If any readers are interested in localised strings for Commander, please 👍 the opening comment here so we know there is interest!)

I do not have experience with localisation in JavaScript.

  1. I have looked at it casually in the past, and wasn't sure how to handle building error messages. The code uses the convenient template literals a lot, like:
    const message = `error: too many arguments${forSubcommand}. Expected ${expected} argument${s} but got ${receivedArgs.length}.`;

Have you got an idea of how to handle these? (The parameters, not the plural. That is a separate issue on its own.)

In other languages I have used formatting support that allows numbered parameters so translations may change the order of the substituted parameters, like say "too many arguments %1. Expected %2". But I have not found anything native in Node.js that does this.

  1. If we add localisation, then we want people to be able to share them!

We did add an organisation recently, so now have somewhere that we could host localisation projects: https://github.com/commander-js

[Edit: I currently think we might ship localisations as part of Commander rather than separate packages.]

@gylove1994
Copy link
Author

Sorry for my slow reply.

It does need to change a lot of code to achieve it, so I will be waiting to handle the next step after the discussion result come out.

For the issue which how to handle error messages, I found some discussions here: javascript-equivalent-to-printf-string-format

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 17, 2022

The keys for the localised strings could be the original text, or an identifier.

{ 
   "Hello, world!": "¡Hola Mundo!" // english original as key
}
{
   helloWorld: "¡Hola Mundo!" // identifier as key
}

I slightly prefer the english original as the key, so can see the text and parameters all at once when used. Although for the errors, we do already have the code like commander.missingArgument.

Reference: https://www.transifex.com/blog/2015/naming-string-identifiers-best-practice/

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 17, 2022

Numbers and plurals are tricky in general, but I don't think there are many in Commander, perhaps just one:

    const message = `error: too many arguments${forSubcommand}. Expected ${expected} argument${s} but got ${receivedArgs.length}.`;

For interest, the Unicode CLDR has tags for: zero, one, two, few, many, and other.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 17, 2022

For interest, Yargs uses original string as key, and uses plural tags for (just) one and other:

https://github.com/yargs/yargs/blob/main/locales/en.json

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 18, 2022

Trying out numbered and named parameters:

const templated = `error: missing required argument '${name}'`;
const numbered = localise("error: missing required argument '{0}'", name);
const named = localise("error: missing required argument '{name}'", { name });

// or with code as key, and numbered
const coded = localise('commander.missingArgument', name);

[Edit, added] Tagged Template per #1801 (comment)

const taggedTemplate = localise`error: missing required argument '${name}'`;

@shadowspawn
Copy link
Collaborator

There are lots of interpolation conventions!

This guide includes three different examples:

  • polyglot: Welcome to my little spot on the interwebs, %{username}!
  • i18next: Welcome to my little spot on the interwebs, {{username}}!
  • Globalize: Welcome to my little spot on the interwebs, {username}!

We have simple message strings and don't need extra characters to avoid false matches, so {username} should be fine.

@shadowspawn
Copy link
Collaborator

This is not quite all the strings, but most of them.

Help

  • Usage:
  • Arguments:
  • Commands:
  • Options:

suggestSimilar

  • \n(Did you mean one of ${similar.join(', ')}?)
  • \n(Did you mean ${similar[0]}?)

version and help

  • commander.version: output the version number
  • commander.help: display help for command

commander.invalidArgument for argument

  • error: command-argument value '${value}' is invalid for argument '${argument.name()}'. ${err.message}
  • Allowed choices are ${this.argChoices.join(', ')}.

commander.invalidArgument for Option

  • error: option '${option.flags}' argument '${val}' is invalid.
  • error: option '${option.flags}' value '${val}' from env '${option.envVar}' is invalid.
  • ${invalidValueMessage} ${err.message}
  • Allowed choices are ${this.argChoices.join(', ')}.

commander.missingArgument

  • error: missing required argument '${name}'

commander.optionMissingArgument

  • error: option '${option.flags}' argument missing

commander.missingMandatoryOptionValue

  • error: required option '${option.flags}' not specified

commander.conflictingOption

  • error: ${getErrorMessage(option)} cannot be used with ${getErrorMessage(conflictingOption)}

commander.unknownOption

  • error: unknown option '${flag}'${suggestion}

commander.excessArguments

  • error: too many arguments${forSubcommand}. Expected ${expected} argument${s} but got ${receivedArgs.length}.

commander.unknownCommand

  • error: unknown command '${unknownName}'${suggestion}

Is it worth localising the author errors too? I guess so.

throw new Error

  • Command passed to .addCommand() must have a name
\nspecify the name in Command constructor or using .name()
  • only the last argument can be variadic '${previousArgument.name()}'
  • a default value for a required argument is never used: '${argument.name()}'
  • Unexpected value for event passed to hook : '${event}'.
\nExpecting one of '${allowedValues.join("', '")}'
  • To add an Option object use addOption() instead of option() or requiredOption()
  • passThroughOptions can not be used without turning on enablePositionalOptions for parent command(s)
  • call .storeOptionsAsProperties() before adding options
  • first parameter to parse must be array or undefined
  • unexpected parse option { from: '${parseOptions.from}' }
  • executableMissing is a huge multiline message!
  • '${executableFile}' not executable
  • Command alias can\'t be the same as its name
  • outputHelp callback must return a string or a Buffer [deprecated]
  • Unexpected value for position to addHelpText.\n
Expecting one of '${allowedValues.join("', '")}'

@gylove1994
Copy link
Author

gylove1994 commented Sep 19, 2022

I think I found an appropriate way to deal with interpolation using Tagged templates.

The advantage of using tagged templates:

  • ES6 native
  • Do less change to the original code

It works by adding a tag to the original template literals:

    const message = localization`error: unknown command '${unknownName}'${suggestion}`;

And all done.

Here have a demo in typescript: https://stackblitz.com/edit/typescript-qmax2v?devToolsHeight=33&file=index.ts

@shadowspawn
Copy link
Collaborator

Ah, I have seen Tagged Templates in past, but didn't understand how it would work here. The implementation is a bit subtle and the calling convention is a little unfamiliar at first, but does keep the code very similar using template to construct the string. Interesting.

I think the localisation keys will have to use numbers, I think the names get lost in the process. e.g. error: missing required argument {1}

An overview and example: https://javascript.plainenglish.io/template-literals-and-a-practical-use-of-tagged-templates-58526d525d72

@shadowspawn
Copy link
Collaborator

At least initially, would the localisation be an opt-in behaviour, or automatic?

I can see a few advantages to opt-in:

  • the author supplied text is likely to be in a single language
  • I am not sure how possible it is to get user locale preference on Windows
  • if it is opt-in, then can include the help and version options too (having these vary depending on end-user locale could introduce short-option conflicts with other commands)

@shadowspawn
Copy link
Collaborator

A reminder to myself. Localisation for the error messages will mean there is a better solution than overriding the private routines like optionMissingArgument to change the text, so a good time to eliminate them or at least rename to start with a leading underscore so clearer they are private.

@gylove1994
Copy link
Author

Sorry for my slow reply.

I am not quite sure I understood your thinking.

Do you want localisation can follow the user's PC locale preference automatically?

I think it's a good idea, I will try to find a way to achieve it.

By the way, I change the keys to use numbers like yours e.g. error: missing required argument {1} and I hosted a live demo to show how it works here: demo.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 21, 2022

Do you want localisation can follow the user's PC locale preference automatically?

I was suggesting we do not do automatic. Require author to select localisation.

But I am interested in how automatic might work! For reference, Sindre Sorhus writes lots of small utility packages and has one for os-locale. My quick look was that without spawning a command, it just uses environment variables, which I suspect does not work in native Windows shell.

Two problems I can see with automatic locale which do not occur if author sets locale to match their code:

  • author has written help descriptions in their language, but help section titles do not match for end-user (like Commands:)
  • author has chosen their short options to not clash with -h (help) and -V (version), but if end-user locale has different short options, may clash. (Or do other languages stick with -h and -V because they are the default?)

@shadowspawn
Copy link
Collaborator

I hosted a live demo to show how it works here

Neat!

Note that you can construct the lookup key from the string array passed to localization without referring to the localisations. Then look up the key in the map. If the key is not found, you can use the key itself as the template. (We might still want to lookup locale first, then English, then fallback to the key. To pickup minor fixes to the English.)

(Does not matter if my explanation is too vague to understand! The code details don't matter yet.)

@shadowspawn
Copy link
Collaborator

Is there value in letting people supply a partial or complete localisation map themselves, rather than only pick from supplied locales? This would let people change (say) just the help titles. Perhaps to add colour.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 21, 2022

I am excited. 🎉

I am still worried about the code changes and maintenance of the translations, but if there is interest from users then I think it could be worth it.

@shadowspawn shadowspawn added enhancement semver: major Releasing requires a major version bump, not backwards compatible and removed semver: major Releasing requires a major version bump, not backwards compatible labels Sep 21, 2022
@shadowspawn
Copy link
Collaborator

Sorry for my slow reply.

Two days? No problem! Please do not feel under pressure to reply quickly.

@gylove1994
Copy link
Author

Is there value in letting people supply a partial or complete localisation map themselves, rather than only pick from supplied locales? This would let people change (say) just the help titles. Perhaps to add colour.

Yes, that is the same thing I thought!

With the translationMap( But I think it is time to change its name now because we extended its usage ) and the tagged templates, we can give the authors an easy way to fully control template strings and the error messages directly compare to using configureOutput() to modify it.

@gylove1994
Copy link
Author

Neat!

I will do it right away, and I will try to add some new thoughts to it!

Thanks for your patience to read it and gave me suggestions!

@gylove1994
Copy link
Author

The new demo is ready: https://stackblitz.com/edit/typescript-qmax2v?devToolsHeight=33&file=types.ts

Mainly neat and added a customTransform() method and change some names.

@gylove1994
Copy link
Author

I found angular also use Tagged templates to do localize: https://angular.io/api/localize

@shadowspawn
Copy link
Collaborator

Another package with an API for locale support, both locales and custom strings: https://day.js.org/docs/en/customization/customization

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 27, 2022

I am thinking about what to try first. Being able to customise the strings would let people do things themselves without waiting for a full locale/localisation to be available. And does not require us deciding how to store the localisations and when they are loaded, and fallback et al.

  1. I am thinking perhaps a routine called .configureStrings(), following the naming of .configureOutput() and .configureHelp(). This would take an object with replacement templates/strings (the TemplateStrings type in the demo.)

  2. Probably start with some error messages, because they are fairly tidy and self-contained. (Or the Help strings would be fun seeing lots at once, but wouldn't use tagged template as much.)

I am not sure if the support should be built straight into Command or into another class. I guess start with adding to Command and see whether it takes more than a single property and routine.

@shadowspawn

This comment was marked as outdated.

@shadowspawn
Copy link
Collaborator

I opened a PR with some experimental work: #1807

I have added calls to the tagged template function for most of the error messages. I have not done the one that needs rethinking due to pasting together pieces of text including plurals.

Added en.js but currently just to collect together the key strings and notes on issues. My current focus is just on supporting the strings. I haven't looked deeper into file naming or file types or loading patterns for locales. (Used .js rather than .json so could add comments.)

@shadowspawn

This comment was marked as outdated.

@shadowspawn

This comment was marked as outdated.

@shadowspawn

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants