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

ignore_lines keys are not stringified from YAML #206

Closed
markedmondson opened this issue Nov 1, 2016 · 9 comments
Closed

ignore_lines keys are not stringified from YAML #206

markedmondson opened this issue Nov 1, 2016 · 9 comments
Labels
Milestone

Comments

@markedmondson
Copy link
Contributor

markedmondson commented Nov 1, 2016

When using the YML format for scanner configuration like:

  scanners:
  - - "::I18n::Tasks::Scanners::PatternWithScopeScanner"
    - exclude:
      - "*.rb"
      ignore_lines:
        opal: ^\s*#(?!\si18n-tasks-use)
        haml: ^\s*-\\s*#(?!\si18n-tasks-use)
        coffee: ^\s*#(?!\si18n-tasks-use)

The ignore_lines keys are not stringified, therefore when they hit the pattern scanner, they're not found.

The fix is as simple as: markedmondson@17925c4

@glebm
Copy link
Owner

glebm commented Nov 1, 2016

The keys in the example you've given are already strings:

> require 'yaml'; YAML.load 'x: 1'
 => {"x"=>1}

> YAML.load <<-'YAML'
scanners:
- - "::I18n::Tasks::Scanners::PatternWithScopeScanner"
  - exclude:
    - "*.rb"
    ignore_lines:
      opal: ^\s*#(?!\si18n-tasks-use)
      haml: ^\s*-\\s*#(?!\si18n-tasks-use)
      coffee: ^\s*#(?!\si18n-tasks-use)
YAML
 => {"scanners"=>[["::I18n::Tasks::Scanners::PatternWithScopeScanner", {"exclude"=>["*.rb"], "ignore_lines"=>{"opal"=>"^\\s*#(?!\\si18n-tasks-use)", "haml"=>"^\\s*-\\\\s*#(?!\\si18n-tasks-use)", "coffee"=>"^\\s*#(?!\\si18n-tasks-use)"}}]]}
> YAML
 => Psych

@markedmondson
Copy link
Contributor Author

I thought the same thing so had to double check. A test case could probably be written around this but here's an awkward gif to show it.

i18n

@glebm
Copy link
Owner

glebm commented Nov 1, 2016

Ah, perhaps it's because of this:

conf = (config[:search] || {}).deep_symbolize_keys

@markedmondson
Copy link
Contributor Author

Aha, good spot, always easier for someone familiar with the codebase to find the potential issue quicker :)

Do you want me to try and add a PR? Would you prefer a solution around the config or the initializer?

@glebm
Copy link
Owner

glebm commented Nov 1, 2016

Your current solution (ext.to_s) seems good to me, please send a PR! Thanks

@glebm
Copy link
Owner

glebm commented Nov 1, 2016

Actually, scratch that, I'll fix it in a way that works for scanners other than pattern scanners

@glebm glebm closed this as completed Nov 1, 2016
@glebm glebm reopened this Nov 1, 2016
@glebm
Copy link
Owner

glebm commented Nov 1, 2016

Ah, other scanners don't have that setting

glebm added a commit that referenced this issue Nov 1, 2016
@glebm glebm closed this as completed Nov 1, 2016
@glebm glebm added this to the v0.9.6 milestone Nov 1, 2016
@glebm glebm added the bug label Nov 1, 2016
@markedmondson
Copy link
Contributor Author

Great - thanks for the prompt response!

@glebm
Copy link
Owner

glebm commented Nov 1, 2016

Released v0.9.6!

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