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

Fix autocorrect for SpecialGlobalVars #565

Merged
merged 1 commit into from
Oct 16, 2013

Conversation

nevir
Copy link
Contributor

@nevir nevir commented Oct 12, 2013

It was replacing $! with "$ERROR_INFO from English library", for example.

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 12, 2013

Nice catch. You can simplify the code a lot by simply checking if a constant is defined? - if it's not the English library should be included. That will remove the need for the use of a set the complex message generation logic. You should also update the changelog.

@nevir
Copy link
Contributor Author

nevir commented Oct 16, 2013

I'm not sure I follow:

  • I can't use defined? as a proxy for the blacklist because rubocop requires English.
  • Also, since English is just a bunch of alias calls, there doesn't appear to be a consistent way to determine whether or not a global comes from it at runtime (short of parsing the file...) :(

@nevir
Copy link
Contributor Author

nevir commented Oct 16, 2013

Updated the changelog

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 16, 2013

Yep, you're right. I didn't think this through.

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 16, 2013

You'll have to rebase this again.

@nevir
Copy link
Contributor Author

nevir commented Oct 16, 2013

good to go!

bbatsov added a commit that referenced this pull request Oct 16, 2013
Fix autocorrect for SpecialGlobalVars
@bbatsov bbatsov merged commit 9a63df6 into rubocop:master Oct 16, 2013
@nevir nevir deleted the fix-special-globals branch October 16, 2013 20:34
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.

2 participants