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.

Sign up for GitHub

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

Jump to bottom

HelpProvider supports localisation #1349

Merged
merged 7 commits into from
Nov 10, 2023
Merged

HelpProvider supports localisation #1349

merged 7 commits into from
Nov 10, 2023

Conversation

FrankRay78
Copy link
Contributor

@FrankRay78 FrankRay78 commented Oct 30, 2023 •
edited
Loading

  • Extracted hardcoded strings into a default resource file
  • Included localised strings for French
  • Wrote unit tests for both

image

@FrankRay78
HelpProvider supports localisation
5d14a06 
+ included localised strings for French
@FrankRay78 FrankRay78 self-assigned this Oct 30, 2023
@FrankRay78 FrankRay78 added the area-CLI Command-Line Interface label Oct 30, 2023
@FrankRay78 FrankRay78 linked an issue Oct 30, 2023 that may be closed by this pull request
Export strings to .resx file to allow localization #1213
Closed
@FrankRay78 FrankRay78 added this to the 0.48 milestone Oct 30, 2023
@FrankRay78 FrankRay78 requested a review from a team October 30, 2023 16:18
@FrankRay78 FrankRay78 mentioned this pull request Oct 30, 2023
Closed
@FrankRay78
Copy link
Contributor Author

@patriksvensson - I've removed the need to set the culture on the current thread by eliminating statics, and so ensured that no funny business will happen when parallel running tests.

nils-a
nils-a approved these changes Nov 3, 2023
View reviewed changes
Copy link
Contributor

@nils-a nils-a left a comment

Choose a reason for hiding this comment

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

Two minor changes, but generally LGTM!

test/Spectre.Console.Cli.Tests/Unit/CommandAppTests.Help.cs Outdated Show resolved Hide resolved
test/Spectre.Console.Cli.Tests/Unit/CommandAppTests.Help.cs Outdated Show resolved Hide resolved
patriksvensson
patriksvensson requested changes Nov 3, 2023
View reviewed changes
Copy link
Contributor

@patriksvensson patriksvensson left a comment

Choose a reason for hiding this comment

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

A minor thing that I think we should dicuss.

src/Spectre.Console.Cli/ConfiguratorExtensions.cs Outdated Show resolved Hide resolved
@FrankRay78
Copy link
Contributor Author

@patriksvensson , @nils-a

FYI. If you provide me with the localised strings for the following in your own language, along with the culture, I can see about pushing these into this branch ahead of merging:

Prints help information
Prints version information
DESCRIPTION
USAGE
OPTIONS
COMMAND
COMMANDS
EXAMPLES
ARGUMENTS
DEFAULT

@patriksvensson
Copy link
Contributor

patriksvensson commented Nov 4, 2023 •
edited
Loading

Sure!

Prints help information
Skriver ut hjälpinformation

Prints version information
Skriver ut versionsnummer

DESCRIPTION
BESKRIVNING

USAGE
ANVÄNDING

OPTIONS
VAL

COMMAND
KOMMANDO

COMMANDS
KOMMANDON

EXAMPLES
EXEMPEL

ARGUMENTS
ARGUMENT

DEFAULT
STANDARD

@nils-a
Copy link
Contributor

nils-a commented Nov 4, 2023

en de
Prints help information Zeigt Hilfe an
Prints version information Zeigt Versionsinformationen an
DESCRIPTION BESCHREIBUNG
USAGE VERWENDUNG
OPTIONS OPTIONEN
COMMAND KOMMANDO
COMMANDS KOMMANDOS
EXAMPLES BEISPIELE
ARGUMENTS ARGUMENTE
DEFAULT STANDARDWERT

@0xced
Copy link
Contributor

0xced commented Nov 5, 2023

For French I'd go slightly less verbose.

en fr
Prints help information Affiche l'aide
Prints version information Affiche la version
USAGE UTILISATION (was missing in the screenshot)

@FrankRay78
Copy link
Contributor Author

@patriksvensson, @nils-a, @0xced - I believe the latest push addresses your excellent review comments, as well as updating the French translations (as suggested) and adding Swedish and German localisations.

FrankRay78
FrankRay78 commented Nov 5, 2023
View reviewed changes
Copy link
Contributor Author

@FrankRay78 FrankRay78 left a comment

Choose a reason for hiding this comment

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

I'm happy with the changes.

@FrankRay78
Copy link
Contributor Author

@0xced Addressed all further review comments in the latest push.

0xced added a commit to 0xced/spectre.console that referenced this pull request Nov 9, 2023
@0xced
Allow passing a nullable style in DefaultValueStyle() and ChoicesStyle()
ac876fa 
This will allow to slightly simplify the implementation of spectreconsole#1210

See also related discussion on spectreconsole#1349 (comment)
@0xced 0xced mentioned this pull request Nov 9, 2023
Merged
patriksvensson pushed a commit that referenced this pull request Nov 10, 2023
@0xced @patriksvensson
Allow passing a nullable style in DefaultValueStyle() and ChoicesStyle()
4219bbb 
This will allow to slightly simplify the implementation of #1210

See also related discussion on #1349 (comment)
@patriksvensson patriksvensson merged commit 250b1f4 into spectreconsole:main Nov 10, 2023
5 checks passed
@FrankRay78 FrankRay78 deleted the 1213-Export-strings-to-.resx-file-to-allow-localization branch January 6, 2024 10:15
@Tolitech
Copy link

Hello @patriksvensson,
I would like the HelpProvider to support some additional languages, as listed below.

image

Would it be possible for me to contribute by creating the Resource files that are not yet supported?

@FrankRay78
Copy link
Contributor Author

Yes please @Tolitech! I can review and merge.

@Tolitech
Copy link

Hello, @FrankRay78.
The changes have already been made in my FORK.
I have created 6 new resource files.
I have tested it locally and will submit the Pull Request soon.

@Tolitech Tolitech mentioned this pull request Dec 19, 2024
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nils-a nils-a nils-a approved these changes

@0xced 0xced 0xced left review comments

@patriksvensson patriksvensson patriksvensson approved these changes

Assignees

@FrankRay78 FrankRay78

Labels
area-CLI Command-Line Interface
Projects
Spectre Console
Status: No status
Milestone
0.48
Development

Successfully merging this pull request may close these issues.

Export strings to .resx file to allow localization
5 participants
@FrankRay78 @patriksvensson @nils-a @0xced @Tolitech