bilal-fazlani / commanddotnet

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

Localisation #431

Closed poison2k closed 1 year ago

poison2k commented 2 years ago

I updatet the current resx files in the repository to make them useable. The problem was, that the Name property in the resx was equal to the value that can't work because the name field is the idenifier and can inherit some special characters. I also escaped the special charakters in the Value property.

I added also a German localisation for test usage. However, it would be helpful overall if partial translations are not inserted into other messages. This makes translation into other languages difficult.

I added also an localisaation Example to show how the localization currently works. The Example inherits two language examples en-GB and de-DE yuu can switch the culture in the appesettings.json.

drewburlingame commented 2 years ago

Hi @poison2k. Thanks for taking the time to contribute german translations and to create an example app.

First of all, please forgive any brevity or criticism that feels pointed. It's not intended that way. I'm on holidays and trying to be thorough but also quick so I fear some of my questions and suggestions may be less thoughtful than I'd like.

re: resx translations

I remember now there were some issues with the resx format but I didn't follow up on it at the time because I thought there would be more interest in the new IStringLocalizer pattern using json and not much for resx. I'm sure this is due to my not working directly with localization for quite a while.

I see that you created the CoreResources override for to use keys resx supports. I'm currently using a test class to auto-generate both the resx and the json files from the resources classes. It was quick and dirty because I didn't have time to make it more elegant and as I mentioned above, I didn't think there would be much desire for resx so I didn't want to put time into it. I think we need to find a more sustainable approach that will ensure new resource strings will be included in the resx files and in the CoreResources class and we need to find a way to make CoreResources part of the core. I don't want to be responsible for ensuring these are manually added every time. I'm sure I will forget. Any thoughts?

re: example app

One of the things I've realized with this framework is that a lot of the people who find it are hobbyists or relatively new to programming. This is why I spent a lot of time rewriting the help and adding the getting started docs. These users don't have the experience to know what is required for the example and what is extra, so it's important for the examples to be as bare bones as possible. Users will more easily understand it this way. I've had to spend a fair amount of time groking the many parts of this example and I'm familiar with CommandDotNet.

I'm wondering if we can simplify this example app and remove stuff that's not needed and follow patterns common in the rest of the examples and docs.
Examples:

I wonder if it's necessary to show logging stuff here. It would simplify the example to exclude it and users advanced enough care enough for logging could probably figure it out themselves. Alternatively, move the registration to its own section or class and call it in a way to make it obvious it's optional.

Not adding Resources? resourcesOverride to AppRunner<T> ctor was an oversight on my part. If you updated that, we could change the example to use new AppRunner<Menu> instead of new AppRunner(typeof(Menu), ...

In Summary

This looks promising. Let me know if anything I mentioned wasn't clear or if you have other questions. My availability will be spotty over the next few weeks. I'll respond and availability allows.

Cheers...

drewburlingame commented 2 years ago

Hi @poison2k. Thanks for taking the time to contribute german translations and to create an example app.

First of all, please forgive any brevity or criticism that feels pointed. It's not intended that way. I'm on holidays and trying to be thorough but also quick so I fear some of my questions and suggestions may be less thoughtful than I'd like.

re: resx translations

I remember now there were some issues with the resx format but I didn't follow up on it at the time because I thought there would be more interest in the new IStringLocalizer pattern using json and not much for resx. I'm sure this is due to my not working directly with localization for quite a while.

I see that you created the CoreResources override for to use keys resx supports. I'm currently using a test class to auto-generate both the resx and the json files from the resources classes. It was quick and dirty because I didn't have time to make it more elegant and as I mentioned above, I didn't think there would be much desire for resx so I didn't want to put time into it. I think we need to find a more sustainable approach that will ensure new resource strings will be included in the resx files and in the CoreResources class and we need to find a way to make CoreResources part of the core. I don't want to be responsible for ensuring these are manually added every time. I'm sure I will forget. Any thoughts?

re: example app

One of the things I've realized with this framework is that a lot of the people who find it are hobbyists or relatively new to programming. This is why I spent a lot of time rewriting the help and adding the getting started docs. These users don't have the experience to know what is required for the example and what is extra, so it's important for the examples to be as bare bones as possible. Users will more easily understand it this way. I've had to spend a fair amount of time groking the many parts of this example and I'm familiar with CommandDotNet.

I'm wondering if we can simplify this example app and remove stuff that's not needed and follow patterns common in the rest of the examples and docs.
Examples:

I wonder if it's necessary to show logging stuff here. It would simplify the example to exclude it and users advanced enough care enough for logging could probably figure it out themselves. Alternatively, move the registration to its own section or class and call it in a way to make it obvious it's optional.

Not adding Resources? resourcesOverride to AppRunner<T> ctor was an oversight on my part. If you updated that, we could change the example to use new AppRunner<Menu> instead of new AppRunner(typeof(Menu), ...

In Summary

This looks promising. Let me know if anything I mentioned wasn't clear or if you have other questions. My availability will be spotty over the next few weeks. I'll respond and availability allows.

Cheers...

drewburlingame commented 2 years ago

Hi @poison2k, I created #439 as a way to include support for resx into CommandDotNet. This way the english resx file will be updated as new resources are added. It also uses the member name as a key instead of the value and escapes the xml. Can you look it over and see if it fits with how you'd use it?

To use the resx file with the ResourceProxy class, it should be instantiated with memberNameAsKey=true. This does not seem terribly obvious so I'll need to call it out in the docs. I'm open to better naming or another more clear approach. It may be better to allow this as an AppSettings property so devs don't have to instantiate an instance of each ResourceProxy.

poison2k commented 2 years ago

Hi @drewburlingame , first of all thanks for your work and sorry for not responding as fast as I would like to at the moment. But I am currently a bit overloaded with work.

I will look at your changes as soon as I can and it is still important to me to support you here.

What I have seen in the skim looks good for now.

The resx support is also essential for me. Our doc and localaization teams use only xml content. Therefore I can't switch to json.

I made the example application a bit more complex to show the problems I had with the framework and to present my solutions.

But you are right, as a simple example it is not good. But I am bound to certain designs e.g. Domain driven design in the use of Enterprise Business Applications. So I always have to be able to inject all dependencies and not use any static code.

Thanks again and I will look at everything in detail as soon as possible.

drewburlingame commented 1 year ago

Hi @poison2k,

I went ahead and merged the other branch. I still like the idea of your sample app. I think if you merge in master it will make the PR much smaller. I'll go ahead and close it for now and you can take your time getting the next iteration up.