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

Silence warnings being emitted when running tests #4134

Merged
merged 2 commits into from
May 21, 2018
Merged

Conversation

Alhadis
Copy link
Collaborator

@Alhadis Alhadis commented May 16, 2018

I saw these warnings as I was running tests for #4133:

lib/linguist/heuristics.rb:414: warning: character class has duplicated range: /[A-Z\.][\w\d\.]*:{/
lib/linguist/language.rb:504: warning: assigned but unused variable - filenames

They're trivial but still unrelated to what #4133 is doing, so I siphoned these changes off to a separate PR. :-) Explanation of changes:

warning: character class has duplicated range: /[A-Z\.][\w\d\.]*:{/

That's happening because \w already covers the character ranges that \d does. Using them together in a bracketed character class is therefore redundant.

I also removed the backslash from the dots too, because hold no special meaning when used within a character class. It wouldn't make sense for it to match any single character in a defined subset of specific characters you're trying to match. =)

warning: assigned but unused variable - filenames

Admittedly, I don't even know what this line is doing:

filenames = Samples.cache['filenames']

But I didn't wanna remove it in case it was there to deliberately invoke a side-effect by accessing Samples.cache['filenames']. But I wanted it to shut up so I found a way to make it "used" by sprucing up a conditional assignment:

-    if interpreters == nil	+    interpreters = {} if interpreters.nil?
-      interpreters = {}	+    filenames    = {} if filenames.nil?
-    end
+    interpreters = {} if interpreters.nil?
+    filenames    = {} if filenames.nil?

Which inevitably lead to me polishing whitespace elsewhere in the file, as you'll see by the diffs. 😁 I'd apologise for being a neat-freak, but you guys know I'm not actually sorry. ❤️

Fixes two warnings displayed in recent test feedback:

* lib/linguist/heuristics.rb:414:
  warning: character class has duplicated range:

      /[A-Z\.][\w\d\.]*:{/

* lib/linguist/language.rb:504:
  warning: assigned but unused variable:

      filenames
interpreters = Samples.cache['interpreters']
filenames = Samples.cache['filenames']
popular = YAML.load_file(File.expand_path("../popular.yml", __FILE__))
filenames = Samples.cache['filenames']
Copy link
Contributor

Choose a reason for hiding this comment

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

This is safe to remove. I forgot to do it in 0bf4b8a.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perfect! Works for me. 😀 Consider it gone, and my drastic spike of Ruby fluency to be concerning

(for me)

Copy link
Collaborator Author

@Alhadis Alhadis May 16, 2018

Choose a reason for hiding this comment

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

And don't worry, I did double-check the ||= operator to guarantee it did what I thought it does. 😉

There's no good reason I should be this good at Ruby, I'm always bitching about why its ecosystem are the antichrist 😢

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

And if by magic, all the Linguist-related warnings are gone. LGTM. 🙇

@Alhadis Alhadis merged commit 44f107b into master May 21, 2018
@Alhadis Alhadis deleted the fix-warnings branch May 21, 2018 08:42
@Alhadis
Copy link
Collaborator Author

Alhadis commented May 21, 2018

Now we just need to get Charlock Holmes to STFU about that encoding variable not being initialised. 😂👌

lildude added a commit that referenced this pull request May 30, 2018
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants