Skip to content

thor fixups and component/locale standardization#121

Merged
movermeyer merged 3 commits intomainfrom
movermeyer/thor_fixups
Apr 19, 2022
Merged

thor fixups and component/locale standardization#121
movermeyer merged 3 commits intomainfrom
movermeyer/thor_fixups

Conversation

@movermeyer
Copy link
Copy Markdown
Collaborator

@movermeyer movermeyer commented Apr 18, 2022

What are you trying to accomplish?

thor has changed the way that you specify options. I've refreshed our use of thor to match.
As part of this, I standardize the names used to refer to components and locales on the command line (and internally in the codebase).

What approach did you choose and why?

I arbitrarily picked the capital-case of the module names.
Before, you could specify CountryCodes, country-codes, or country_codes and all would work. There was no need to support this flexibility, so I picked one style to enforce. Now, only CountryCodes is valid.

I also arbitrarily picked the ruby-i18n version of locales codes (en-US instead of en_US)
Before, it was expecting CLDR versions of locale codes (e.g., en_US).
Standardizing on this format allowed me to remove some unnecessary calls to to_i18n.

Since type: array values don't yet get enum validation from thor, this validation of the inputs had to be done in ruby-cldr.

What should reviewers focus on?

🤷

There is only one change to the output of thor cldr:export, which was incorrectly outputting the wrong locale code:

diff -r before/pt-PT/plurals.rb after/pt-PT/plurals.rb
1c1
< { :'pt_PT' => { :i18n => { :plural => { :keys => [:one, :other], :rule => lambda { |n| n = n.respond_to?(:abs) ? n.abs : ((m = n.to_s)[0] == "-" ? m[1,m.length] : m); (n.to_i == 1 && ((v = n.to_s.split(".")[1]) ? v.length : 0) == 0) ? :one : :other } } } } }
\ No newline at end of file
---
> { :'pt-PT' => { :i18n => { :plural => { :keys => [:one, :other], :rule => lambda { |n| n = n.respond_to?(:abs) ? n.abs : ((m = n.to_s)[0] == "-" ? m[1,m.length] : m); (n.to_i == 1 && ((v = n.to_s.split(".")[1]) ? v.length : 0) == 0) ? :one : :other } } } } }
\ No newline at end of file

The impact of these changes

  • Users will have to specify the capital-case versions of the component names when they use thor cldr:export.
  • Users will have to specify the ruby-cldr version of the locale codes when they use thor cldr:export.
  • When users use thor help cldr:export, the printout is much nicer, and relies on defaults that come from the constants actually used by the code.

Testing

thor cldr:export

See the new help docs with:

thor help cldr:download
thor help cldr:export

@movermeyer movermeyer force-pushed the movermeyer/thor_fixups branch 6 times, most recently from cb08daa to 64ff691 Compare April 18, 2022 20:35
@movermeyer movermeyer changed the title thor fixups and component name standardization thor fixups and component/locale standardization Apr 18, 2022
@movermeyer movermeyer force-pushed the movermeyer/thor_fixups branch 4 times, most recently from 5c9ecdb to 85206fd Compare April 19, 2022 12:47
@movermeyer movermeyer marked this pull request as ready for review April 19, 2022 13:04
Comment thread lib/cldr/thor.rb
`method_options` has been renamed `options`

Load the defaults from the actual code
Before, it wasn't clear how to specify a name that was multiple words.
You could specify `CountryCodes`, `country-codes`, or `country_codes` and all would work.
Standardize on capital case arbitrarily.

Also, throughout the code use `Symbol`s for the component names.
Before, it wasn't clear whether a user should provide CLDR-style locales (e.g., `en_US`) or `ruby-i18n`-style locales (e.g., `en-US`).
@movermeyer movermeyer force-pushed the movermeyer/thor_fixups branch from 85206fd to 4aa7b47 Compare April 19, 2022 16:49
@movermeyer movermeyer merged commit b66849b into main Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants