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.

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

Interpolate now works for an array #282

Closed
wants to merge 6 commits into from
Closed

Conversation

Bartuz
Copy link
Contributor

@Bartuz Bartuz commented Aug 25, 2014

Fixes #281

Credits for @ahmadsherif, thank you Ahmad!

@Bartuz Bartuz changed the title Interpolate now works for array Interpolate now works for an array Aug 25, 2014
@tigrish
Copy link
Collaborator

tigrish commented Aug 29, 2014

Looks good! Happy to merge when I get back from holidays unless anybody else wants to look into this first?

On 26 Aug 2014, at 00:37, Filip Bartuzi notifications@github.com wrote:

Fixes #281

Credits for @ahmadsherif, thank you Ahmad!

You can merge this Pull Request by running

git pull https://github.com/Bartuz/i18n master
Or view, comment on, or merge it at:

#282

Commit Summary

Interpolate now works for array
File Changes

M lib/i18n/backend/base.rb (2)
M lib/i18n/tests/interpolation.rb (6)
M lib/i18n/tests/lookup.rb (6)
Patch Links:

https://github.com/svenfuchs/i18n/pull/282.patch
https://github.com/svenfuchs/i18n/pull/282.diff

Reply to this email directly or view it on GitHub.

@Bartuz
Copy link
Contributor Author

Bartuz commented Sep 3, 2014

Any updates?

@seabornlee
Copy link

What happened? Still not support?

@Bartuz
Copy link
Contributor Author

Bartuz commented Jan 24, 2015

Code works but it's not merged yet. If you want to use it is I would recommend you reading this: http://stackoverflow.com/a/25484080

Wysłane z iPhone'a

Dnia 24 sty 2015 o godz. 15:05 Seaborn Lee notifications@github.com napisał(a):

What happened? Still not support?


Reply to this email directly or view it on GitHub.

@seabornlee
Copy link

@Bartuz Thanks!

@Bartuz
Copy link
Contributor Author

Bartuz commented Mar 9, 2015

Still not any news?

@PikachuEXE
Copy link
Contributor

@tigrish Please take a look when you have time :)

@zimkies
Copy link

zimkies commented Aug 18, 2015

@tigrish any word on this? Seems like a really useful feature without any downsides

@Bartuz
Copy link
Contributor Author

Bartuz commented Aug 19, 2015

It's been almost a year since I opened pull request 😞

@carlosantoniodasilva
Copy link
Member

Ya, sorry about that, I'm planning to get back to i18n in the upcoming days/weeks, things have been a bit crazy around here :), I'll keep this one on my radar and check back soon. Thanks everyone!

@jhaagmans
Copy link

Do mind that @tigrish has once said he wished "that i18n didn't support it as a data type in the first place". I don't fully agree, but this might have something to do with not merging the initial PR:
#242

@@ -150,6 +150,8 @@ def pluralize(locale, entry, count)
def interpolate(locale, string, values = {})
if string.is_a?(::String) && !values.empty?
I18n.interpolate(string, values)
elsif string.is_a?(::Array) && !values.empty?
string.map { |el| interpolate(locale, el, values) }
else
string
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO this whole method should be rewritten to make it clearer and cut down repetition:

def interpolate(locale, subject, values = {})
  return subject if values.empty?

  case subject
  when ::String then I18n.interpolate(subject, values)
  when ::Array then subject.map { |element| interpolate(locale, element, values) }
  else
    subject
  end
end

Bartuz added a commit to Bartuz/i18n that referenced this pull request Sep 12, 2015
It covers full use case now.
Thankfully to ruby-i18n#282 (comment)
@Bartuz
Copy link
Contributor Author

Bartuz commented Sep 12, 2015

Passed on my machine... - ruby 2.1.5p273 (2014-11-13 revision 48405) [x86_64-darwin14.0]

#324

@PikachuEXE
Copy link
Contributor

Travis matrix buggy :S

@Bartuz
Copy link
Contributor Author

Bartuz commented Sep 12, 2015

But something is wrong anyway: #324

@cavi21
Copy link

cavi21 commented May 30, 2016

There is no news about this right?
If there is no plans to include this it would be great to know

Thanks to all

@Bartuz Bartuz closed this Nov 2, 2016
@Bartuz Bartuz reopened this Nov 2, 2016
@radar
Copy link
Collaborator

radar commented Nov 15, 2016

Looks fine to me. Just going to wait on the build and then once that's showing green I'll merge this.

@radar radar added this to the 0.8.0 milestone Nov 15, 2016
@Bartuz
Copy link
Contributor Author

Bartuz commented Nov 19, 2016

Installing json 1.8.2 with native extensions
Gem::Ext::BuildError: ERROR: Failed to build gem native extension.

on Rails 4.2.x travis build.

is it possible my commits caused it? I doubt it...

@radar
Copy link
Collaborator

radar commented Nov 20, 2016

Try a "bundle update json" and then committing Gemfile.lock and see if that fixes it

On 19 Nov. 2016, at 20:58, Filip Bartuzi notifications@github.com wrote:

Installing json 1.8.2 with native extensions
Gem::Ext::BuildError: ERROR: Failed to build gem native extension.
on Rails 4.2.x travis build.

is it possible my commits caused it? I doubt it...


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@shaneog
Copy link

shaneog commented Dec 4, 2016

Anxiously awaiting this 👍

@florrain
Copy link

+1

@AM-R
Copy link

AM-R commented Nov 18, 2017

Damned, why this fix is still not applied?

@radar
Copy link
Collaborator

radar commented Nov 18, 2017 via email

@radar
Copy link
Collaborator

radar commented Nov 18, 2017

@AM-R: perhaps you could test the fix in #395 for me and tell me if works? That would certainly expedite the process of getting this fixed.

@radar
Copy link
Collaborator

radar commented Nov 19, 2017

#395 has been merged. Please try the latest i18n master.

@radar radar closed this Nov 19, 2017
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.

Interpolation in I18n array