Adding a global locale setter #92

Merged
merged 25 commits into from Jan 12, 2013

Projects

None yet

2 participants

@camertron
Collaborator

Whereas before you had to rely on a third-party library like I18n or FastGettext, to set the global locale for TwitterCLDR, the global locale setter allows users to easily set TwitterCldr.locale instead:

# January 7, 2013 at 4:57:04 p.m. -08:00
DateTime.now.localize.to_long_s

TwitterCldr.locale = :es

#7 de enero de 2013 16:53:55 -08:00
DateTime.now.localize.to_long_s

You can use the TwitterCldr.with_locale method to change the locale only within the scope of a block:

# January 7, 2013 at 4:57:04 p.m. -08:00
DateTime.now.localize.to_long_s

TwitterCldr.with_locale(:es) do
  #7 de enero de 2013 16:53:55 -08:00
  DateTime.now.localize.to_long_s
end

# January 7, 2013 at 4:57:04 p.m. -08:00
DateTime.now.localize.to_long_s

Finally, you can now add customized locale fallbacks to control which locale is used. The most recently added fallback is applied first:

TwitterCldr.register_locale_fallback(
  lambda { nil }  # fallback always fails / returns nil
)

TwitterCldr.locale  # :en

TwitterCldr.register_locale_fallback(
  lambda { if 1 + 1 == 2 then :ru else :pt end }
)

TwitterCldr.locale  # :ru
@KL-7 KL-7 commented on an outdated diff Jan 8, 2013
lib/twitter_cldr.rb
(supported_locale?(locale) ? locale : DEFAULT_LOCALE).to_sym
end
+ def with_locale(locale)
+ raise "Unsupported locale" unless supported_locale?(locale)
+ old_locale = @locale
+ @locale = locale
+ result = yield
+ @locale = old_locale
@KL-7
KL-7 Jan 8, 2013 Contributor

Probably @locale = old_locale should be moved to ensure section. Otherwise, something like

TwitterCldr.with_locale(:es) { raise 'asdf' }

does not switch locale back.

@KL-7 KL-7 and 1 other commented on an outdated diff Jan 8, 2013
lib/twitter_cldr.rb
(supported_locale?(locale) ? locale : DEFAULT_LOCALE).to_sym
end
+ def with_locale(locale)
+ raise "Unsupported locale" unless supported_locale?(locale)
+ old_locale = @locale
+ @locale = locale
+ result = yield
+ @locale = old_locale
+ result
+ end
+
+ def register_locale_fallback(proc_or_locale)
+ case proc_or_locale.class.to_s
@KL-7
KL-7 Jan 8, 2013 Contributor

I believe you can 'case' on an object class like that:

case proc_or_locale
  when Symbol, String, Proc
    locale_fallback << proc_or_locale
  else 
    raise "A locale fallback must be of type String, Symbol, or Proc."
  end
end
@camertron
camertron Jan 8, 2013 Collaborator

Hmm, I don't think that works in 1.9 or 1.8. Try this:

arr = []
case arr.class
  when Array then puts "Yep, it's an array"
end

The puts never executes.

@KL-7
KL-7 Jan 8, 2013 Contributor

Remove .class call.

When you do something like this:

case obj
  when choice then ...
end

Ruby executes something similar to choice === obj (note triple equals) for each alternative and selects the first one that returns true.

Since Module#=== checks if the passed object is an instance of the module itself, you can do things like:

case obj
  when KlassA then ...
  when KlassB then ...
end
@KL-7 KL-7 commented on an outdated diff Jan 8, 2013
lib/twitter_cldr.rb
@@ -86,8 +116,29 @@ def supported_locales
def supported_locale?(locale)
!!locale && supported_locales.include?(convert_locale(locale))
end
+
+ protected
+
+ def find_fallback
+ (locale_fallbacks.size - 1).downto(0).each do |i|
@KL-7
KL-7 Jan 8, 2013 Contributor

Wouldn't Array#reverse_each look nicer here?

@KL-7 KL-7 and 1 other commented on an outdated diff Jan 8, 2013
lib/twitter_cldr.rb
@@ -86,8 +116,29 @@ def supported_locales
def supported_locale?(locale)
!!locale && supported_locales.include?(convert_locale(locale))
end
+
+ protected
+
+ def find_fallback
+ (locale_fallbacks.size - 1).downto(0).each do |i|
+ result = if locale_fallbacks[i].is_a?(Proc)
+ begin
+ locale_fallbacks[i].call
+ rescue
+ nil
@KL-7
KL-7 Jan 8, 2013 Contributor

Do you think it's a good idea to hide from users exceptions inside their own lambdas?

@camertron
camertron Jan 8, 2013 Collaborator

Fallbacks are meant to either return a locale symbol, nil, or fail silently. If they raise or return nil, the system "falls back" on the next fallback or the default locale if no other fallbacks exist.

@KL-7
Contributor
KL-7 commented Jan 8, 2013

Nice work! This will close #70.

Btw, maybe you could rebase this branch on master – it'll reduce the number of commits in the PR by half :)

@camertron
Collaborator

Great suggestions as always, @KL-7! Thanks for reviewing :)

@KL-7
Contributor
KL-7 commented Jan 8, 2013

Huh, I got an email about this comment, but can't see it here after the rebase:

Fallbacks are meant to either return a locale symbol, nil, or fail silently. If they raise or return nil, the system "falls back" on the next fallback or the default locale if no other fallbacks exist.

I can see why it makes sense to suppress any exceptions inside the fallbacks in most cases, but on the other hand silently ignoring these exceptions can make it pretty difficult to track down an error inside a user-defined fallback, because user won't have any stack trace or any other sign of an exception that was raised inside the fallback. Anyway, it's up to you, just sharing an idea.

@KL-7 KL-7 commented on an outdated diff Jan 8, 2013
lib/twitter_cldr.rb
(supported_locale?(locale) ? locale : DEFAULT_LOCALE).to_sym
end
+ def with_locale(locale)
+ raise "Unsupported locale" unless supported_locale?(locale)
+ old_locale = @locale
+ @locale = locale
+ result = yield
+ ensure
+ @locale = old_locale
+ result
+ end
@KL-7
KL-7 Jan 8, 2013 Contributor

Wouldn't that go crazy if you pass an unsupported locale? In that case old_locale will be undefined, but still used in the ensure section.

I think you'll have to do something like that:

def with_locale(locale)
  raise "Unsupported locale" unless supported_locale?(locale)

  begin
    old_locale = @locale
    @locale = locale
    result = yield
  ensure
    @locale = old_locale
    result
  end
end

begin...end inside a method looks ugly, but I don't know how to handle it nicer.

@KL-7 KL-7 commented on the diff Jan 8, 2013
lib/twitter_cldr.rb
+ ensure
+ @locale = old_locale
+ result
+ end
+
+ def register_locale_fallback(proc_or_locale)
+ case proc_or_locale.class.to_s
+ when "Symbol", "String", "Proc"
+ locale_fallbacks << proc_or_locale
+ else
+ raise "A locale fallback must be of type String, Symbol, or Proc."
+ end
+ nil
+ end
+
+ def reset_locale_fallbacks
@KL-7
KL-7 Jan 8, 2013 Contributor

Look down.

@KL-7 KL-7 commented on an outdated diff Jan 8, 2013
lib/twitter_cldr.rb
+ raise "A locale fallback must be of type String, Symbol, or Proc."
+ end
+ nil
+ end
+
+ def reset_locale_fallbacks
+ locale_fallbacks.clear
+ TwitterCldr.register_locale_fallback(lambda { I18n.locale if defined?(I18n) && I18n.respond_to?(:locale) })
+ TwitterCldr.register_locale_fallback(lambda { FastGettext.locale if defined?(FastGettext) && FastGettext.respond_to?(:locale) })
+ end
+
+ def locale_fallbacks
+ @locale_fallbacks ||= []
+ end
+
+ def reset_locale_fallbacks
@KL-7
KL-7 Jan 8, 2013 Contributor

Look up.

@KL-7 KL-7 commented on the diff Jan 8, 2013
lib/twitter_cldr.rb
+ locale_fallbacks.clear
+ TwitterCldr.register_locale_fallback(lambda { I18n.locale if defined?(I18n) && I18n.respond_to?(:locale) })
+ TwitterCldr.register_locale_fallback(lambda { FastGettext.locale if defined?(FastGettext) && FastGettext.respond_to?(:locale) })
+ end
+
+ def locale_fallbacks
+ @locale_fallbacks ||= []
+ end
+
+ def reset_locale_fallbacks
+ locale_fallbacks.clear
+ TwitterCldr.register_locale_fallback(lambda { I18n.locale if defined?(I18n) && I18n.respond_to?(:locale) })
+ TwitterCldr.register_locale_fallback(lambda { FastGettext.locale if defined?(FastGettext) && FastGettext.respond_to?(:locale) })
+ end
+
+ def locale_fallbacks
@KL-7
KL-7 Jan 8, 2013 Contributor

Another method duplicate. Looks like git failed to merge correctly.

@camertron
camertron Jan 8, 2013 Collaborator

Woops! Nice catch, that's my fault during the rebase :)

@KL-7 KL-7 commented on the diff Jan 8, 2013
lib/twitter_cldr.rb
+ old_locale = @locale
+ @locale = locale
+ result = yield
+ ensure
+ @locale = old_locale
+ result
+ end
+
+ def register_locale_fallback(proc_or_locale)
+ case proc_or_locale.class.to_s
+ when "Symbol", "String", "Proc"
+ locale_fallbacks << proc_or_locale
+ else
+ raise "A locale fallback must be of type String, Symbol, or Proc."
+ end
+ nil
@KL-7
KL-7 Jan 8, 2013 Contributor

Just curious, why an explicit nil here?

@camertron
camertron Jan 8, 2013 Collaborator

Otherwise it returns the fallback proc, which isn't all that useful.

@camertron
Collaborator

Thanks again :) Looks like the comment you couldn't see is being hidden by a new github feature. Scroll up a bit and look for "KL-7 discussed an outdated diff 11 hours ago". There should be a link there that will show you the comment history.

@KL-7 KL-7 and 1 other commented on an outdated diff Jan 9, 2013
lib/twitter_cldr.rb
def resources
@resources ||= TwitterCldr::Resources::Loader.new
end
- def get_locale
- if defined?(FastGettext)
- locale = FastGettext.locale
- locale = DEFAULT_LOCALE if locale.to_s.empty?
- else
- locale = DEFAULT_LOCALE
- end
+ def locale=(new_locale)
@KL-7
KL-7 Jan 9, 2013 Contributor

Is that necessary with the attr_accessor above? Besides, since a custom getter is defined below, maybe it should be just attr_writer.

@camertron
camertron Jan 12, 2013 Collaborator

Good call.

@jasonkb jasonkb referenced this pull request Jan 11, 2013
Merged

Territories #93

@camertron camertron merged commit 1b959dc into master Jan 12, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment