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

Silently ignore the parent #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nvanoorschot
Copy link

@nvanoorschot nvanoorschot commented May 19, 2017

Closes #82

First I order the translations so that a parent element would be the last result. This makes sure that when checking the key of the first result it is not parents value that is returned.

In the build_translation_hash_by_key method I simply return an empty Hash when the parent is evaluated.

@timfjord
Copy link
Collaborator

timfjord commented May 24, 2017

Hey @nvanoorschot
I've had look at you PR.
In general I think it is a good idea to provide either better error message or some default behaviour and not raise some strange error.
I will try to do more deeper review later

Copy link
Collaborator

@timfjord timfjord left a comment

Choose a reason for hiding this comment

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

Could you please also add specs for this new functionality?

@@ -49,29 +49,28 @@ def store_translations(locale, data, options = {})

def lookup(locale, key, scope = [], options = {})
key = normalize_flat_keys(locale, key, scope, options[:separator])
result = Translation.locale(locale).lookup(key)
results = Translation.locale(locale).lookup(key).order(key: :desc)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need ordering here?
It will just make query more complicated but I don't see any benefits of using it

iterator[key] = translation_nested_keys[index + 1] ? {} : translation.value
iterator[key]
translation_nested_keys = translation.key.slice(chop_range)
return {} if translation_nested_keys.nil?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use unless translation_nested_keys instead of if translation_nested_keys.nil?

@bacrossland
Copy link

👍 for this PR

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.

None yet

3 participants