Fix warnings

[rkday]

I included dnsruby 1.57.0 in a project, and hit several Ruby warnings. I'd like to fix those up to reduce the noise in my tests (and make it easier to spot warnings that apply to my own code) - here's a pull request fixing the first few.

[coveralls]

Coverage decreased (-0.05%) to 75.65% when pulling a7d3a82 on rkday:fix_warnings into b3accf2 on alexdalitz:master.

[coveralls]

Coverage decreased (-0.05%) to 75.65% when pulling a7d3a82 on rkday:fix_warnings into b3accf2 on alexdalitz:master.

[alexdalitz]

Hi -

Thanks for this!

I’m not able to look at it until next week, I’m afraid, so sorry for the delay…

Thanks,

Alex.

On 31 Jan 2015, at 22:45, Rob Day notifications@github.com wrote:

I included dnsruby 1.57.0 in a project, and hit several Ruby warnings. I'd like to fix those up to reduce the noise in my tests (and make it easier to spot warnings that apply to my own code) - here's a pull request fixing the first few.

You can view, comment on, or merge this pull request online at:

#66 #66
Commit Summary

Fix 'warning: character class has duplicated range'
Fix 'warning: method redefined; discarding old regexp='
Fix 'warning: assigned but unused variable - e'
Fix 'warning: mismatched indentations at 'end' with 'def' at 243'
File Changes

M lib/dnsruby/name.rb https://github.com/alexdalitz/dnsruby/pull/66/files#diff-0 (4)
M lib/dnsruby/resource/NAPTR.rb https://github.com/alexdalitz/dnsruby/pull/66/files#diff-1 (4)
M lib/dnsruby/resource/NSEC3.rb https://github.com/alexdalitz/dnsruby/pull/66/files#diff-2 (4)
M lib/dnsruby/resource/TXT.rb https://github.com/alexdalitz/dnsruby/pull/66/files#diff-3 (4)
Patch Links:

https://github.com/alexdalitz/dnsruby/pull/66.patch https://github.com/alexdalitz/dnsruby/pull/66.patch
https://github.com/alexdalitz/dnsruby/pull/66.diff https://github.com/alexdalitz/dnsruby/pull/66.diff
—
Reply to this email directly or view it on GitHub #66.

Alex Dalitz
alex@caerkettontech.com

[alexdalitz]

Hi -

Sorry for the delay! I'm now back from the snowboarding in waist deep powder snow :-) and am happy to accept your pull request. However, I do have a question regarding the change in name.rb - could you please help to understand what is going on there?

Thanks!

Alex.

[rkday]

The regular expression /\Gx([0..9a..fA..F][0..9a..fA..F])/o triggers the warning warning: character class has duplicated range: /\Gx([0..9a..fA..F][0..9a..fA..F])/. (You can reproduce this quite easily with echo 'puts "xFF".match(/\Gx([0..9a..fA..F][0..9a..fA..F])/o)' | ruby -d).

I assumed that the duplication of [0..9a..fA..F] was the problem, and so added {2} (to match the preceding element twice). But actually, I think there's a different problem - .. isn't the range operator in regexes, - is. So [0..9a..fA..F] seems to only match 0, 9, a, f, A, F and .:

$ echo 'puts "xFF".match(/\Gx([0..9a..fA..F][0..9a..fA..F])/o) || "no match"' | ruby
xFF
$ echo 'puts "xBB".match(/\Gx([0..9a..fA..F][0..9a..fA..F])/o) || "no match"' | ruby
no match
$ echo 'puts "x..".match(/\Gx([0..9a..fA..F][0..9a..fA..F])/o) || "no match"' | ruby
x..

I think \h is the right thing to match hexadecimal here:

$ echo 'puts "x..".match(/\Gx(\h\h)/o) || "no match"' | ruby
no match
$ echo 'puts "xBB".match(/\Gx(\h\h)/o) || "no match"' | ruby
xBB
$ echo 'puts "xFF".match(/\Gx(\h\h)/o) || "no match"' | ruby
xFF
$ echo 'puts "xaF".match(/\Gx(\h\h)/o) || "no match"' | ruby
xaF
$ echo 'puts "xgG".match(/\Gx(\h\h)/o) || "no match"' | ruby
no match
$ echo 'puts "x9A".match(/\Gx(\h\h)/o) || "no match"' | ruby
x9A

If that makes sense to you, I'll update the pull request.

[alexdalitz]

Again, sorry for the delay...

Yes, that sounds more sensible to me - thanks!

Alex.

[coveralls]

Coverage increased (+0.01%) to 75.72% when pulling 7b6eb15 on rkday:fix_warnings into b3accf2 on alexdalitz:master.