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

Using scope in rails views is broken #224

Closed
josepjaume opened this issue Jan 31, 2017 · 13 comments
Closed

Using scope in rails views is broken #224

josepjaume opened this issue Jan 31, 2017 · 13 comments
Labels
Milestone

Comments

@josepjaume
Copy link

First of all, thanks for the good amount of work you've put in this gem. It works like a charm! It's definitely made my life easier :)

Since the 0.9.9 update, I'm experiencing issues when using scope in translations in my rails views.

Example:

# app/views/layouts/application.html.erb
<%= t(".title", scope: "specific") %>

This resolves to specific.layouts.application.title but while 0.9.8 works fine and picks it up, 0.9.9 doesn't detect it and my locales now show up as unused.

Is there anything I should take into account?

@josepjaume josepjaume changed the title Using scope in views is broken Using scope in rails views is broken Jan 31, 2017
@glebm glebm added the bug label Jan 31, 2017
@glebm glebm closed this as completed in bee1c61 Jan 31, 2017
@glebm glebm added this to the v0.9.10 milestone Jan 31, 2017
@josepjaume
Copy link
Author

Wow, that was fast! 💃

I don't know if it helps, but it looks like replacing it by: <%= t(".title", scope: ["specific"]) %> (using an array as a scope) makes it work.

@glebm
Copy link
Owner

glebm commented Jan 31, 2017

Good catch! Fixed and released v0.9.10 🚢

@josepjaume
Copy link
Author

Hi @glebm! Thanks for such a quick fix!

There's still some scenarios where this is failing though. For example, I have this in my views:

<%= t "actions.manage", scope: "app.admin" %>

This should resolve to app.admin.actions.manage but it shows up as unused in my application.

I tried to run the test suite and fix it but couldn't manage to.

glebm added a commit that referenced this issue Feb 2, 2017
@glebm
Copy link
Owner

glebm commented Feb 2, 2017

@josepjaume I tried to reproduce it but I can't. Are you sure you are on v0.9.10? What's in your config? What is the exact line with the usage?

@josepjaume
Copy link
Author

josepjaume commented Feb 2, 2017

glebm added a commit that referenced this issue Feb 2, 2017
glebm added a commit that referenced this issue Feb 2, 2017
@glebm
Copy link
Owner

glebm commented Feb 2, 2017

Fix released in v0.9.11 🚢

@glebm glebm modified the milestones: v0.9.11, v0.9.10 Feb 2, 2017
@josepjaume
Copy link
Author

Awesome news! Thanks!! <3

@josepjaume
Copy link
Author

Hi again! I'm sorry to be such a hassle, but there's still some instances where this is failing, although most of them got fixed.

This one is also reporting as missing:

https://github.com/AjuntamentdeBarcelona/decidim/blob/master/decidim-admin/app/views/decidim/admin/participatory_process_steps/index.html.erb#L6

We can work against a branch if you'd like, and keep iterating until they're all fixed :)

I tried to do this myself but to be honest I'm having a hard time understanding the internals :S

@glebm
Copy link
Owner

glebm commented Feb 2, 2017

The nested calls are difficult to get to work with the current regexp parser that's used in the views. The best thing you can do is move this to a helper, then the AST parser will parse them correctly.

@josepjaume
Copy link
Author

ok @glebm, thanks so much!

glebm added a commit that referenced this issue Feb 12, 2017
@glebm
Copy link
Owner

glebm commented Feb 12, 2017

@josepjaume I've managed to fix the nested calls case as well but any further work should be done towards adding AST scanner support for various view formats. The regex hack is at its limits at this point.

@josepjaume
Copy link
Author

Thanks @glebm! I can see how using regexp has its limits - been there with https://github.com/codegram/spinach and ended up writing a language parser.

Let me confirm it works and maybe you can release a new minor?

@josepjaume
Copy link
Author

Confirmed! It chased all my use cases 💪 . Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants