From bb96d631537d3d8181f0d3b762603a012219c3e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 30 Dec 2015 00:52:50 -0500 Subject: New implementation for highlighting diff files. #3945 * It is more performant given now we process all the diff file instead of processing line by line. * Multiline comments are highlighted correctly. --- app/helpers/diff_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 24134310fc5..2ff8c65e5ca 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -54,9 +54,9 @@ module DiffHelper # right_line_type, right_line_number, right_line_content, right_line_code # ] # - diff_file.diff_lines.each do |line| + diff_file.highlighted_diff_lines.each do |line| - full_line = line.text + full_line = line.highlighted_text type = line.type line_code = generate_line_code(diff_file.file_path, line) line_new = line.new_pos @@ -67,7 +67,7 @@ module DiffHelper if next_line next_line_code = generate_line_code(diff_file.file_path, next_line) next_type = next_line.type - next_line = next_line.text + next_line = next_line.highlighted_text end if type == 'match' || type.nil? -- cgit v1.2.1 From 7de90f4b53f865dc417d022a9133372e57274549 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 30 Dec 2015 18:42:11 -0500 Subject: Fix broken spec and small refactor. #3945 --- app/helpers/diff_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 2ff8c65e5ca..22deb69cec5 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -56,7 +56,7 @@ module DiffHelper # diff_file.highlighted_diff_lines.each do |line| - full_line = line.highlighted_text + full_line = line.text type = line.type line_code = generate_line_code(diff_file.file_path, line) line_new = line.new_pos @@ -67,7 +67,7 @@ module DiffHelper if next_line next_line_code = generate_line_code(diff_file.file_path, next_line) next_type = next_line.type - next_line = next_line.highlighted_text + next_line = next_line.text end if type == 'match' || type.nil? -- cgit v1.2.1 From 21b602c60ad787b63039d804a5e15b43d0d3c32c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Thu, 7 Jan 2016 22:37:01 -0500 Subject: Change strategy to highlight diffs. #3945 Now we apply syntax highlighting to the whole old and new files. This basically help us to highlight adequately multiline content. --- app/helpers/diff_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 22deb69cec5..668610364c5 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -19,13 +19,13 @@ module DiffHelper end end - def safe_diff_files(diffs) + def safe_diff_files(diffs, diff_refs, repository) lines = 0 safe_files = [] diffs.first(allowed_diff_size).each do |diff| lines += diff.diff.lines.count break if lines > allowed_diff_lines - safe_files << Gitlab::Diff::File.new(diff) + safe_files << Gitlab::Diff::File.new(diff, diff_refs, repository) end safe_files end -- cgit v1.2.1 From 80a4c808b1e5d331833f7b2ed531cb4fc81c7ef3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Tue, 12 Jan 2016 11:02:16 -0500 Subject: Make diff_line_content helper return a safe String. #3945 --- app/helpers/diff_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 668610364c5..28c8e58b2ad 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -111,9 +111,9 @@ module DiffHelper def diff_line_content(line) if line.blank? - "  " + "  ".html_safe else - line + line.try(:html_safe) end end -- cgit v1.2.1 From c0385488fbfaf1285122793cea417e607c8e771e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Tue, 12 Jan 2016 15:12:30 -0500 Subject: Fix broken spec. #3945 --- app/helpers/diff_helper.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 28c8e58b2ad..0ec532a9a90 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -113,7 +113,8 @@ module DiffHelper if line.blank? "  ".html_safe else - line.try(:html_safe) + # Return line if it isn't a String, it helps when it's Numeric + line.is_a?(String) ? line.html_safe : line end end -- cgit v1.2.1 From 0f0af19139db71255934e9a7a5b5cd86420b7186 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 13 Jan 2016 11:39:15 -0500 Subject: Little refactor for usage of html_safe. #3945 --- app/helpers/diff_helper.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 0ec532a9a90..d49e22e8c84 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -1,4 +1,6 @@ module DiffHelper + BLANK_SPACE = " ".html_safe + def diff_view params[:view] == 'parallel' ? 'parallel' : 'inline' end @@ -83,7 +85,7 @@ module DiffHelper elsif next_type == 'old' || next_type.nil? # Left side has text removed, right side doesn't have any change # No next line code, no new line number, no new line text - line = [type, line_old, full_line, line_code, next_type, nil, " ", nil] + line = [type, line_old, full_line, line_code, next_type, nil, BLANK_SPACE, nil] lines.push(line) end elsif type == 'new' @@ -93,7 +95,7 @@ module DiffHelper next else # Change is only on the right side, left side has no change - line = [nil, nil, " ", line_code, type, line_new, full_line, line_code] + line = [nil, nil, BLANK_SPACE, line_code, type, line_new, full_line, line_code] lines.push(line) end end @@ -113,8 +115,7 @@ module DiffHelper if line.blank? "  ".html_safe else - # Return line if it isn't a String, it helps when it's Numeric - line.is_a?(String) ? line.html_safe : line + line.html_safe end end -- cgit v1.2.1 From c881627d114eb9c050d605e93673ef65a9da9a58 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 14 Jan 2016 14:52:08 +0100 Subject: Refactor parallel_diff generation a bit. --- app/helpers/diff_helper.rb | 86 +++++++++++++++++++++++++++++++++------------- 1 file changed, 63 insertions(+), 23 deletions(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index d49e22e8c84..1596f9e7d19 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -1,6 +1,4 @@ module DiffHelper - BLANK_SPACE = " ".html_safe - def diff_view params[:view] == 'parallel' ? 'parallel' : 'inline' end @@ -49,15 +47,7 @@ module DiffHelper lines = [] skip_next = false - # Building array of lines - # - # [ - # left_type, left_line_number, left_line_content, left_line_code, - # right_line_type, right_line_number, right_line_content, right_line_code - # ] - # diff_file.highlighted_diff_lines.each do |line| - full_line = line.text type = line.type line_code = generate_line_code(diff_file.file_path, line) @@ -72,31 +62,81 @@ module DiffHelper next_line = next_line.text end - if type == 'match' || type.nil? + case type + when 'match', nil # line in the right panel is the same as in the left one - line = [type, line_old, full_line, line_code, type, line_new, full_line, line_code] - lines.push(line) - elsif type == 'old' - if next_type == 'new' + lines << { + left: { + type: type, + number: line_old, + text: full_line, + line_code: line_code, + }, + right: { + type: type, + number: line_new, + text: full_line, + line_code: line_code + } + } + when 'old' + case next_type + when 'new' # Left side has text removed, right side has text added - line = [type, line_old, full_line, line_code, next_type, line_new, next_line, next_line_code] - lines.push(line) + lines << { + left: { + type: type, + number: line_old, + text: full_line, + line_code: line_code, + }, + right: { + type: next_type, + number: line_new, + text: next_line, + line_code: next_line_code + } + } skip_next = true - elsif next_type == 'old' || next_type.nil? + when 'old', nil # Left side has text removed, right side doesn't have any change # No next line code, no new line number, no new line text - line = [type, line_old, full_line, line_code, next_type, nil, BLANK_SPACE, nil] - lines.push(line) + lines << { + left: { + type: type, + number: line_old, + text: full_line, + line_code: line_code, + }, + right: { + type: next_type, + number: nil, + text: "", + line_code: nil + } + } end - elsif type == 'new' + when 'new' if skip_next # Change has been already included in previous line so no need to do it again skip_next = false next else # Change is only on the right side, left side has no change - line = [nil, nil, BLANK_SPACE, line_code, type, line_new, full_line, line_code] - lines.push(line) + lines << { + left: { + type: nil, + number: nil, + text: "", + line_code: line_code, + }, + right: { + type: type, + number: line_new, + text: full_line, + line_code: line_code + } + } end end end -- cgit v1.2.1 From 6b9c730e91962a6d6343bcb7fc4dc75c99b41bde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Thu, 14 Jan 2016 16:38:37 -0500 Subject: More refactoring from last code review. #3945 * Use commit objects instead of IDs when generating diffs * Use proper references when generating MR's source and target * Update broken specs --- app/helpers/diff_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 1596f9e7d19..425a8ced549 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -19,13 +19,13 @@ module DiffHelper end end - def safe_diff_files(diffs, diff_refs, repository) + def safe_diff_files(diffs, diff_refs) lines = 0 safe_files = [] diffs.first(allowed_diff_size).each do |diff| lines += diff.diff.lines.count break if lines > allowed_diff_lines - safe_files << Gitlab::Diff::File.new(diff, diff_refs, repository) + safe_files << Gitlab::Diff::File.new(diff, diff_refs) end safe_files end -- cgit v1.2.1 From 701513dcc7afb403372bc738642a9a52e9be5983 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 20 Jan 2016 14:51:56 +0100 Subject: Move parallel diff logic to separate class --- app/helpers/diff_helper.rb | 104 --------------------------------------------- 1 file changed, 104 deletions(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 425a8ced549..e524f6da9c8 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -39,110 +39,6 @@ module DiffHelper end end - def generate_line_code(file_path, line) - Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos) - end - - def parallel_diff(diff_file, index) - lines = [] - skip_next = false - - diff_file.highlighted_diff_lines.each do |line| - full_line = line.text - type = line.type - line_code = generate_line_code(diff_file.file_path, line) - line_new = line.new_pos - line_old = line.old_pos - - next_line = diff_file.next_line(line.index) - - if next_line - next_line_code = generate_line_code(diff_file.file_path, next_line) - next_type = next_line.type - next_line = next_line.text - end - - case type - when 'match', nil - # line in the right panel is the same as in the left one - lines << { - left: { - type: type, - number: line_old, - text: full_line, - line_code: line_code, - }, - right: { - type: type, - number: line_new, - text: full_line, - line_code: line_code - } - } - when 'old' - case next_type - when 'new' - # Left side has text removed, right side has text added - lines << { - left: { - type: type, - number: line_old, - text: full_line, - line_code: line_code, - }, - right: { - type: next_type, - number: line_new, - text: next_line, - line_code: next_line_code - } - } - skip_next = true - when 'old', nil - # Left side has text removed, right side doesn't have any change - # No next line code, no new line number, no new line text - lines << { - left: { - type: type, - number: line_old, - text: full_line, - line_code: line_code, - }, - right: { - type: next_type, - number: nil, - text: "", - line_code: nil - } - } - end - when 'new' - if skip_next - # Change has been already included in previous line so no need to do it again - skip_next = false - next - else - # Change is only on the right side, left side has no change - lines << { - left: { - type: nil, - number: nil, - text: "", - line_code: line_code, - }, - right: { - type: type, - number: line_new, - text: full_line, - line_code: line_code - } - } - end - end - end - lines - end - def unfold_bottom_class(bottom) (bottom) ? 'js-unfold-bottom' : '' end -- cgit v1.2.1 From 88bd13851300b98c9d0290cfac5ad1f14673b34d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 20 Jan 2016 14:59:14 +0100 Subject: Restore helper --- app/helpers/diff_helper.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index e524f6da9c8..f500d3572e0 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -39,6 +39,10 @@ module DiffHelper end end + def generate_line_code(file_path, line) + Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos) + end + def unfold_bottom_class(bottom) (bottom) ? 'js-unfold-bottom' : '' end -- cgit v1.2.1 From 577f2fb47a7ef57415b78b49d5c746d4e99f6a98 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 20 Jan 2016 18:44:27 +0100 Subject: Save and use actual diff base commit for MR diff highlighting --- app/helpers/diff_helper.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index f500d3572e0..62971d1e14b 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -102,8 +102,7 @@ module DiffHelper def commit_for_diff(diff) if diff.deleted_file - first_commit = @first_commit || @commit - first_commit.parent || @first_commit + @base_commit || @commit.parent || @commit else @commit end -- cgit v1.2.1 From 677b4db9e682b29bb15dddb84fe80b976490a671 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 29 Jan 2016 19:37:17 +0100 Subject: Mark inline difference between old and new paths when a file is renamed --- app/helpers/diff_helper.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 62971d1e14b..0b4fda12788 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -1,4 +1,13 @@ module DiffHelper + def mark_inline_diffs(old_line, new_line) + old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_line, new_line).inline_diffs + + marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs).html_safe + marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs).html_safe + + [marked_old_line, marked_new_line] + end + def diff_view params[:view] == 'parallel' ? 'parallel' : 'inline' end @@ -55,7 +64,7 @@ module DiffHelper if line.blank? "  ".html_safe else - line.html_safe + line end end -- cgit v1.2.1 From 47afe7ae8ff33c756c9ff6e2339a82f07bf11c07 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 30 Jan 2016 12:54:19 +0100 Subject: Fewer html_safes --- app/helpers/diff_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/helpers/diff_helper.rb') diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 0b4fda12788..f9bacc8ba45 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -2,8 +2,8 @@ module DiffHelper def mark_inline_diffs(old_line, new_line) old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_line, new_line).inline_diffs - marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs).html_safe - marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs).html_safe + marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs) + marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs) [marked_old_line, marked_new_line] end -- cgit v1.2.1