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

i18n-tasks missing resulting in false positive when I18n.t is called from a helper #191

Closed
lucascaton opened this issue Mar 5, 2016 · 6 comments
Milestone

Comments

@lucascaton
Copy link

Hi there, I think have found a bug :-)

With a simple Rails app:

# app/helpers/pages_helper.rb
module PagesHelper
  def locale_test
    t '.something'
  end
end
<%# app/views/pages/index.html.erb %>
<h1><%= locale_test %></h1>
# config/locales/en.yml
en:
  pages:
    index:
      something: 'Something!!!'

It works:
image

But:

$ i18n-tasks missing

Missing translations (1) | i18n-tasks v0.9.4
+--------+------------------------------------+----------------------------------+
| Locale | Key                                | Value in other locales or source |
+--------+------------------------------------+----------------------------------+
|  all   | pages_helper.locale_test.something | app/helpers/pages_helper.rb:3    |
+--------+------------------------------------+----------------------------------+
@lucascaton
Copy link
Author

I tried to add # i18n-tasks-use, but it didn't work as well:

# app/helpers/pages_helper.rb
module PagesHelper
  def locale_test
    # i18n-tasks-use t('pages.index.something')
    t '.something'
  end
end

(running $ i18n-tasks missing I get the same false positive)

@glebm
Copy link
Owner

glebm commented Mar 5, 2016

Unfortunately, this is not a bug but a limitation of i18n-tasks. The static analysis always resolves relative keys based on the filename (+ the enclosing method name in case of .rb files).

There are ways to work around this. First, make i18n-tasks ignore this key:

ignore_missing:
# Ignore this relative key defined in a helper, as it is resolved differently
# depending on where the helper is called from. We handle these separately in ...
- pages_helper.locale_test.something

Then, there are two options:

A) Add an i18n-tasks-use hint at every call sites, e.g.:

<h1>
  <%# i18n-tasks-use t('pages.index.something') %>
  <%= locale_test %>
</h1>

This is a good option if there are few call sites.

B) Add a custom scanner for calls to locale_test:

# config/i18n-tasks/something_scanner.rb
require 'i18n/tasks/scanners/file_scanner'
class SomethingScanner < I18n::Tasks::Scanners::FileScanner
  include I18n::Tasks::Scanners::RelativeKeys
  include I18n::Tasks::Scanners::OccurrenceFromPosition

  # @return [Array<[absolute key, Results::Occurrence]>]
  def scan_file(path)
    text = read_file(path)
    text.scan(/^\s*<%=\s*locale_test\b/).map do |_match|
      occurrence = occurrence_from_position(
          path, text, Regexp.last_match.offset(0).first)
      [absolute_key('.something', path), occurrence]
    end
  end
end
I18n::Tasks.add_scanner 'SomethingScanner', only: %w(*.html.erb)
# config/i18n-tasks.yml.erb
<% require './config/i18n-tasks/something_scanner.rb' %>

This is a better option if there are many call sites.

@lucascaton
Copy link
Author

Hi @glebm, thanks for replying so quickly.

Wouldn't it be worth adding somehow this Scanner class to the gem codebase?

@glebm
Copy link
Owner

glebm commented Mar 6, 2016

Perhaps it would make sense to add a scanner that maps patterns to keys, so it can be configured with something like:

<% I18n::Tasks.add_scanner, 'I18n::Tasks::Scanners::PatternMapper', only: %w(*.html.erb),
  patterns: [[/^\s*<%=\s*locale_test\b/, '.something']] %>

glebm added a commit that referenced this issue Mar 6, 2016
@glebm glebm added this to the v0.9.5 milestone Mar 6, 2016
glebm added a commit that referenced this issue Mar 6, 2016
@glebm
Copy link
Owner

glebm commented Mar 6, 2016

Released in v0.9.5. Simply add the key to ignore_missing and add this to the config:

<% I18n::Tasks.add_scanner 'I18n::Tasks::Scanners::PatternMapper',
                            only: %w(*.html.erb),
                            patterns: [['<%=\s*locale_test\b', '.something']] %>

@glebm glebm closed this as completed Mar 6, 2016
@lucascaton
Copy link
Author

Thanks @glebm!

bartimaeus added a commit to bartimaeus/i18n-tasks that referenced this issue Apr 26, 2016
* upstream/master: (28 commits)
  Fix translation of plural HTML keys glebm#193
  Default config: exclude app/assets/videos glebm#194
  fix template config
  Bump to 0.9.5
  Pattern mapper documentation glebm#191
  Pattern mapper implementation glebm#191
  Add missing keys with `nil` value glebm#170
  Avoid changing the locale after running i18n-tasks
  changelog: 187 still not fixed
  Bump to 0.9.4
  Reference resolution reporting
  Document used_keys include_raw_references param
  Remove redundant filtering from unused_keys
  Improve reporting for unused keys
  Possible fix to glebm#187
  Improve reporting for missing ref keys
  minor refactors
  Bump to 0.9.3
  Readme: reference keys
  Copy reference keys as is in add/translate-missing
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants