bilal-fazlani / commanddotnet

A modern framework for building modern CLI apps
https://commanddotnet.bilal-fazlani.com
MIT License
570 stars 29 forks source link

Localization of command and option description annotations. #352

Closed poison2k closed 2 years ago

poison2k commented 2 years ago

Hello, is there a proper way to localize the description tags of commands and options?

I localized the rest of my .NetCore console application with the ILocalizer.

Best regards

drewburlingame commented 2 years ago

Localization has not been implemented. We except PRs 😀

If localizing help output is good enough, I believe you can work around this by setting the relevant attributes to the localization keys and using a custom help provider to convert to Kerry to the localized text. If you do this, consider submitting the provider in a PR for us.

There are still a few errors the framework will output in English and they will need to be handled separately. We can keep this issue around for that work.

drewburlingame commented 2 years ago

When override the HelpTextProvider for this, if you're just localizing Description and ExtendedHelpText, then you'll only need to override the methods: ExtendedHelpTex(Command command) and ArgumentDescription(T argument)

poison2k commented 2 years ago

Thank you for your help, I played a little bit with an old test app and get a solution.

You can look it in my prototype repo, if you want to take over something.

https://github.com/poison2k/ConsoleApp

Here you find: CutomHelpProvider

drewburlingame commented 2 years ago

Looks good. Did you try subclassing the HelpTextProvider and only overriding the properties you needed?

poison2k commented 2 years ago

Yes, you are absolutely right, and it was my first intention, but not all needed parts are accessible in the class and the extension classes.

I changed it now but I had to copy some extension classes and privat functions.

drewburlingame commented 2 years ago

Ah, thank you, this is good feedback. I'll look at making those available.

drewburlingame commented 2 years ago

Looking through them real quick...

It looks like it's just CommandReplacements and PadFront that you'd need. That's an easy fix.

To replace the hard-coded text, it's probably best if we had a resx for all CommandDotNet strings and used that here.

If anyone wants to take on the work, we'll except PRs. The resx file wouldn't need to include all hard-coded strings in the first go. Iterative works if that's the bandwidth available.

poison2k commented 2 years ago

Thank you, and I will check if I can create a PR next week, if you see in my approach. I prefer to set string keys in a constant class and replace the hard-coded strings with this keys. This is for me best practice to work with resx files, because you have a centralized class with all keys you need to create a resx file. Then you don't need to search through the whole source code in the hope that you have found all needed resx keys.

drewburlingame commented 2 years ago

Interesting approach. When the resx compiles, don't the kyes serve as the constants class? In that way, you don't have a manual process to compare the strings in a constants class with the strings in the resx to make sure they're all covered.

poison2k commented 2 years ago

For me it is only to prevent typo errors or something else: In a localized app you can use:

Localizer["About"]
or
Localizer.GetString("About")

to get the value from the current culture but I prefer something like this:

public const string AboutKey = "About";

and call then the localizer with:

Localizer(Consts.AboutKey);

The localizer don't check if any key is set in a resx file because there is a culture fallback strategy. For Example: If the culture is set to en-US the localizer checks first the resx file.: mylocalize.en-US.resx if the key is not set it checks mylocalize.en.resx for the key, then: mylocalize.resx and if all files not include the key the localizer returns the search key as string and no error.

With the const file I will only prevent that a typo error in a localizer call will produce display errors, because the result is that the the wrong typed key will be shown. And it's easier to create new resx files because all needed keys are listed in the const file.

drewburlingame commented 2 years ago

I added a resx file to my project

image

Added a few entries

image

dotnet generated the constants file from the resx file

image

And this is now accessable from code

image

No need for magic strings in a constants file.

poison2k commented 2 years ago

Yes absolute true for this case. And now add localization to your app.

  1. So add localizaion to your service provider
  2. Add for example six allowed cultures, (en-US, en-GB, de-DE , fr-FR, hr-HR, nl-NL)
  3. Create the resx files for these cultures.
    LocalizableText.en-US.resx
    LocalizableText.en-GB.resx
    LocalizableText.de-DE.resx
    LocalizableText.fr-FR.resx
    LocalizableText.hr-HR.resx
    LocalizableText.nl-NL.resx
  4. Add your three arguments to all files with translated values.
  5. Set the Access Modifier of all resx files to "no code gen" because with the localizer function you don't need the overhead of the desinger.cs files
  6. Go to a service class and add a string output which depends on the current system culture.

And now try again your solution. I think you will get some problems.

But no problem, inject the IStringLocalizer to your service and use it to get the localized string depending on system culture with localizer.GetString() example:

localizer.GetString("Argument")

Ok we can hold your resx file and can do something like this:

localizer.GetString(namof(LocalizableText.Argument))

This is nearly the same what I do with my constant class but I have not the xml overhead of the needed base resx file.

Or am I missing something?

drewburlingame commented 2 years ago

I haven't had to work directly with localization for a while so the IStringLocalizer is new to me. Looks handy. What I remember is from the old days. :)

I tried this out using the HelpTextProvider, with the following changes.

https://github.com/bilal-fazlani/commanddotnet/blob/localization/CommandDotNet/Help/HelpTextProvider.cs#L23

image

image

By default, when adding a resx file with a culture in the name, it doesn't add the designer class.

And then added this LocalizeDirective to the CommandDotNet.Example app so I could test it

https://github.com/bilal-fazlani/commanddotnet/blob/localization/CommandDotNet.Example/LocalizeDirectives.cs

In English

image

In German

image

That's what I was thinking. Am I missing something?

drewburlingame commented 2 years ago

This approach won't work for values assigned in attributes. In that case, the constants file seems like the best approach.

drewburlingame commented 2 years ago

Are you not using resx with the IStringLocalizer? If not, what are you using?

With the IStringLocalizer approach, what if we use extension methods instead?

localizer.Help_Command();

With this approach, it's also clear when values will be interpolated into the string and what they are.

drewburlingame commented 2 years ago

Ok, I read up a bit about the updates to localization patterns. I think I'll go with an old-school pattern here that will support extending to use IStringLocalizer or any other pattern that comes along and does not burden consumers of the library with resource files and the perf impact of loading resource files when they are not needed.

I created HelpText and ErrorText with virtual members. The default is to return the string with no fluff. To localize, you'll inherit from HelpText, override the members to call IStringLocalizer using the value from the base member. I'll add a config method to accept the two overridden classes and they'll be available at HelpText.Instance and ErrorText.Instance. I generally avoid this Singleton pattern but I think it works well here and is available everywhere.

I'll create another package to support IStringLocalizer and where we can start capturing translations for CommandDotNet values.

I'll also add a tool to TestTool so consumers can detect when new values have been added that aren't yet localized from a subclass.

poison2k commented 2 years ago

Hi sorry, that I not reply directly, big workload .... Your solution sounds good, I will test your approch as soon as possible.

drewburlingame commented 2 years ago

@poison2k I've released a beta of localization in the latest version. I've included the package for FluentValidation and DataAnnotations. https://commanddotnet.bilal-fazlani.com/localization/overview/

Give it a spin and let me know what you think. There are still strings to localize but this should cover most cases.

I want to add a package to provide resx files and another for json, and tooling to identify missing entries in those files.

drewburlingame commented 2 years ago

Alright, I've finished localizing the files I think need localizing, and I've generated json and resx files in English. At this point, it's all released. I just need feedback on how it works and if you need anything changed. I'd also welcome the translation files you generated to make those available to the community.

Hope this takes care of your needs.

Cheers, Drew

drewburlingame commented 2 years ago

@poison2k Did you find this approach worked for you? I'll close this issue this week if there's nothing else to be done.

poison2k commented 2 years ago

Sorry for the very late reply. your approch worked. But I also created a pull request today and made some changes and suggestions for the localization.