Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Trailing newlines are calculated and represented in differ output. #290

Closed
wants to merge 5 commits into from
Closed

Trailing newlines are calculated and represented in differ output. #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5a332b7
References changes to rspec-expectations branch
ChaeOkay Jul 4, 2016
3a0af72
Account for trailing newlines
ChaeOkay Jul 4, 2016
5579391
Includes newline in diff calculation
ChaeOkay Jul 4, 2016
7de6553
Prevent trailing newlines when Regexp's need a string representation for
ChaeOkay Jul 4, 2016
b4a3bac
Adds message to last hunk output if `+` or `-` is found
ChaeOkay Jul 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ source 'https://rubygems.org'
gemspec

branch = File.read(File.expand_path("../maintenance-branch", __FILE__)).chomp
%w[rspec rspec-core rspec-expectations rspec-mocks].each do |lib|
#%w[rspec rspec-core rspec-expectations rspec-mocks].each do |lib|
%w[rspec rspec-core rspec-mocks].each do |lib|
library_path = File.expand_path("../../#{lib}", __FILE__)
if File.exist?(library_path) && !ENV['USE_GIT_REPOS']
gem lib, :path => library_path
Expand All @@ -13,6 +14,9 @@ branch = File.read(File.expand_path("../maintenance-branch", __FILE__)).chomp
end
end

gem 'rspec-expectations', git: 'git://github.com/chaeokay/rspec-expectations.git',
branch: 'chaeokay/display_trailing_newlines'

### dep for ci/coverage
gem 'simplecov', '~> 0.8'

Expand Down
11 changes: 9 additions & 2 deletions lib/rspec/support/differ.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ def diff_as_string(actual, expected)
end
end

finalize_output(output, hunks.last.diff(format_type).to_s) if hunks.last

finalize_output(output, final_line(hunks.last)) if hunks.last
color_diff output
rescue Encoding::CompatibilityError
handle_encoding_errors(actual, expected)
Expand Down Expand Up @@ -116,6 +115,12 @@ def build_hunks(actual, expected)
HunkGenerator.new(actual, expected).hunks
end

def final_line(last_hunk)
output_message = last_hunk.diff(format_type).to_s.strip
# Checks last char of string for + or -, appends message on match
output_message.gsub(/(?<=[+-]$)\z/, "\\ No newline at end of input")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Escaping still needs work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than doing this, have you tried leaving the old logic as it was, and just checking for the missing trailing \n?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm up for giving it another go! By 'old logic' do you mean

def expected_lines
  @expected.to_s.split("\n", -1).map! { |e| e.chomp }
end

in https://github.com/ChaeOkay/rspec-support/blob/b4a3bacd999429e7d64b88486e86597eefdbbb14/lib/rspec/support/hunk_generator.rb#L27 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach I think I'm suggesting, is to refactor expected_lines and actual_lines to check for a missing \n and add the text to the array if so, then allowing the differ to proceed as normal

Copy link
Member

@JonRowe JonRowe Jul 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something along the lines of:

def expected_lines
  split_into_lines(@expected)
end

def actual_lines
  split_into_lines(@actual)
end

private

def split_into_lines(string)
  string << "\ No newline at end of input" if string[-1] != "\n"
  string.split("\n").map! { |e| e.chomp }
end

This is psuedo code though, I haven't tried it directly

Copy link
Author

@ChaeOkay ChaeOkay Jul 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay.

Diff:
@@ -1,4 +1,5 @@
 Chewy
+
 bagel.
-\ No newline at end of input
+

Would this (above) be an acceptable output for

expected = "Chewy\nbagel."
actual = "Chewy\n\nbagel.\n"

The feature spec currently shows

Diff:
 @@ -1,3 +1,5 @@
  Chewy
  +
  bagel.
  +\ No newline at end of input

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to not have the trailing + unless it actually represents a proper \n\n, if you can't get quite that, feel free to push it up anyway I'll take another look :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eek, I've been away, but casually fiddling. Hope to move the discussion further, soon. Apologies!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • If we are going to add text to indicate that there is an addition/deletion of an eof newline (instead of nothing at all), do we want this to happen before the actual or encoded string becomes and EncodedString?
  • How do we want to handle internationalization? Would the message always be in English?

end

def finalize_output(output, final_line)
add_to_output(output, final_line)
add_to_output(output, "\n")
Expand Down Expand Up @@ -184,6 +189,8 @@ def object_to_string(object)
PP.pp(ObjectFormatter.prepare_for_inspection(object), "")
when String
object =~ /\n/ ? object : object.inspect
when Regexp
PP.pp(object, "").chomp
else
PP.pp(object, "")
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rspec/support/hunk_generator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ def diffs
end

