podigee/device_detector

warnings on Readme's user agent sample

Closed this issue · 10 comments

I was trying out your gem with the user agent from Readme which is:
Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/30.0.1599.17 Safari/537.36.

However when I tried to get device_name, it returned nil along with these warnings:

/Users/victor/.rvm/gems/ruby-2.2.1/gems/device_detector-0.9.0/lib/device_detector/parser.rb:74: warning: character class has '-' without escape
/Users/victor/.rvm/gems/ruby-2.2.1/gems/device_detector-0.9.0/lib/device_detector/parser.rb:74: warning: nested repeat operator '+' and '?' was replaced with '*' in regular expression
/Users/victor/.rvm/gems/ruby-2.2.1/gems/device_detector-0.9.0/lib/device_detector/parser.rb:74: warning: nested repeat operator '+' and '?' was replaced with '*' in regular expression
/Users/victor/.rvm/gems/ruby-2.2.1/gems/device_detector-0.9.0/lib/device_detector/device.rb:53: warning: nested repeat operator '+' and '?' was replaced with '*' in regular expression
=> nil

Is this a bug for ruby 2.2.1?

The result for device_name is actually expected, I think we need a better example user agent here :)

For the warnings: How do you run the example? I tried running it in IRB and via executing a file (in ruby 2.1.5, 2.2.5, 2.3.1) and did not encounter the warnings at all.

The only occasion where I get the warnings is when running the specs for device_detector.

Background for where the warnings come from: We are using the database of user agents from the upstream project https://github.com/piwik/device-detector and there seem to be some regexes which are not optimal, but still working fine. Perhaps we can try to get those "fixed" in the upstream project.

That's weird. I tried on both 2.2.1 and 2.2.5 and got the same warnings.

First, I install your gem:
gem install device_detector

Then I just tried to use it in IRB and I got those warnings and nil was returned.

Ah, now I see it. I had not updated to the most recent version. We updated the regex DB for version 0.9.

We'll need to check if those warnings can be suppressed somehow, or if we should rather aim for fixing the issue upstream.

Opinions? /cc @YAGoOaR @peteygao

Suppressing the warnings works with this: http://mentalized.net/journal/2010/04/02/suppress-warnings-from-ruby/

Still not sure if it's a good idea :)

I don't think that suppressing the warnings is the way to go. I would rather try to get these changes upstream (maybe @sgiehl is kind enough to join the conversation).

If the piwik team decides not to accept the changes, we could pre-process the yaml files with the regular expressions before parsing them into Regexp objects.

That's my humble idea, since I really dislike messing with the Kernel module.

Sure we are always open for improvements. If there are problems with some regexes feel free to create a PR changing them (as long as it doesn't change the result). Or give me detailed information what to change. I currently don't have time to check that on my own.

I'm also getting the same warning.

/home/rails-dev/.rvm/gems/ruby-2.2.2@mypro/gems/device_detector-0.9.0/lib/device_detector/parser.rb:74: warning: character class has '-' without escape
/home/rails-dev/.rvm/gems/ruby-2.2.2@mypro/gems/device_detector-0.9.0/lib/device_detector/parser.rb:74: warning: nested repeat operator '+' and '?' was replaced with '' in regular expression
/home/rails-dev/.rvm/gems/ruby-2.2.2@mypro/gems/device_detector-0.9.0/lib/device_detector/parser.rb:74: warning: nested repeat operator '+' and '?' was replaced with '' in regular expression
/home/rails-dev/.rvm/gems/ruby-2.2.2@mypro/gems/device_detector-0.9.0/lib/device_detector/device.rb:53: warning: nested repeat operator '+' and '?' was replaced with '*' in regular expression

2.3.1 also have such warnings:

/usr/local/rvm/gems/ruby-2.3.1/gems/device_detector-0.9.0/lib/device_detector/parser.rb:74: warning: character class has '-' without escape
/usr/local/rvm/gems/ruby-2.3.1/gems/device_detector-0.9.0/lib/device_detector/parser.rb:74: warning: nested repeat operator '+' and '?' was replaced with '*' in regular expression
/usr/local/rvm/gems/ruby-2.3.1/gems/device_detector-0.9.0/lib/device_detector/parser.rb:74: warning: nested repeat operator '+' and '?' was replaced with '*' in regular expression
/usr/local/rvm/gems/ruby-2.3.1/gems/device_detector-0.9.0/lib/device_detector/device.rb:53: warning: nested repeat operator '+' and '?' was replaced with '*' in regular expression

Take a look at #22

This issue was fixed in v1.0.7.