search

crystal-community / icr

Interactive console for Crystal programming language
MIT License
505 stars 40 forks source link

Fixes #61 And fixes the regex to correctly handle constant assignment… #62

Closed ktaragorn closed 7 years ago

ktaragorn commented 7 years ago

… detection in general

Also widen tests available to cover various cases

ktaragorn commented 7 years ago

Good Catches.. Fixed!

On 26 September 2017 at 12:59, V. Elenhaupt notifications@github.com wrote:

@veelenga requested changes on this pull request.

Nicely done. However, I would prefer few slight modifications.

In spec/integration/icr_spec.cr https://github.com/crystal-community/icr/pull/62#discussion_r140957099:

@@ -260,6 +262,40 @@ describe "icr command" do icr(input).should match /false/ end

  • it "allows for regex checking with constant" do
  • input = <<-CRYSTAL
  • A = 0
  • B=1
  • HTTP_STATUS = 404
  • Constant = "cheese"
  • ISO8859_1 = :latin
  • A =~ /test/
  • Abc_B = "123"
  • Abc_B =~ /^\d+/
  • CRYSTAL
  • icr(input).should match /0/

I don't think .should match /0/ is ok here. Remove all lines before Abc_B = "123" or adjust the matcher accordingly.

In spec/integration/icr_spec.cr https://github.com/crystal-community/icr/pull/62#discussion_r140957251:

  • Abc_B = "123"
  • Abc_B =~ /^\d+/
  • CRYSTAL
  • icr(input).should match /0/
  • end
  • it "catches constant equality with constants with lower case letters" do
  • input = <<-CRYSTAL
  • Aa = 0
  • Bb = 1
  • Aa == Bb
  • CRYSTAL
  • icr(input).should match /false/
  • end
  • it "doesnt catch object comparison by .new" do

"does not..."

In spec/integration/icr_spec.cr https://github.com/crystal-community/icr/pull/62#discussion_r140957372:

  • Aa = 0
  • Bb = 1
  • Aa == Bb
  • CRYSTAL
  • icr(input).should match /false/
  • end
  • it "doesnt catch object comparison by .new" do
  • input = <<-CRYSTAL
  • Reference.new == Reference.new
  • CRYSTAL
  • icr(input).should match /false/
  • input = <<-CRYSTAL
  • Reference.new != Reference.new
  • CRYSTAL
  • icr(input).should match /true/

one like inputs can be written without heredoc:

icr("Reference.new == Reference.new").should match /false/ icr("Reference.new != Reference.new").should match /false/

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/crystal-community/icr/pull/62#pullrequestreview-65100529, or mute the thread https://github.com/notifications/unsubscribe-auth/ACbm-AkRfApGXfReFU8MjCiVfrLUjlezks5smIS0gaJpZM4PjqVa .

veelenga commented 7 years ago

👍 /cc @jwoertink @greyblake

jwoertink commented 7 years ago

I checked it out, and it looks good 👍