def expected_lines
@expected.split("\n").map! { |e| e.chomp }
@expected.to_s.split("\n", -1).map! { |e| e.chomp }
end

def actual_lines
@actual.split("\n").map! { |e| e.chomp }
@actual.to_s.split("\n", -1).map! { |e| e.chomp }
end

def build_hunk(piece)
Expand Down
22 changes: 11 additions & 11 deletions spec/rspec/support/differ_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ module Support
this
is
soo
@@ -9,6 +9,5 @@
@@ -9,7 +9,6 @@
equal
insert
a
Expand All @@ -52,7 +52,7 @@ module Support
| this
| is
| soo
|@@ -9,6 +9,5 @@
|@@ -9,7 +9,6 @@
| equal
| insert
| a
Expand Down Expand Up @@ -106,15 +106,15 @@ def differ_ivars
it 'handles differently encoded strings that are compatible' do
expected = "abc\n".encode('us-ascii')
actual = "강인철\n".encode('UTF-8')
expected_diff = "\n@@ -1,2 +1,2 @@\n-abc\n+강인철\n"
expected_diff = "\n@@ -1,3 +1,3 @@\n-abc\n+강인철\n"
diff = differ.diff(actual, expected)
expect(diff).to be_diffed_as(expected_diff)
end

it 'uses the default external encoding when the two strings have incompatible encodings', :failing_on_appveyor do
expected = "Tu avec carte {count} item has\n"
actual = "Tu avec carté {count} itém has\n".encode('UTF-16LE')
expected_diff = "\n@@ -1,2 +1,2 @@\n-Tu avec carte {count} item has\n+Tu avec carté {count} itém has\n"
expected_diff = "\n@@ -1,3 +1,3 @@\n-Tu avec carte {count} item has\n+Tu avec carté {count} itém has\n"

diff = differ.diff(actual, expected)
expect(diff).to be_diffed_as(expected_diff)
Expand Down Expand Up @@ -154,7 +154,7 @@ def inspect

expected_diff = <<'EOD'

@@ -1,5 +1,5 @@
@@ -1,6 +1,6 @@
<Animal
name=bob,
- species=tortoise
Expand All @@ -173,7 +173,7 @@ def inspect
expected_diff = <<'EOD'


@@ -5,7 +5,7 @@
@@ -5,8 +5,8 @@
:metasyntactic,
"variable",
:delta,
Expand Down Expand Up @@ -202,7 +202,7 @@ def inspect; "<BrokenObject>"; end

expected_diff = <<-EOD

@@ -1,2 +1,2 @@
@@ -1,3 +1,3 @@
-[]
+[<BrokenObject>]
EOD
Expand Down Expand Up @@ -296,7 +296,7 @@ def inspect; "<BrokenObject>"; end

it "outputs unified diff message of two arrays with Time object keys" do
expected_diff = %Q{
@@ -1,2 +1,2 @@
@@ -1,3 +1,3 @@
-[#{formatted_time}, "b"]
+[#{formatted_time}, "c"]
}
Expand All @@ -307,7 +307,7 @@ def inspect; "<BrokenObject>"; end

it "outputs unified diff message of two arrays with hashes inside them" do
expected_diff = %Q{
@@ -1,2 +1,2 @@
@@ -1,3 +1,3 @@
-[{"b"=>#{formatted_time}}, "c"]
+[{"a"=>#{formatted_time}}, "c"]
}
Expand Down Expand Up @@ -402,7 +402,7 @@ def inspect; "<BrokenObject>"; end

expected_diff = dedent(<<-EOS)
|
|@@ -1,2 +1,2 @@
|@@ -1,3 +1,3 @@
|-[#<SimpleDelegator(#{object.inspect})>]
|+[#{object.inspect}]
|
Expand Down Expand Up @@ -441,7 +441,7 @@ def inspect; "<BrokenObject>"; end
it "outputs colored diffs" do
expected = "foo bar baz\n"
actual = "foo bang baz\n"
expected_diff = "\e[0m\n\e[0m\e[34m@@ -1,2 +1,2 @@\n\e[0m\e[31m-foo bang baz\n\e[0m\e[32m+foo bar baz\n\e[0m"
expected_diff = "\e[0m\n\e[0m\e[34m@@ -1,3 +1,3 @@\n\e[0m\e[31m-foo bang baz\n\e[0m\e[32m+foo bar baz\n\e[0m"

diff = differ.diff(expected,actual)
expect(diff).to be_diffed_as(expected_diff)
Expand Down