Separate core_ext #88
Conversation
lib/humanize/module.rb
Outdated
| @@ -117,15 +118,3 @@ def initialize | |||
| end | |||
| end | |||
| end | |||
|
|
|||
There was a problem hiding this comment.
The big change is moving this out to core_ext.rb.
lib/humanize/module.rb
Outdated
|
|
||
| module Humanize | ||
| SPACE = ' '.freeze | ||
| EMPTY = ''.freeze |
There was a problem hiding this comment.
No need for this trick since # frozen_string_literal: true above. Other string literals benefit as well.
| # Big numbers are big: http://wiki.answers.com/Q/What_number_is_after_vigintillion&src=ansTT | ||
|
|
||
| def humanize(locale: Humanize.config.default_locale, | ||
| class << self | ||
| def format(number, |
There was a problem hiding this comment.
Note that this changed from humanize with (where self was mixed in as the implicit number) to format with an explicit number arg.
| Humanize.config.default_locale == locale | ||
| end | ||
|
|
||
| def process_decimals(number, locale_class, locale, parts, decimals_as, spacer) |
There was a problem hiding this comment.
Note that this previously polluted the mixin namespace (it became a public method of Integer, Float, and BigDecimal). Now it's pushed down into the module...where it has an explicit number arg.
| @@ -0,0 +1,20 @@ | |||
| module Humanize | |||
| def humanize(locale: Humanize.config.default_locale, | |||
| decimals_as: Humanize.config.decimals_as) | |||
There was a problem hiding this comment.
This is the glue code that converts from implicit self arg to explicit self passed as the number arg.
|
Hey, thanks for this! I think it's a great PR. Would you mind looking into the build failures here? |
| when :tr | ||
| [Humanize::Tr, ' '] | ||
| when :jp | ||
| [Humanize::Jp, ''] |
There was a problem hiding this comment.
This is missing the recent zh-TW addition.
There was a problem hiding this comment.
@radar Oops, I didn't mean to make any changes to this locale list. I just looked over in master and didn't find any reference to zh. Can you point me to it?
And I'm looking at the build failures.
There was a problem hiding this comment.
The build failures were rubocop suggestions. All addressed here: 0abcfc9
There was a problem hiding this comment.
Ok, merged #87. So can we now get that in here? I think it's just the two lines for the when + the config.
There was a problem hiding this comment.
Ok, I've merged latest master containing that PR into this branch.
|
@radar BTW I think the |
|
Fine by me regarding FSL. Rubocop is being a grump. Would you mind fixing that up? I can't push to your branch due to permission restrictions. |
Ah, I'd made this change suppress that rubucop failure but lost it in the merge-up. Merged back here: a347bbb Also, I found I'd missed this recursive call that was depending on the core And finally, I updated the README to document the new functional usage: 32bd066 |
|
All checks out to me. Thanks very much! ❤️ |
NOTE
Best reviewed by skipping the first commit that moved
humanize.rbtohumanize/module.rband ignoring whitespace: https://github.com/radar/humanize/pull/88/files/a84b371db91fe64c184215df1b147f7a70c8912d..500e185faeb990f4e801d07ec6ac20643249f540Changes
The
Humanizegem would formerly always duck punch itself into theInteger,Float, andBigDecimalclasses.This PR takes inspiration from how
active_supportdoes core extensions. The core extensions are now optional with this require:And that is required automatically if you
to match previous behavior. But new code has the option to set
require: falsein the Gemfile and just doThen rather than doing
it can instead use the pure functional form (which is always available):
This is useful for code bases that want don't want to duck punch into Ruby core classes.