Clarify boolean comparison

[rikhuijzer]

This is a bit confusing for readers (or, at least to me):

if x != false
  f()
end
if x == true
  g()
end

[codecov]

Codecov Report

Merging #103 (be09028) into master (36128a7) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
+ Coverage   82.75%   82.76%   +0.01%     
==========================================
  Files          12       12              
  Lines        1461     1462       +1     
==========================================
+ Hits         1209     1210       +1     
  Misses        252      252              

Impacted Files	Coverage Δ
src/scanner.jl	83.42% <100.00%> (+0.02%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36128a7...be09028. Read the comment docs.

[rikhuijzer]

Hmm, chomping == true is not logically equivalent to !isnothing(chomping) because when chomping is false, then the former is false and the latter is true. I'll just put it back.

[kescobo]

I can see how that would be confusing, thanks for doing this! If I can presume on your time a bit more, I think it would be worth adding a comment above that said of operations saying # chomping may be Nothing or Bool, and then (1) change the following conditional to if chomping (since this is logically identical to if chomping == true and (possibly) nest it inside of the other conditional (possibly with short-circuit notation chomping && push!(...)

[rikhuijzer]

You're welcome. I think it is cool to see that even 8 years old code can be changed (8 years according to git blame). According to Lehman, these changes are necessary: "[T]he quality of an E-type system [software performing a real-world activity] will appear to be declining unless it is rigorously maintained and adapted to operational environment changes".

I have tried to use chomping && push!(...) but this failed when chomping === nothing. So, instead I split the cases up and I think it is now much clearer. Let me know if you don't agree.

[kescobo]

Oh right. The way you have it now bothers some "don't repeat code" aesthetic, but I agree that it's probably clearest this way, and simple enough that it's not likely to cause problems.

[kescobo]

Not sure why travis is still trying to fire after we moved to github CI - everything is passing here.

[rikhuijzer]

Oh right. The way you have it now bothers some "don't repeat code" aesthetic, but I agree that it's probably clearest this way, and simple enough that it's not likely to cause problems.

Agreed about the DRY. Anyway, thanks for merging 👍 😄