command-line-api icon indicating copy to clipboard operation
command-line-api copied to clipboard

Consider a new localization design

Open jonsequitur opened this issue 3 years ago • 6 comments

    // A) I don't know of another area that exposes their localization resources
    // B) All of the methods need verbs; or to instead be properties.
    public class LocalizationResources 

jonsequitur avatar Nov 02 '22 16:11 jonsequitur

The current system can be used for these scenarios

  1. ensuring that custom parsers or validators output the same error messages as built-in ones used in the same application
  2. perhaps also for adding localisation for languages that are not natively supported by System.CommandLine, but I haven't yet tried that in practice

These are for user-visible strings, not for Exception.Message which often just goes to a log for server operators.

DateFormatInfo etc. classes also expose localisation but those have writable properties.

Microsoft.Extensions.Localization has IStringLocalizer, which could possibly be used instead of LocalizationResources, but then the key strings would have to be documented (and perhaps exposed as public constants), and it would become easier to make mistakes in parameterised strings.

KalleOlaviNiemitalo avatar Nov 05 '22 13:11 KalleOlaviNiemitalo

I took a look at the usage of LocalizationResources in dotnet/sdk as @jonsequitur told me that we added this type to satisfy the SDK requirements.

SDK defines a custom type CommandLineValidationMessages that derives from LocalizationResources. It overrides 14 out of 35 virtual methods. So not all strings are properly translated.

It defines it's own localizable strings: https://github.com/dotnet/sdk/tree/main/src/Cli/dotnet/CommandLineValidation/xlf I don't see why these translations should be part of SDK rather than System.CommandLine. Currently the translation work is duplicated and incomplete.

The instance of this custom type is passed to the configuration:

https://github.com/dotnet/sdk/blob/41ab0bc743b8b7effe145f0936a8fb3b48614060/src/Cli/dotnet/Parser.cs#L156

But it's not used everywhere it should:

https://github.com/dotnet/sdk/blob/41ab0bc743b8b7effe145f0936a8fb3b48614060/src/Cli/dotnet/Parser.cs#L245

https://github.com/dotnet/sdk/blob/41ab0bc743b8b7effe145f0936a8fb3b48614060/src/Tests/Microsoft.TemplateEngine.Cli.UnitTests/ParserTests/HelpTests.cs#L75

The APIs exposed by LocalizationResources are used in the SDK only by custom help builder:

https://github.com/dotnet/sdk/blob/main/src/Cli/Microsoft.TemplateEngine.Cli/Commands/create/InstantiateCommand.Help.cs#L319

And out of all 35 methods only 3 are used: HelpUsageTitle, HelpUsageOptions and HelpUsageCommand and they don't have a custom translation in CommandLineValidationMessages, moreover System.CommandLine currently is providing only English translation for these three texts ("Usage:", "[options]", "[command]").

I think we should do the following:

  1. Take translations defined in SDK and move them to System.CommandLine to avoid duplications.
  2. Ask the Community to translate the missing parts in System.CommandLine.
  3. Make LocalizationResources internal and sealed as System.* libraries that are part of base class libraries don't expose the possibility to provide custom translations for errors etc.
  4. Revisit the need for custom translations when working on HelpBuilder re-design.

adamsitnik avatar Feb 01 '23 15:02 adamsitnik

This gem in .NET SDK searches ParseResult.Errors for the localized UnrecognizedCommandOrArgument string:

https://github.com/dotnet/sdk/blob/98720693e228934405de152651b2e21390c3bfec/src/Cli/dotnet/ParseResultExtensions.cs#L41-L42

If LocalizationResources is made internal, then this will have to be implemented differently. Alternatives:

  • Copy all the UnrecognizedCommandOrArgument translations from System.CommandLine to .NET SDK and add a test to verify that they stay in sync -- both projects support exactly the same languages, and all translations match.
  • API change: add public enum ParseErrorKind { None, UnrecognizedCommandOrArgument, … }, a read-only property to class ParseError, and a parameter to SymbolResult.AddError in https://github.com/dotnet/command-line-api/pull/2033.
  • Assume that, if TreatUnmatchedTokensAsErrors is true, then each element of ParseResult.UnmatchedTokens results in one element of ParseResult.Errors; so you can just compare the number of elements and figure out whether there are any other errors.

KalleOlaviNiemitalo avatar Feb 01 '23 20:02 KalleOlaviNiemitalo

@KalleOlaviNiemitalo has identified my shame :smile: We could probably remove this extension method if we got all of the commands in the .NET CLI to accurately describe their CLI surface areas (or at least allow unmatched tokens and provide an Argument to aggregate them), but given the different teams that contribute to the CLI it's hard to enforce that kind of uniformity. In lieu of that, having a more structured way to check if the only errors are unknown token errors would serve our need.

baronfel avatar Feb 01 '23 20:02 baronfel

As far as I'm aware, we don't have another precedent for third-party translations for user-facing strings in a library like what we've enabled in System.CommandLine. We also don't have a process for accepting translations via OSS contributions. Figuring out the best way forward on this will take a bit longer and we don't want that decision to hold up the System.CommandLine GA, so the best course for now is to make these details internal, while adding out-of-the-box support the set of languages that are currently supported by the SDK.

The upside is that developers using System.CommandLine won't have to do anything special for their customers to see messages in those languages. This is an improvement over what we had in beta4.

The downside is that we don't yet have a story for additional languages.

jonsequitur avatar Feb 23 '23 02:02 jonsequitur

Copy all the UnrecognizedCommandOrArgument translations from System.CommandLine to .NET SDK and add a test to verify that they stay in sync -- both projects support exactly the same languages, and all translations match.

Since https://github.com/dotnet/sdk/pull/29746, .NET SDK assumes that the UnrecognizedCommandOrArgument translations match between System.CommandLine and CommandLineValidation.LocalizableStrings. However, they do not actually match. There are some differences in quotation marks and punctuation, and the es translations are very different: command-line-api, sdk. So, it appears that the proposed test was not implemented.

KalleOlaviNiemitalo avatar Sep 11 '23 10:09 KalleOlaviNiemitalo