rspec-support icon indicating copy to clipboard operation
rspec-support copied to clipboard

Differ disregards trailing newlines.

Open phiggins opened this issue 11 years ago • 3 comments

Differ currently disregards any trailing newlines when making a diff of two strings. This has two main consequences I can find.

  • If two strings differ and one has a trailing newline, the diff is identical to the diff of the strings without trailing newlines:
>   puts RSpec::Support::Differ.new.diff("baz:\nfoo\n", "baz:\nbar")

@@ -1,3 +1,3 @@
 baz:
-bar
+foo
 => nil 
> puts RSpec::Support::Differ.new.diff("baz:\nfoo", "baz:\nbar")

@@ -1,3 +1,3 @@
 baz:
-bar
+foo
 => nil 

This isn't necessarily a bug, but it could make the output harder to interpret.

  • If two strings differ only in one having a trailing newline, Differ returns just a newline. The protocol Differ follows is to either output a diff for diffable arguments, or an empty string for non-diffable arguments. Thus two single-line strings get no diff:
> RSpec::Support::Differ.new.diff("foo", "bar")
 => "" 

One multi-line string and one single-line string get a diff:

> puts RSpec::Support::Differ.new.diff("f\noo", "bar")
@@ -1,2 +1,3 @@
-bar
+f
+oo 

Identical strings get no diff:

> RSpec::Support::Differ.new.diff("foo", "foo")
 => "" 

If two strings differ only in one having a trailing newline, Differ returns a newline:

> RSpec::Support::Differ.new.diff("foo\n", "foo")
 => "\n" 

This screws up the usage of Differ in rspec-expectations because it expects the return value of Differ to follow the "empty or diff" pattern. When I attempt to compare two strings that only differ by one newline, I get output that indicates there should be a diff but without a diff:

$ cat test_spec.rb 
describe "test case" do
  it "diffs its arguments" do
    expect("foo\n").to eq("foo")
  end
end

Trimmed output:

test case
  diffs its arguments (FAILED - 1)

Failures:

  1) test case diffs its arguments
     Failure/Error: expect("foo\n").to eq("foo")

       expected: "foo"
            got: "foo\n"

       (compared using ==)

       Diff:

     # /home/pete/projects/rspec-expectations/lib/rspec/expectations/fail_with.rb:33:in `fail_with'
     # /home/pete/projects/rspec-expectations/lib/rspec/expectations/handler.rb:36:in `handle_failure'
     # /home/pete/projects/rspec-expectations/lib/rspec/expectations/handler.rb:49:in `handle_matcher'

I'm not sure how to fix this. It looks like HunkGenerator's preprocessing of strings is getting rid of newlines so I don't know if this can be fixed without mucking with that, potentially breaking other things.

FWIW, I played around with GNU diff in these scenarios and it indicates a trailing new line in diffs:

pete@balloon:/tmp$ cat foo_trailing_newline 
baz:
foo

pete@balloon:/tmp$ cat foo_no_trailing_newline 
baz:
foo
pete@balloon:/tmp$ cat bar 
baz:
bar
pete@balloon:/tmp$ diff foo_trailing_newline foo_no_trailing_newline 
3d2
< 
pete@balloon:/tmp$ diff foo_trailing_newline bar 
2,3c2
< foo
< 

---
> bar
pete@balloon:/tmp$ diff foo_no_trailing_newline bar 
2c2
< foo

---
> bar

phiggins avatar May 26 '14 22:05 phiggins