Skip to content

Commit 9bd108d

Browse files
kpaulissekpaulisse
authored
Merge pull request #78 from github/kpaulisse-newline-diff
Whitespace diff display
2 parents 0edf99b + dea9bbb commit 9bd108d

File tree

3 files changed

+415
-9
lines changed

3 files changed

+415
-9
lines changed

‎lib/octocatalog-diff/catalog-diff/display/text.rb

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -257,13 +257,67 @@ def self.loc_string(loc, compilation_dir, logger)
257257
# @param depth [Fixnum] Depth, for correct indentation
258258
# @return Array<String> Displayable result
259259
def self.diff_two_strings_with_diffy(string1, string2, depth)
260-
# prevent 'No newline at end of file' for single line strings
261-
string1 += "\n" unless string1 =~ /\n/
262-
string2 += "\n" unless string2 =~ /\n/
260+
# Single line strings?
261+
if single_lines?(string1, string2)
262+
string1, string2 = add_trailing_newlines(string1, string2)
263+
diff = Diffy::Diff.new(string1, string2, context: 2, include_diff_info: false).to_s.split("\n")
264+
return diff.map { |x| left_pad(2 * depth + 2, make_trailing_whitespace_visible(adjust_position_of_plus_minus(x))) }
265+
end
266+
267+
# Multiple line strings
268+
string1, string2 = add_trailing_newlines(string1, string2)
263269
diff = Diffy::Diff.new(string1, string2, context: 2, include_diff_info: true).to_s.split("\n")
264270
diff.shift # Remove first line of diff info (filename that makes no sense)
265271
diff.shift # Remove second line of diff info (filename that makes no sense)
266-
diff.map { |x| left_pad(2 * depth + 2, x) }
272+
diff.map { |x| left_pad(2 * depth + 2, make_trailing_whitespace_visible(x)) }
273+
end
274+
275+
# Determine if two incoming strings are single lines. Returns true if both
276+
# incoming strings are single lines, false otherwise.
277+
# @param string_1 [String] First string
278+
# @param string_2 [String] Second string
279+
# @return [Boolean] Whether both incoming strings are single lines
280+
def self.single_lines?(string_1, string_2)
281+
string_1.strip !~ /\n/ && string_2.strip !~ /\n/
282+
end
283+
284+
# Add "\n" to the end of both strings, only if both strings are lacking it.
285+
# This prevents "\\ No newline at end of file" for single string comparison.
286+
# @param string_1 [String] First string
287+
# @param string_2 [String] Second string
288+
# @return [Array<String>] Adjusted string_1, string_2
289+
def self.add_trailing_newlines(string_1, string_2)
290+
return [string_1, string_2] unless string_1 !~ /\n\Z/ && string_2 !~ /\n\Z/
291+
[string_1 + "\n", string_2 + "\n"]
292+
end
293+
294+
# Adjust the space after of the `-` / `+` in the diff for single line diffs.
295+
# Diffy prints diffs with no space between the `-` / `+` in the text, but for
296+
# single lines it's easier to read with that space added.
297+
# @param string_in [String] Input string, which is a line of a diff from diffy
298+
# @return [String] Modified string
299+
def self.adjust_position_of_plus_minus(string_in)
300+
string_in.sub(/\A(\e\[\d+m)?([\-\+])/, '\1\2 ')
301+
end
302+
303+
# Convert trailing whitespace to underscore for display purposes. Also convert special
304+
# whitespace (\r, \n, \t, ...) to character representation.
305+
# @param string_in [String] Input string, which might contain trailing whitespace
306+
# @return [String] Modified string
307+
def self.make_trailing_whitespace_visible(string_in)
308+
return string_in unless string_in =~ /\A((?:.|\n)*?)(\s+)(\e\[0m)?\Z/
309+
beginning = Regexp.last_match(1)
310+
trailing_space = Regexp.last_match(2)
311+
end_escape = Regexp.last_match(3)
312+
313+
# Trailing space adjustment for line endings
314+
trailing_space.gsub! "\n", '\n'
315+
trailing_space.gsub! "\r", '\r'
316+
trailing_space.gsub! "\t", '\t'
317+
trailing_space.gsub! "\f", '\f'
318+
trailing_space.tr! ' ', '_'
319+
320+
[beginning, trailing_space, end_escape].join('')
267321
end
268322

269323
# Get the diff of two hashes. Call the 'diffy' gem for this.
@@ -319,9 +373,9 @@ def self.hash_diff(obj, depth, key_in, nested = false)
319373
if nested && obj[:old].is_a?(Hash) && obj[:new].is_a?(Hash)
320374
# Nested hashes will be stringified and then use 'diffy'
321375
result.concat diff_two_hashes_with_diffy(depth: depth, hash1: obj[:old], hash2: obj[:new])
322-
elsif obj[:old].is_a?(String) && obj[:new].is_a?(String) && (obj[:old] =~ /\n/ || obj[:new] =~ /\n/)
323-
# Multi-line strings will be split and then use 'diffy' to mimic the
324-
# output seen when using "diff" on the command line
376+
elsif obj[:old].is_a?(String) && obj[:new].is_a?(String)
377+
# Strings will use 'diffy' to mimic the output seen when using
378+
# "diff" on the command line.
325379
result.concat diff_two_strings_with_diffy(obj[:old], obj[:new], depth)
326380
else
327381
# Stuff we don't recognize will be converted to a string and printed

‎spec/octocatalog-diff/integration/text_diff_spec.rb

Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
1+
# frozen_string_literal: true
2+
3+
require_relative '../tests/spec_helper'
4+
5+
require OctocatalogDiff::Spec.require_path('catalog')
6+
require OctocatalogDiff::Spec.require_path('catalog-diff/differ')
7+
require OctocatalogDiff::Spec.require_path('catalog-diff/display')
8+
9+
require 'json'
10+
11+
describe 'text diff display for whitespace' do
12+
let(:base_catalog) do
13+
{
14+
'resources' => [
15+
{
16+
'type' => 'File',
17+
'title' => '/tmp/foo',
18+
'parameters' => { 'content' => 'THIS SHOULD BE OVERRIDDEN' }
19+
}
20+
]
21+
}
22+
end
23+
24+
let(:display_opts) { { format: :text } }
25+
let(:logger) { OctocatalogDiff::Spec.setup_logger.first }
26+
let(:diff_opts) { { logger: logger } }
27+
28+
def build_catalog(hash_in, string_in)
29+
hash_in['resources'].first['parameters']['content'] = string_in
30+
OctocatalogDiff::Catalog.new(json: JSON.generate(hash_in))
31+
end
32+
33+
context 'single line identical' do
34+
it 'should display no diffs at all' do
35+
catalog1 = build_catalog(base_catalog, 'file line')
36+
catalog2 = build_catalog(base_catalog, 'file line')
37+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
38+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
39+
answer = []
40+
expect(result).to eq(answer)
41+
end
42+
end
43+
44+
context 'single lines with different newlines' do
45+
it 'should display newline message when newline exists in old' do
46+
catalog1 = build_catalog(base_catalog, "file line\n")
47+
catalog2 = build_catalog(base_catalog, 'file line')
48+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
49+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
50+
answer = [
51+
' File[/tmp/foo] =>',
52+
' parameters =>',
53+
' content =>',
54+
' - file line',
55+
' + file line',
56+
' \\ No newline at end of file',
57+
'*******************************************'
58+
]
59+
expect(result).to eq(answer)
60+
end
61+
62+
it 'should display newline message when newline exists in new' do
63+
catalog1 = build_catalog(base_catalog, 'file line')
64+
catalog2 = build_catalog(base_catalog, "file line\n")
65+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
66+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
67+
answer = [
68+
' File[/tmp/foo] =>',
69+
' parameters =>',
70+
' content =>',
71+
' - file line',
72+
' \\ No newline at end of file',
73+
' + file line',
74+
'*******************************************'
75+
]
76+
expect(result).to eq(answer)
77+
end
78+
end
79+
80+
context 'single line vs. multiple line' do
81+
it 'should display newline message for old' do
82+
catalog1 = build_catalog(base_catalog, 'file line')
83+
catalog2 = build_catalog(base_catalog, "file line\nfile line\n")
84+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
85+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
86+
answer = [
87+
' File[/tmp/foo] =>',
88+
' parameters =>',
89+
' content =>',
90+
' @@ -1 +1,2 @@',
91+
' -file line',
92+
' \\ No newline at end of file',
93+
' +file line',
94+
' +file line',
95+
'*******************************************'
96+
]
97+
expect(result).to eq(answer)
98+
end
99+
end
100+
101+
context 'multiple lines with different newlines' do
102+
it 'should display newline message when newline exists in old' do
103+
catalog1 = build_catalog(base_catalog, "file line\nfile line\n")
104+
catalog2 = build_catalog(base_catalog, "file line\nfile line")
105+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
106+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
107+
answer = [
108+
' File[/tmp/foo] =>',
109+
' parameters =>',
110+
' content =>',
111+
' @@ -1,2 +1,2 @@',
112+
' file line',
113+
' -file line',
114+
' +file line',
115+
' \\ No newline at end of file',
116+
'*******************************************'
117+
]
118+
expect(result).to eq(answer)
119+
end
120+
121+
it 'should display newline message when newline exists in new' do
122+
catalog1 = build_catalog(base_catalog, "file line\nfile line")
123+
catalog2 = build_catalog(base_catalog, "file line\nfile line\n")
124+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
125+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
126+
answer = [
127+
' File[/tmp/foo] =>',
128+
' parameters =>',
129+
' content =>',
130+
' @@ -1,2 +1,2 @@',
131+
' file line',
132+
' -file line',
133+
' \\ No newline at end of file',
134+
' +file line',
135+
'*******************************************'
136+
]
137+
expect(result).to eq(answer)
138+
end
139+
end
140+
141+
context 'both multi-line strings do not end in newline' do
142+
it 'should display differences but no newline alert' do
143+
catalog1 = build_catalog(base_catalog, "file line\nold line\nlast line")
144+
catalog2 = build_catalog(base_catalog, "file line\nnew line\nlast line")
145+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
146+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
147+
answer = [
148+
' File[/tmp/foo] =>',
149+
' parameters =>',
150+
' content =>',
151+
' @@ -1,3 +1,3 @@',
152+
' file line',
153+
' -old line',
154+
' +new line',
155+
' last line',
156+
'*******************************************'
157+
]
158+
expect(result).to eq(answer)
159+
end
160+
end
161+
162+
context 'different line endings' do
163+
it 'should handle windows vs. unix' do
164+
catalog1 = build_catalog(base_catalog, "file line\r\nline two\r\nline three\r\n")
165+
catalog2 = build_catalog(base_catalog, "file line\nline two\nline three\n")
166+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
167+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
168+
answer = [
169+
' File[/tmp/foo] =>',
170+
' parameters =>',
171+
' content =>',
172+
' @@ -1,3 +1,3 @@',
173+
' -file line\\r',
174+
' -line two\\r',
175+
' -line three\\r',
176+
' +file line',
177+
' +line two',
178+
' +line three',
179+
'*******************************************'
180+
]
181+
expect(result).to eq(answer)
182+
end
183+
end
184+
185+
context 'whitespace on single line' do
186+
it 'should display proper diffs for leading whitespace difference' do
187+
catalog1 = build_catalog(base_catalog, ' file line')
188+
catalog2 = build_catalog(base_catalog, 'file line')
189+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
190+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
191+
answer = [
192+
' File[/tmp/foo] =>',
193+
' parameters =>',
194+
' content =>',
195+
' - file line',
196+
' + file line',
197+
'*******************************************'
198+
]
199+
expect(result).to eq(answer)
200+
end
201+
202+
it 'should display proper diffs for trailing whitespace difference' do
203+
catalog1 = build_catalog(base_catalog, 'file line ')
204+
catalog2 = build_catalog(base_catalog, 'file line')
205+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
206+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
207+
answer = [
208+
' File[/tmp/foo] =>',
209+
' parameters =>',
210+
' content =>',
211+
' - file line____',
212+
' + file line',
213+
'*******************************************'
214+
]
215+
expect(result).to eq(answer)
216+
end
217+
218+
it 'should display proper diffs for trailing whitespace size difference' do
219+
catalog1 = build_catalog(base_catalog, 'file line ')
220+
catalog2 = build_catalog(base_catalog, 'file line ')
221+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
222+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
223+
answer = [
224+
' File[/tmp/foo] =>',
225+
' parameters =>',
226+
' content =>',
227+
' - file line____',
228+
' + file line___',
229+
'*******************************************'
230+
]
231+
expect(result).to eq(answer)
232+
end
233+
end
234+
235+
context 'whitespace manipulation with colors enabled' do
236+
let(:display_opts) { { format: :color_text } }
237+
238+
it 'should properly add space between +/- and single string diff' do
239+
catalog1 = build_catalog(base_catalog, 'old value')
240+
catalog2 = build_catalog(base_catalog, 'new value')
241+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
242+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
243+
answer = [
244+
' File[/tmp/foo] =>',
245+
' parameters =>',
246+
' content =>',
247+
" \e[31m- old value\e[0m",
248+
" \e[32m+ new value\e[0m",
249+
'*******************************************'
250+
]
251+
expect(result).to eq(answer)
252+
end
253+
254+
it 'should properly colorize newline warning in old diff' do
255+
catalog1 = build_catalog(base_catalog, 'file line')
256+
catalog2 = build_catalog(base_catalog, "file line\n")
257+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
258+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
259+
answer = [
260+
' File[/tmp/foo] =>',
261+
' parameters =>',
262+
' content =>',
263+
" \e[31m- file line\e[0m",
264+
' \\ No newline at end of file',
265+
" \e[32m+ file line\e[0m",
266+
'*******************************************'
267+
]
268+
expect(result).to eq(answer)
269+
end
270+
271+
it 'should properly colorize newline warning in new diff' do
272+
catalog1 = build_catalog(base_catalog, "file line\n")
273+
catalog2 = build_catalog(base_catalog, 'file line')
274+
diff = OctocatalogDiff::CatalogDiff::Differ.new(diff_opts, catalog1, catalog2)
275+
result = OctocatalogDiff::CatalogDiff::Display.output(diff, display_opts, logger)
276+
answer = [
277+
' File[/tmp/foo] =>',
278+
' parameters =>',
279+
' content =>',
280+
" \e[31m- file line\e[0m",
281+
" \e[32m+ file line\e[0m",
282+
' \\ No newline at end of file',
283+
'*******************************************'
284+
]
285+
expect(result).to eq(answer)
286+
end
287+
end
288+
end

0 commit comments

Comments
 (0)