diff options
| author | Robert Speicher <robert@gitlab.com> | 2016-01-20 21:52:05 +0000 |
|---|---|---|
| committer | Robert Speicher <rspeicher@gmail.com> | 2016-01-20 16:56:13 -0500 |
| commit | eebb691cb60c7786638667acec0e1c191c0a85b9 (patch) | |
| tree | 052a1abdd02b95492e95fefe8fc0d25e01de06a9 | |
| parent | 6a5cd3ca41f61a188b221e6436e9a0d06c0484f2 (diff) | |
| download | gitlab-ce-eebb691cb60c7786638667acec0e1c191c0a85b9.tar.gz | |
Merge branch 'issue_3945' into 'master'
Add syntax highlighting to diff view
Closes #3945
See merge request !2109
50 files changed, 1152 insertions, 392 deletions
diff --git a/CHANGELOG b/CHANGELOG index 66a94334c17..88a5a5f5ef6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -10,6 +10,7 @@ v 8.4.0 (unreleased) - Autocomplete data is now always loaded, instead of when focusing a comment text area - Improved performance of finding issues for an entire group - Added custom application performance measuring system powered by InfluxDB + - Add syntax highlighting to diffs - Gracefully handle invalid UTF-8 sequences in Markdown links (Stan Hu) - Bump fog to 1.36.0 (Stan Hu) - Add user's last used IP addresses to admin page (Stan Hu) diff --git a/app/assets/stylesheets/highlight/dark.scss b/app/assets/stylesheets/highlight/dark.scss index 6a2b25ddc67..8201735beb5 100644 --- a/app/assets/stylesheets/highlight/dark.scss +++ b/app/assets/stylesheets/highlight/dark.scss @@ -90,4 +90,22 @@ .vg { color: #cc6666 } /* Name.Variable.Global */ .vi { color: #cc6666 } /* Name.Variable.Instance */ .il { color: #de935f } /* Literal.Number.Integer.Long */ + + .line_holder { + &.parallel .new.new_line, + &.parallel .new.line_content, + &.new .old_line, + &.new .new_line, + &.new .line_content { + @include diff_background(255, 255, 255, #808080); + } + + &.parallel .old.old_line, + &.parallel .old.line_content, + &.old .old_line, + &.old .new_line, + &.old .line_content { + @include diff_background(255, 51, 51, #808080); + } + } } diff --git a/app/assets/stylesheets/highlight/monokai.scss b/app/assets/stylesheets/highlight/monokai.scss index 8560c3c490f..cc03ed6ae45 100644 --- a/app/assets/stylesheets/highlight/monokai.scss +++ b/app/assets/stylesheets/highlight/monokai.scss @@ -90,4 +90,22 @@ .gu { color: #75715e; } /* Generic.Subheading & Diff Unified/Comment? */ .gd { color: #f92672; } /* Generic.Deleted & Diff Deleted */ .gi { color: #a6e22e; } /* Generic.Inserted & Diff Inserted */ + + .line_holder { + &.parallel .new.new_line, + &.parallel .new.line_content, + &.new .old_line, + &.new .new_line, + &.new .line_content { + @include diff_background(156, 175, 183, #808080); + } + + &.parallel .old.old_line, + &.parallel .old.line_content, + &.old .old_line, + &.old .new_line, + &.old .line_content { + @include diff_background(254, 147, 140, #808080); + } + } } diff --git a/app/assets/stylesheets/highlight/solarized_dark.scss b/app/assets/stylesheets/highlight/solarized_dark.scss index 7d489a9666b..fdfac6cd249 100644 --- a/app/assets/stylesheets/highlight/solarized_dark.scss +++ b/app/assets/stylesheets/highlight/solarized_dark.scss @@ -111,4 +111,22 @@ .vg { color: #268bd2 } /* Name.Variable.Global */ .vi { color: #268bd2 } /* Name.Variable.Instance */ .il { color: #2aa198 } /* Literal.Number.Integer.Long */ + + .line_holder { + &.parallel .new.new_line, + &.parallel .new.line_content, + &.new .old_line, + &.new .new_line, + &.new .line_content { + @include diff_background(255, 255, 255, #808080); + } + + &.parallel .old.old_line, + &.parallel .old.line_content, + &.old .old_line, + &.old .new_line, + &.old .line_content { + @include diff_background(255, 51, 51, #808080); + } + } } diff --git a/app/assets/stylesheets/highlight/solarized_light.scss b/app/assets/stylesheets/highlight/solarized_light.scss index 200ed346446..f9788951aa8 100644 --- a/app/assets/stylesheets/highlight/solarized_light.scss +++ b/app/assets/stylesheets/highlight/solarized_light.scss @@ -111,4 +111,23 @@ .vg { color: #268bd2 } /* Name.Variable.Global */ .vi { color: #268bd2 } /* Name.Variable.Instance */ .il { color: #2aa198 } /* Literal.Number.Integer.Long */ + + + .line_holder { + &.parallel .new.new_line, + &.parallel .new.line_content, + &.new .old_line, + &.new .new_line, + &.new .line_content { + @include diff_background(92, 164, 169, #FAF3DD); + } + + &.parallel .old.old_line, + &.parallel .old.line_content, + &.old .old_line, + &.old .new_line, + &.old .line_content { + @include diff_background(237, 106, 90, #FAF3DD); + } + } } diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 1e2b8b51827..012232a708e 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -392,3 +392,18 @@ right: 15px; } } + +@mixin diff_background($r, $g, $b, $custom-border) { + /* Fallback for web browsers that doesn't support RGBa */ + background: rgb($r, $g, $b); + /* RGBa with 0.3 opacity */ + background: rgba($r, $g, $b, 0.3); + + &.new_line, &.old_line { + border-right-color: $custom-border !important; + } + + &.line_content span.idiff { + background: rgb($r, $g, $b); + } +} diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 633c3f55614..2d735b90597 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -25,6 +25,7 @@ class ApplicationController < ActionController::Base helper_method :abilities, :can?, :current_application_settings helper_method :import_sources_enabled?, :github_import_enabled?, :github_import_configured?, :gitlab_import_enabled?, :gitlab_import_configured?, :bitbucket_import_enabled?, :bitbucket_import_configured?, :gitorious_import_enabled?, :google_code_import_enabled?, :fogbugz_import_enabled?, :git_import_enabled? + helper_method :repository rescue_from Encoding::CompatibilityError do |exception| log_exception(exception) diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index c56a3497bb2..8133de90a41 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -65,8 +65,9 @@ class Projects::BlobController < Projects::ApplicationController end def diff - @form = UnfoldForm.new(params) - @lines = @blob.data.lines[@form.since - 1..@form.to - 1] + @form = UnfoldForm.new(params) + @lines = Gitlab::Highlight.highlight_lines(repository, @ref, @path) + @lines = @lines[@form.since - 1..@form.to - 1] if @form.bottom? @match_line = '' diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 870f6795219..f5a169e5aa9 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -72,6 +72,7 @@ class Projects::CommitController < Projects::ApplicationController @diffs = commit.diffs end + @diff_refs = [commit.parent || commit, commit] @notes_count = commit.notes.count @statuses = ci_commit.statuses if ci_commit diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 5200d609cc9..f8ec76cd4e5 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -21,7 +21,8 @@ class Projects::CompareController < Projects::ApplicationController @commits = Commit.decorate(compare_result.commits, @project) @diffs = compare_result.diffs @commit = @project.commit(head_ref) - @first_commit = @project.commit(base_ref) + @base_commit = @project.commit(base_ref) + @diff_refs = [@base_commit, @commit] @line_notes = [] end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index a6284a24223..ed3050d59aa 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -58,7 +58,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController def diffs @commit = @merge_request.last_commit - @first_commit = @merge_request.first_commit + @base_commit = @merge_request.diff_base_commit + + # MRs created before 8.4 don't have a diff_base_commit, + # but we need it for the "View file @ ..." link by deleted files + @base_commit ||= @merge_request.first_commit.parent || @merge_request.first_commit @comments_allowed = @reply_allowed = true @comments_target = { @@ -102,7 +106,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @source_project = merge_request.source_project @commits = @merge_request.compare_commits.reverse @commit = @merge_request.last_commit - @first_commit = @merge_request.first_commit + @base_commit = @merge_request.diff_base_commit @diffs = @merge_request.compare_diffs @ci_commit = @merge_request.ci_commit diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index d31d4cde08f..7c55934edda 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -1,21 +1,10 @@ module BlobHelper - def highlight(blob_name, blob_content, nowrap: false, continue: false) - @formatter ||= Rouge::Formatters::HTMLGitlab.new( - nowrap: nowrap, - cssclass: 'code highlight', - lineanchors: true, - lineanchorsid: 'LC' - ) - - begin - @lexer ||= Rouge::Lexer.guess(filename: blob_name, source: blob_content).new - result = @formatter.format(@lexer.lex(blob_content, continue: continue)).html_safe - rescue - @lexer = Rouge::Lexers::PlainText - result = @formatter.format(@lexer.lex(blob_content)).html_safe - end + def highlighter(blob_name, blob_content, nowrap: false) + Gitlab::Highlight.new(blob_name, blob_content, nowrap: nowrap) + end - result + def highlight(blob_name, blob_content, nowrap: false) + Gitlab::Highlight.highlight(blob_name, blob_content, nowrap: nowrap) end def no_highlight_files diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 24134310fc5..62971d1e14b 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) 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) end safe_files end @@ -43,64 +43,6 @@ module DiffHelper Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos) end - def parallel_diff(diff_file, index) - 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.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 - - if type == 'match' || type.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' - # 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) - skip_next = true - 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] - lines.push(line) - end - elsif type == '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, " ", line_code, type, line_new, full_line, line_code] - lines.push(line) - end - end - end - lines - end - def unfold_bottom_class(bottom) (bottom) ? 'js-unfold-bottom' : '' end @@ -111,9 +53,9 @@ module DiffHelper def diff_line_content(line) if line.blank? - " " + " ".html_safe else - line + line.html_safe end end @@ -160,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 diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index c63d0c01653..41dd248d80a 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -180,6 +180,14 @@ class MergeRequest < ActiveRecord::Base merge_request_diff ? merge_request_diff.first_commit : compare_commits.first end + def diff_base_commit + if merge_request_diff + merge_request_diff.base_commit + else + self.target_project.commit(self.target_branch) + end + end + def last_commit_short_sha last_commit.short_id end @@ -254,7 +262,7 @@ class MergeRequest < ActiveRecord::Base end def mergeable? - return false unless open? && !work_in_progress? + return false unless open? && !work_in_progress? && !broken? check_if_can_be_merged @@ -477,8 +485,7 @@ class MergeRequest < ActiveRecord::Base end def target_sha - @target_sha ||= target_project. - repository.commit(target_branch).sha + @target_sha ||= target_project.repository.commit(target_branch).sha end def source_sha @@ -517,4 +524,10 @@ class MergeRequest < ActiveRecord::Base def ci_commit @ci_commit ||= source_project.ci_commit(last_commit.id) if last_commit && source_project end + + def diff_refs + return nil unless diff_base_commit + + [diff_base_commit, last_commit] + end end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index c499a4b5b4c..ba0194cd0a6 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -73,6 +73,12 @@ class MergeRequestDiff < ActiveRecord::Base commits.last end + def base_commit + return nil unless self.base_commit_sha + + merge_request.target_project.commit(self.base_commit_sha) + end + def last_commit_short_sha @last_commit_short_sha ||= last_commit.short_id end @@ -156,6 +162,9 @@ class MergeRequestDiff < ActiveRecord::Base end self.st_diffs = new_diffs + + self.base_commit_sha = merge_request.target_project.commit(target_branch).try(:sha) + self.save end diff --git a/app/views/projects/blame/show.html.haml b/app/views/projects/blame/show.html.haml index 8d9ec068a43..d5d04954490 100644 --- a/app/views/projects/blame/show.html.haml +++ b/app/views/projects/blame/show.html.haml @@ -15,6 +15,7 @@ .file-content.blame.highlight %table - current_line = 1 + - blame_highlighter = highlighter(@blob.name, @blob.data, nowrap: true) - @blame.each do |blame_group| %tr %td.blame-commit @@ -41,5 +42,5 @@ %pre{class: 'code highlight white'} %code - blame_group[:lines].each do |line| - :erb - <%= highlight(@blob.name, line, nowrap: true, continue: true).html_safe %> + :preserve + #{blame_highlighter.highlight(line)} diff --git a/app/views/projects/blob/diff.html.haml b/app/views/projects/blob/diff.html.haml index f3b01ff3288..2e913802be1 100644 --- a/app/views/projects/blob/diff.html.haml +++ b/app/views/projects/blob/diff.html.haml @@ -11,7 +11,7 @@ %td.old_line.diff-line-num{data: {linenumber: line_old}} = link_to raw(line_old), "#" %td.new_line= link_to raw(line_new) , "#" - %td.line_content.noteable_line= ' ' * @form.indent + line + %td.line_content.noteable_line==#{' ' * @form.indent}#{line} - if @form.unfold? && @form.bottom? && @form.to < @blob.loc %tr.line_holder{ id: @form.to } diff --git a/app/views/projects/blob/preview.html.haml b/app/views/projects/blob/preview.html.haml index e7c3460ad78..fed483d6788 100644 --- a/app/views/projects/blob/preview.html.haml +++ b/app/views/projects/blob/preview.html.haml @@ -20,6 +20,6 @@ - else %td.old_line %td.new_line - %td.line_content{class: "#{line.type}"}= raw diff_line_content(line.text) + %td.line_content{class: "#{line.type}"}= diff_line_content(line.text) - else .nothing-here-block No changes. diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml index 02297158dec..05dbe5ebea4 100644 --- a/app/views/projects/commit/show.html.haml +++ b/app/views/projects/commit/show.html.haml @@ -9,5 +9,6 @@ = render "ci_menu" - else %div.block-connector -= render "projects/diffs/diffs", diffs: @diffs, project: @project += render "projects/diffs/diffs", diffs: @diffs, project: @project, + diff_refs: @diff_refs = render "projects/notes/notes_with_form" diff --git a/app/views/projects/compare/show.html.haml b/app/views/projects/compare/show.html.haml index 51088a7dea8..da731f28bb6 100644 --- a/app/views/projects/compare/show.html.haml +++ b/app/views/projects/compare/show.html.haml @@ -9,7 +9,7 @@ - if @commits.present? .prepend-top-default = render "projects/commits/commit_list" - = render "projects/diffs/diffs", diffs: @diffs, project: @project + = render "projects/diffs/diffs", diffs: @diffs, project: @project, diff_refs: @diff_refs - else .light-well.prepend-top-default .center diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index f67058ae0ba..d668f483bcb 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -1,7 +1,7 @@ - if diff_view == 'parallel' - fluid_layout true -- diff_files = safe_diff_files(diffs) +- diff_files = safe_diff_files(diffs, diff_refs) .content-block.oneline-block .inline-parallel-buttons diff --git a/app/views/projects/diffs/_parallel_view.html.haml b/app/views/projects/diffs/_parallel_view.html.haml index 37fd1b1ec8a..a15d147ab05 100644 --- a/app/views/projects/diffs/_parallel_view.html.haml +++ b/app/views/projects/diffs/_parallel_view.html.haml @@ -1,42 +1,40 @@ / Side-by-side diff view -%div.text-file.diff-wrap-lines +%div.text-file.diff-wrap-lines.code.file-content.js-syntax-highlight %table - - parallel_diff(diff_file, index).each do |line| - - type_left = line[0] - - line_number_left = line[1] - - line_content_left = line[2] - - line_code_left = line[3] - - type_right = line[4] - - line_number_right = line[5] - - line_content_right = line[6] - - line_code_right = line[7] - + - diff_file.parallel_diff_lines.each do |line| + - left = line[:left] + - right = line[:right] %tr.line_holder.parallel - - if type_left == 'match' - = render "projects/diffs/match_line_parallel", { line: line_content_left, - line_old: line_number_left, line_new: line_number_right } - - elsif type_left == 'old' || type_left.nil? - %td.old_line{id: line_code_left, class: "#{type_left}"} - = link_to raw(line_number_left), "##{line_code_left}", id: line_code_left + - if left[:type] == 'match' + = render "projects/diffs/match_line_parallel", { line: left[:text], + line_old: left[:number], line_new: right[:number] } + - elsif left[:type] == 'nonewline' + %td.old_line + %td.line_content.parallel.matched= left[:text] + %td.new_line + %td.line_content.parallel.matched= left[:text] + - else + %td.old_line{id: left[:line_code], class: "#{left[:type]}"} + = link_to raw(left[:number]), "##{left[:line_code]}", id: left[:line_code] - if @comments_allowed && can?(current_user, :create_note, @project) - = link_to_new_diff_note(line_code_left, 'old') - %td.line_content{class: "parallel noteable_line #{type_left} #{line_code_left}", "line_code" => line_code_left }= raw line_content_left + = link_to_new_diff_note(left[:line_code], 'old') + %td.line_content{class: "parallel noteable_line #{left[:type]} #{left[:line_code]}", data: { line_code: left[:line_code] }}= diff_line_content(left[:text]) - - if type_right == 'new' + - if right[:type] == 'new' - new_line_class = 'new' - - new_line_code = line_code_right + - new_line_code = right[:line_code] - else - new_line_class = nil - - new_line_code = line_code_left + - new_line_code = left[:line_code] - %td.new_line{id: new_line_code, class: "#{new_line_class}", data: { linenumber: line_number_right }} - = link_to raw(line_number_right), "##{new_line_code}", id: new_line_code + %td.new_line{id: new_line_code, class: "#{new_line_class}", data: { linenumber: right[:number] }} + = link_to raw(right[:number]), "##{new_line_code}", id: new_line_code - if @comments_allowed && can?(current_user, :create_note, @project) - = link_to_new_diff_note(line_code_right, 'new') - %td.line_content.parallel{class: "noteable_line #{new_line_class} #{new_line_code}", "line_code" => new_line_code}= raw line_content_right + = link_to_new_diff_note(right[:line_code], 'new') + %td.line_content.parallel{class: "noteable_line #{new_line_class} #{new_line_code}", data: { line_code: new_line_code }}= diff_line_content(right[:text]) - if @reply_allowed - - comments_left, comments_right = organize_comments(type_left, type_right, line_code_left, line_code_right) + - comments_left, comments_right = organize_comments(left[:type], right[:type], left[:line_code], right[:line_code]) - if comments_left.present? || comments_right.present? = render "projects/notes/diff_notes_with_reply_parallel", notes_left: comments_left, notes_right: comments_right diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml index 977ca423f75..f4fc6caba0f 100644 --- a/app/views/projects/diffs/_text_file.html.haml +++ b/app/views/projects/diffs/_text_file.html.haml @@ -3,9 +3,10 @@ .suppressed-container %a.show-suppressed-diff.js-show-suppressed-diff Changes suppressed. Click to show. -%table.text-file{class: "#{'hide' if too_big}"} +%table.text-file.code.js-syntax-highlight{ class: too_big ? 'hide' : '' } + - last_line = 0 - - diff_file.diff_lines.each_with_index do |line, index| + - diff_file.highlighted_diff_lines.each_with_index do |line, index| - type = line.type - last_line = line.new_pos - line_code = generate_line_code(diff_file.file_path, line) @@ -14,14 +15,18 @@ - if type == "match" = render "projects/diffs/match_line", {line: line.text, line_old: line_old, line_new: line.new_pos, bottom: false, new_file: diff_file.new_file} + - elsif type == 'nonewline' + %td.old_line.diff-line-num + %td.new_line.diff-line-num + %td.line_content.matched= line.text - else %td.old_line = link_to raw(type == "new" ? " " : line_old), "##{line_code}", id: line_code - if @comments_allowed && can?(current_user, :create_note, @project) = link_to_new_diff_note(line_code) %td.new_line{data: {linenumber: line.new_pos}} - = link_to raw(type == "old" ? " " : line.new_pos) , "##{line_code}", id: line_code - %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line.text) + = link_to raw(type == "old" ? " " : line.new_pos), "##{line_code}", id: line_code + %td.line_content{class: "noteable_line #{type} #{line_code}", data: { line_code: line_code }}= diff_line_content(line.text) - if @reply_allowed - comments = @line_notes.select { |n| n.line_code == line_code && n.active? }.sort_by(&:created_at) diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index dd2c59e112a..4c5a9818e3e 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -38,7 +38,7 @@ = render "projects/merge_requests/show/commits" #diffs.diffs.tab-pane.active - if @diffs.present? - = render "projects/diffs/diffs", diffs: @diffs, project: @project + = render "projects/diffs/diffs", diffs: @diffs, project: @project, diff_refs: @merge_request.diff_refs - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE .alert.alert-danger %h4 This comparison includes more than #{MergeRequestDiff::COMMITS_SAFE_SIZE} commits. diff --git a/app/views/projects/merge_requests/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml index d9cfc3d7ae9..64cd484193e 100644 --- a/app/views/projects/merge_requests/show/_diffs.html.haml +++ b/app/views/projects/merge_requests/show/_diffs.html.haml @@ -1,5 +1,6 @@ - if @merge_request_diff.collected? - = render "projects/diffs/diffs", diffs: params[:w] == '1' ? @merge_request.diffs_no_whitespace : @merge_request.diffs, project: @merge_request.project + = render "projects/diffs/diffs", diffs: params[:w] == '1' ? @merge_request.diffs_no_whitespace : @merge_request.diffs, + project: @merge_request.project, diff_refs: @merge_request.diff_refs - elsif @merge_request_diff.empty? .nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch} - else diff --git a/app/views/projects/notes/discussions/_diff.html.haml b/app/views/projects/notes/discussions/_diff.html.haml index 0301445b5b2..46962b184ce 100644 --- a/app/views/projects/notes/discussions/_diff.html.haml +++ b/app/views/projects/notes/discussions/_diff.html.haml @@ -24,7 +24,7 @@ = raw(type == "new" ? " " : line.old_pos) %td.new_line = raw(type == "old" ? " " : line.new_pos) - %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line.text) + %td.line_content{class: "noteable_line #{type} #{line_code}", line_code: line_code}= diff_line_content(line.text) - if line_code == note.line_code = render "projects/notes/diff_notes_with_reply", notes: discussion_notes diff --git a/app/views/shared/_file_highlight.html.haml b/app/views/shared/_file_highlight.html.haml index 2bc98983d67..1bef1de433a 100644 --- a/app/views/shared/_file_highlight.html.haml +++ b/app/views/shared/_file_highlight.html.haml @@ -9,5 +9,4 @@ %i.fa.fa-link = i .blob-content{data: {blob_id: blob.id}} - :preserve - #{highlight(blob.name, blob.data)} + = highlight(blob.name, blob.data) diff --git a/db/migrate/20160120172143_add_base_commit_sha_to_merge_request_diffs.rb b/db/migrate/20160120172143_add_base_commit_sha_to_merge_request_diffs.rb new file mode 100644 index 00000000000..d6c6aa4a4e8 --- /dev/null +++ b/db/migrate/20160120172143_add_base_commit_sha_to_merge_request_diffs.rb @@ -0,0 +1,5 @@ +class AddBaseCommitShaToMergeRequestDiffs < ActiveRecord::Migration + def change + add_column :merge_request_diffs, :base_commit_sha, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index b52f3a2d094..67bfc775a1e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160120130905) do +ActiveRecord::Schema.define(version: 20160120172143) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -494,6 +494,7 @@ ActiveRecord::Schema.define(version: 20160120130905) do t.integer "merge_request_id", null: false t.datetime "created_at" t.datetime "updated_at" + t.string "base_commit_sha" end add_index "merge_request_diffs", ["merge_request_id"], name: "index_merge_request_diffs_on_merge_request_id", unique: true, using: :btree @@ -729,19 +730,19 @@ ActiveRecord::Schema.define(version: 20160120130905) do t.string "type" t.string "title" t.integer "project_id" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.boolean "active", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.boolean "active", null: false t.text "properties" - t.boolean "template", default: false - t.boolean "push_events", default: true - t.boolean "issues_events", default: true - t.boolean "merge_requests_events", default: true - t.boolean "tag_push_events", default: true - t.boolean "note_events", default: true, null: false - t.boolean "build_events", default: false, null: false - t.string "category", default: "common", null: false - t.boolean "default", default: false + t.boolean "template", default: false + t.boolean "push_events", default: true + t.boolean "issues_events", default: true + t.boolean "merge_requests_events", default: true + t.boolean "tag_push_events", default: true + t.boolean "note_events", default: true, null: false + t.boolean "build_events", default: false, null: false + t.string "category", default: "common", null: false + t.boolean "default", default: false end add_index "services", ["category"], name: "index_services_on_category", using: :btree @@ -858,7 +859,7 @@ ActiveRecord::Schema.define(version: 20160120130905) do t.boolean "hide_project_limit", default: false t.string "unlock_token" t.datetime "otp_grace_period_started_at" - t.boolean "ldap_email", default: false, null: false + t.boolean "ldap_email", default: false, null: false end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 79061cd0141..a484177ae33 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -1,13 +1,22 @@ module Gitlab module Diff class File - attr_reader :diff + attr_reader :diff, :diff_refs delegate :new_file, :deleted_file, :renamed_file, :old_path, :new_path, to: :diff, prefix: false - def initialize(diff) + def initialize(diff, diff_refs) @diff = diff + @diff_refs = diff_refs + end + + def old_ref + diff_refs[0] if diff_refs + end + + def new_ref + diff_refs[1] if diff_refs end # Array of Gitlab::DIff::Line objects @@ -15,6 +24,14 @@ module Gitlab @lines ||= parser.parse(raw_diff.lines) end + def highlighted_diff_lines + Gitlab::Diff::Highlight.new(self).highlight + end + + def parallel_diff_lines + Gitlab::Diff::ParallelDiff.new(self).parallelize + end + def mode_changed? !!(diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode) end diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb new file mode 100644 index 00000000000..179f8164c84 --- /dev/null +++ b/lib/gitlab/diff/highlight.rb @@ -0,0 +1,75 @@ +module Gitlab + module Diff + class Highlight + attr_reader :diff_file + + delegate :old_path, :new_path, :old_ref, :new_ref, to: :diff_file, prefix: :diff + + def initialize(diff_file) + @diff_file = diff_file + @diff_lines = diff_file.diff_lines + @raw_lines = @diff_lines.map(&:text) + end + + def highlight + @diff_lines.each_with_index do |diff_line, i| + # ignore highlighting for "match" lines + next if diff_line.type == 'match' || diff_line.type == 'nonewline' + + rich_line = highlight_line(diff_line, i) + + if line_inline_diffs = inline_diffs[i] + rich_line = InlineDiffMarker.new(diff_line.text, rich_line).mark(line_inline_diffs) + end + + diff_line.text = rich_line.html_safe + end + + @diff_lines + end + + private + + def highlight_line(diff_line, index) + return html_escape(diff_line.text) unless diff_file.diff_refs + + line_prefix = diff_line.text.match(/\A(.)/) ? $1 : ' ' + + case diff_line.type + when 'new', nil + rich_line = new_lines[diff_line.new_pos - 1] + when 'old' + rich_line = old_lines[diff_line.old_pos - 1] + end + + # Only update text if line is found. This will prevent + # issues with submodules given the line only exists in diff content. + rich_line ? line_prefix + rich_line : html_escape(diff_line.text) + end + + def inline_diffs + @inline_diffs ||= InlineDiff.new(@raw_lines).inline_diffs + end + + def old_lines + @old_lines ||= Gitlab::Highlight.highlight_lines(*processing_args(:old)) + end + + def new_lines + @new_lines ||= Gitlab::Highlight.highlight_lines(*processing_args(:new)) + end + + def processing_args(version) + ref = send("diff_#{version}_ref") + path = send("diff_#{version}_path") + + [ref.project.repository, ref.id, path] + end + + def html_escape(str) + replacements = { '&' => '&', '>' => '>', '<' => '<', '"' => '"', "'" => ''' } + str.gsub(/[&"'><]/, replacements) + end + end + end +end diff --git a/lib/gitlab/diff/inline_diff.rb b/lib/gitlab/diff/inline_diff.rb new file mode 100644 index 00000000000..b8a61ad6115 --- /dev/null +++ b/lib/gitlab/diff/inline_diff.rb @@ -0,0 +1,73 @@ +module Gitlab + module Diff + class InlineDiff + attr_accessor :lines + + def initialize(lines) + @lines = lines + end + + def inline_diffs + inline_diffs = [] + + local_edit_indexes.each do |index| + old_index = index + new_index = index + 1 + old_line = @lines[old_index] + new_line = @lines[new_index] + + # Skip inline diff if empty line was replaced with content + next if old_line[1..-1] == "" + + # Add one, because this is based on the prefixless version + lcp = longest_common_prefix(old_line[1..-1], new_line[1..-1]) + 1 + lcs = longest_common_suffix(old_line[lcp..-1], new_line[lcp..-1]) + + old_diff_range = lcp..(old_line.length - lcs - 1) + new_diff_range = lcp..(new_line.length - lcs - 1) + + inline_diffs[old_index] = [old_diff_range] if old_diff_range.begin <= old_diff_range.end + inline_diffs[new_index] = [new_diff_range] if new_diff_range.begin <= new_diff_range.end + end + + inline_diffs + end + + private + + # Find runs of single line edits + def local_edit_indexes + line_prefixes = @lines.map { |line| line.match(/\A([+-])/) ? $1 : ' ' } + joined_line_prefixes = " #{line_prefixes.join} " + + offset = 0 + local_edit_indexes = [] + while index = joined_line_prefixes.index(" -+ ", offset) + local_edit_indexes << index + offset = index + 1 + end + + local_edit_indexes + end + + def longest_common_prefix(a, b) + max_length = [a.length, b.length].max + + length = 0 + (0..max_length - 1).each do |pos| + old_char = a[pos] + new_char = b[pos] + + break if old_char != new_char + length += 1 + end + + length + end + + def longest_common_suffix(a, b) + longest_common_prefix(a.reverse, b.reverse) + end + end + end +end diff --git a/lib/gitlab/diff/inline_diff_marker.rb b/lib/gitlab/diff/inline_diff_marker.rb new file mode 100644 index 00000000000..1d7fa1bce06 --- /dev/null +++ b/lib/gitlab/diff/inline_diff_marker.rb @@ -0,0 +1,109 @@ +module Gitlab + module Diff + class InlineDiffMarker + attr_accessor :raw_line, :rich_line + + def initialize(raw_line, rich_line = raw_line) + @raw_line = raw_line + @rich_line = rich_line + end + + def mark(line_inline_diffs) + marker_ranges = [] + line_inline_diffs.each do |inline_diff_range| + # Map the inline-diff range based on the raw line to character positions in the rich line + inline_diff_positions = position_mapping[inline_diff_range].flatten + # Turn the array of character positions into ranges + marker_ranges.concat(collapse_ranges(inline_diff_positions)) + end + + offset = 0 + # Mark each range + marker_ranges.each do |range| + offset = insert_around_range(rich_line, range, "<span class='idiff'>", "</span>", offset) + end + + rich_line + end + + private + + # Mapping of character positions in the raw line, to the rich (highlighted) line + def position_mapping + @position_mapping ||= begin + mapping = [] + rich_pos = 0 + (0..raw_line.length).each do |raw_pos| + rich_char = rich_line[rich_pos] + + # The raw and rich lines are the same except for HTML tags, + # so skip over any `<...>` segment + while rich_char == '<' + until rich_char == '>' + rich_pos += 1 + rich_char = rich_line[rich_pos] + end + + rich_pos += 1 + rich_char = rich_line[rich_pos] + end + + # multi-char HTML entities in the rich line correspond to a single character in the raw line + if rich_char == '&' + multichar_mapping = [rich_pos] + until rich_char == ';' + rich_pos += 1 + multichar_mapping << rich_pos + rich_char = rich_line[rich_pos] + end + + mapping[raw_pos] = multichar_mapping + else + mapping[raw_pos] = rich_pos + end + + rich_pos += 1 + end + + mapping + end + end + + # Takes an array of integers, and returns an array of ranges covering the same integers + def collapse_ranges(positions) + return [] if positions.empty? + ranges = [] + + start = prev = positions[0] + range = start..prev + positions[1..-1].each do |pos| + if pos == prev + 1 + range = start..pos + prev = pos + else + ranges << range + start = prev = pos + range = start..prev + end + end + ranges << range + + ranges + end + + # Inserts tags around the characters identified by the given range + def insert_around_range(text, range, before, after, offset = 0) + # Just to be sure + return offset if offset + range.end + 1 > text.length + + text.insert(offset + range.begin, before) + offset += before.length + + text.insert(offset + range.end + 1, after) + offset += after.length + + offset + end + end + end +end diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index 0072194606e..03730b435ad 100644 --- a/lib/gitlab/diff/line.rb +++ b/lib/gitlab/diff/line.rb @@ -1,7 +1,8 @@ module Gitlab module Diff class Line - attr_reader :type, :text, :index, :old_pos, :new_pos + attr_reader :type, :index, :old_pos, :new_pos + attr_accessor :text def initialize(text, type, index, old_pos, new_pos) @text, @type, @index = text, type, index diff --git a/lib/gitlab/diff/parallel_diff.rb b/lib/gitlab/diff/parallel_diff.rb new file mode 100644 index 00000000000..c0db3559e3a --- /dev/null +++ b/lib/gitlab/diff/parallel_diff.rb @@ -0,0 +1,117 @@ +module Gitlab + module Diff + class ParallelDiff + attr_accessor :diff_file + + def initialize(diff_file) + @diff_file = diff_file + end + + def parallelize + 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', 'nonewline', 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 + + private + + def generate_line_code(file_path, line) + Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos) + end + end + end +end diff --git a/lib/gitlab/diff/parser.rb b/lib/gitlab/diff/parser.rb index 516e59b87a3..3666063bf8b 100644 --- a/lib/gitlab/diff/parser.rb +++ b/lib/gitlab/diff/parser.rb @@ -11,13 +11,10 @@ module Gitlab line_new = 1 type = nil - lines_arr = ::Gitlab::InlineDiff.processing lines - - lines_arr.each do |line| + @lines.each do |line| next if filename?(line) - full_line = html_escape(line.gsub(/\n/, '')) - full_line = ::Gitlab::InlineDiff.replace_markers full_line + full_line = line.gsub(/\n/, '') if line.match(/^@@ -/) type = "match" @@ -29,6 +26,10 @@ module Gitlab lines_obj << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) line_obj_index += 1 next + elsif line[0] == '\\' + type = 'nonewline' + lines_obj << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) + line_obj_index += 1 else type = identification_type(line) lines_obj << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) @@ -36,10 +37,13 @@ module Gitlab end - if line[0] == "+" + case line[0] + when "+" line_new += 1 - elsif line[0] == "-" + when "-" line_old += 1 + when "\\" + # No increment else line_new += 1 line_old += 1 @@ -62,19 +66,15 @@ module Gitlab end def identification_type(line) - if line[0] == "+" + case line[0] + when "+" "new" - elsif line[0] == "-" + when "-" "old" else nil end end - - def html_escape(str) - replacements = { '&' => '&', '>' => '>', '<' => '<', '"' => '"', "'" => ''' } - str.gsub(/[&"'><]/, replacements) - end end end end diff --git a/lib/gitlab/highlight.rb b/lib/gitlab/highlight.rb new file mode 100644 index 00000000000..4ddb4fea977 --- /dev/null +++ b/lib/gitlab/highlight.rb @@ -0,0 +1,38 @@ +module Gitlab + class Highlight + def self.highlight(blob_name, blob_content, nowrap: true) + new(blob_name, blob_content, nowrap: nowrap).highlight(blob_content, continue: false) + end + + def self.highlight_lines(repository, ref, file_name) + blob = repository.blob_at(ref, file_name) + return [] unless blob + + highlight(file_name, blob.data).lines.map!(&:html_safe) + end + + def initialize(blob_name, blob_content, nowrap: true) + @formatter = rouge_formatter(nowrap: nowrap) + @lexer = Rouge::Lexer.guess(filename: blob_name, source: blob_content).new rescue Rouge::Lexers::PlainText + end + + def highlight(text, continue: true) + @formatter.format(@lexer.lex(text, continue: continue)).html_safe + rescue + @formatter.format(Rouge::Lexers::PlainText.lex(text)).html_safe + end + + private + + def rouge_formatter(options = {}) + options = options.reverse_merge( + nowrap: true, + cssclass: 'code highlight', + lineanchors: true, + lineanchorsid: 'LC' + ) + + Rouge::Formatters::HTMLGitlab.new(options) + end + end +end diff --git a/lib/gitlab/inline_diff.rb b/lib/gitlab/inline_diff.rb deleted file mode 100644 index 44507bde25d..00000000000 --- a/lib/gitlab/inline_diff.rb +++ /dev/null @@ -1,104 +0,0 @@ -module Gitlab - class InlineDiff - class << self - - START = "#!idiff-start!#" - FINISH = "#!idiff-finish!#" - - def processing(diff_arr) - indexes = _indexes_of_changed_lines diff_arr - - indexes.each do |index| - first_line = diff_arr[index+1] - second_line = diff_arr[index+2] - - # Skip inline diff if empty line was replaced with content - next if first_line == "-\n" - - first_token = find_first_token(first_line, second_line) - apply_first_token(diff_arr, index, first_token) - - last_token = find_last_token(first_line, second_line, first_token) - apply_last_token(diff_arr, index, last_token) - end - - diff_arr - end - - def apply_first_token(diff_arr, index, first_token) - start = first_token + START - - if first_token.empty? - # In case if we remove string of spaces in commit - diff_arr[index+1].sub!("-", "-" => "-#{START}") - diff_arr[index+2].sub!("+", "+" => "+#{START}") - else - diff_arr[index+1].sub!(first_token, first_token => start) - diff_arr[index+2].sub!(first_token, first_token => start) - end - end - - def apply_last_token(diff_arr, index, last_token) - # This is tricky: escape backslashes so that `sub` doesn't interpret them - # as backreferences. Regexp.escape does NOT do the right thing. - replace_token = FINISH + last_token.gsub(/\\/, '\&\&') - diff_arr[index+1].sub!(/#{Regexp.escape(last_token)}$/, replace_token) - diff_arr[index+2].sub!(/#{Regexp.escape(last_token)}$/, replace_token) - end - - def find_first_token(first_line, second_line) - max_length = [first_line.size, second_line.size].max - first_the_same_symbols = 0 - - (0..max_length + 1).each do |i| - first_the_same_symbols = i - 1 - - if first_line[i] != second_line[i] && i > 0 - break - end - end - - first_line[0..first_the_same_symbols][1..-1] - end - - def find_last_token(first_line, second_line, first_token) - max_length = [first_line.size, second_line.size].max - last_the_same_symbols = 0 - - (1..max_length + 1).each do |i| - last_the_same_symbols = -i - shortest_line = second_line.size > first_line.size ? first_line : second_line - - if (first_line[-i] != second_line[-i]) || "#{first_token}#{START}".size == shortest_line[1..-i].size - break - end - end - - last_the_same_symbols += 1 - first_line[last_the_same_symbols..-1] - end - - def _indexes_of_changed_lines(diff_arr) - chain_of_first_symbols = "" - diff_arr.each_with_index do |line, i| - chain_of_first_symbols += line[0] - end - chain_of_first_symbols.gsub!(/[^\-\+]/, "#") - - offset = 0 - indexes = [] - while index = chain_of_first_symbols.index("#-+#", offset) - indexes << index - offset = index + 1 - end - indexes - end - - def replace_markers(line) - line.gsub!(START, "<span class='idiff'>") - line.gsub!(FINISH, "</span>") - line - end - end - end -end diff --git a/spec/fixtures/parallel_diff_result.yml b/spec/fixtures/parallel_diff_result.yml new file mode 100644 index 00000000000..a326b651aad --- /dev/null +++ b/spec/fixtures/parallel_diff_result.yml @@ -0,0 +1,324 @@ +--- +- :left: + :type: match + :number: 6 + :text: "@@ -6,12 +6,18 @@ module Popen" + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 + :right: + :type: match + :number: 6 + :text: "@@ -6,12 +6,18 @@ module Popen" + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 +- :left: + :type: + :number: 6 + :text: |2 + <span id="LC6" class="line"></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 + :right: + :type: + :number: 6 + :text: |2 + <span id="LC6" class="line"></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 +- :left: + :type: + :number: 7 + :text: |2 + <span id="LC7" class="line"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7 + :right: + :type: + :number: 7 + :text: |2 + <span id="LC7" class="line"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7 +- :left: + :type: + :number: 8 + :text: |2 + <span id="LC8" class="line"> <span class="k">unless</span> <span class="n">cmd</span><span class="p">.</span><span class="nf">is_a?</span><span class="p">(</span><span class="no">Array</span><span class="p">)</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8 + :right: + :type: + :number: 8 + :text: |2 + <span id="LC8" class="line"> <span class="k">unless</span> <span class="n">cmd</span><span class="p">.</span><span class="nf">is_a?</span><span class="p">(</span><span class="no">Array</span><span class="p">)</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8 +- :left: + :type: old + :number: 9 + :text: | + -<span id="LC9" class="line"> <span class="k">raise</span> <span class="s2">"System commands must be given as an array of strings"</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9 + :right: + :type: new + :number: 9 + :text: | + +<span id="LC9" class="line"> <span class="k">raise</span> <span class="no"><span class='idiff'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff'> </span><span class="s2">"System commands must be given as an array of strings"</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9 +- :left: + :type: + :number: 10 + :text: |2 + <span id="LC10" class="line"> <span class="k">end</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10 + :right: + :type: + :number: 10 + :text: |2 + <span id="LC10" class="line"> <span class="k">end</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10 +- :left: + :type: + :number: 11 + :text: |2 + <span id="LC11" class="line"></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11 + :right: + :type: + :number: 11 + :text: |2 + <span id="LC11" class="line"></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11 +- :left: + :type: + :number: 12 + :text: |2 + <span id="LC12" class="line"> <span class="n">path</span> <span class="o">||=</span> <span class="no">Dir</span><span class="p">.</span><span class="nf">pwd</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12 + :right: + :type: + :number: 12 + :text: |2 + <span id="LC12" class="line"> <span class="n">path</span> <span class="o">||=</span> <span class="no">Dir</span><span class="p">.</span><span class="nf">pwd</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12 +- :left: + :type: old + :number: 13 + :text: | + -<span id="LC13" class="line"> <span class="n">vars</span> <span class="o">=</span> <span class="p">{</span> <span class="s2">"PWD"</span> <span class="o">=></span> <span class="n">path</span> <span class="p">}</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13 + :right: + :type: old + :number: + :text: '' + :line_code: +- :left: + :type: old + :number: 14 + :text: | + -<span id="LC14" class="line"> <span class="n">options</span> <span class="o">=</span> <span class="p">{</span> <span class="ss">chdir: </span><span class="n">path</span> <span class="p">}</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13 + :right: + :type: new + :number: 13 + :text: | + +<span id="LC13" class="line"></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_13 +- :left: + :type: + :number: + :text: '' + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14 + :right: + :type: new + :number: 14 + :text: | + +<span id="LC14" class="line"> <span class="n">vars</span> <span class="o">=</span> <span class="p">{</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14 +- :left: + :type: + :number: + :text: '' + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15 + :right: + :type: new + :number: 15 + :text: | + +<span id="LC15" class="line"> <span class="s2">"PWD"</span> <span class="o">=></span> <span class="n">path</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15 +- :left: + :type: + :number: + :text: '' + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16 + :right: + :type: new + :number: 16 + :text: | + +<span id="LC16" class="line"> <span class="p">}</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16 +- :left: + :type: + :number: + :text: '' + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17 + :right: + :type: new + :number: 17 + :text: | + +<span id="LC17" class="line"></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17 +- :left: + :type: + :number: + :text: '' + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18 + :right: + :type: new + :number: 18 + :text: | + +<span id="LC18" class="line"> <span class="n">options</span> <span class="o">=</span> <span class="p">{</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18 +- :left: + :type: + :number: + :text: '' + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19 + :right: + :type: new + :number: 19 + :text: | + +<span id="LC19" class="line"> <span class="ss">chdir: </span><span class="n">path</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19 +- :left: + :type: + :number: + :text: '' + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20 + :right: + :type: new + :number: 20 + :text: | + +<span id="LC20" class="line"> <span class="p">}</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20 +- :left: + :type: + :number: 15 + :text: |2 + <span id="LC21" class="line"></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21 + :right: + :type: + :number: 21 + :text: |2 + <span id="LC21" class="line"></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21 +- :left: + :type: + :number: 16 + :text: |2 + <span id="LC22" class="line"> <span class="k">unless</span> <span class="no">File</span><span class="p">.</span><span class="nf">directory?</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22 + :right: + :type: + :number: 22 + :text: |2 + <span id="LC22" class="line"> <span class="k">unless</span> <span class="no">File</span><span class="p">.</span><span class="nf">directory?</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22 +- :left: + :type: + :number: 17 + :text: |2 + <span id="LC23" class="line"> <span class="no">FileUtils</span><span class="p">.</span><span class="nf">mkdir_p</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23 + :right: + :type: + :number: 23 + :text: |2 + <span id="LC23" class="line"> <span class="no">FileUtils</span><span class="p">.</span><span class="nf">mkdir_p</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23 +- :left: + :type: match + :number: 19 + :text: "@@ -19,6 +25,7 @@ module Popen" + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 + :right: + :type: match + :number: 25 + :text: "@@ -19,6 +25,7 @@ module Popen" + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 +- :left: + :type: + :number: 19 + :text: |2 + <span id="LC25" class="line"></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 + :right: + :type: + :number: 25 + :text: |2 + <span id="LC25" class="line"></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 +- :left: + :type: + :number: 20 + :text: |2 + <span id="LC26" class="line"> <span class="vi">@cmd_output</span> <span class="o">=</span> <span class="s2">""</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26 + :right: + :type: + :number: 26 + :text: |2 + <span id="LC26" class="line"> <span class="vi">@cmd_output</span> <span class="o">=</span> <span class="s2">""</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26 +- :left: + :type: + :number: 21 + :text: |2 + <span id="LC27" class="line"> <span class="vi">@cmd_status</span> <span class="o">=</span> <span class="mi">0</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27 + :right: + :type: + :number: 27 + :text: |2 + <span id="LC27" class="line"> <span class="vi">@cmd_status</span> <span class="o">=</span> <span class="mi">0</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27 +- :left: + :type: + :number: + :text: '' + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28 + :right: + :type: new + :number: 28 + :text: | + +<span id="LC28" class="line"></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28 +- :left: + :type: + :number: 22 + :text: |2 + <span id="LC29" class="line"> <span class="no">Open3</span><span class="p">.</span><span class="nf">popen3</span><span class="p">(</span><span class="n">vars</span><span class="p">,</span> <span class="o">*</span><span class="n">cmd</span><span class="p">,</span> <span class="n">options</span><span class="p">)</span> <span class="k">do</span> <span class="o">|</span><span class="n">stdin</span><span class="p">,</span> <span class="n">stdout</span><span class="p">,</span> <span class="n">stderr</span><span class="p">,</span> <span class="n">wait_thr</span><span class="o">|</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29 + :right: + :type: + :number: 29 + :text: |2 + <span id="LC29" class="line"> <span class="no">Open3</span><span class="p">.</span><span class="nf">popen3</span><span class="p">(</span><span class="n">vars</span><span class="p">,</span> <span class="o">*</span><span class="n">cmd</span><span class="p">,</span> <span class="n">options</span><span class="p">)</span> <span class="k">do</span> <span class="o">|</span><span class="n">stdin</span><span class="p">,</span> <span class="n">stdout</span><span class="p">,</span> <span class="n">stderr</span><span class="p">,</span> <span class="n">wait_thr</span><span class="o">|</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29 +- :left: + :type: + :number: 23 + :text: |2 + <span id="LC30" class="line"> <span class="vi">@cmd_output</span> <span class="o"><<</span> <span class="n">stdout</span><span class="p">.</span><span class="nf">read</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30 + :right: + :type: + :number: 30 + :text: |2 + <span id="LC30" class="line"> <span class="vi">@cmd_output</span> <span class="o"><<</span> <span class="n">stdout</span><span class="p">.</span><span class="nf">read</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30 +- :left: + :type: + :number: 24 + :text: |2 + <span id="LC31" class="line"> <span class="vi">@cmd_output</span> <span class="o"><<</span> <span class="n">stderr</span><span class="p">.</span><span class="nf">read</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31 + :right: + :type: + :number: 31 + :text: |2 + <span id="LC31" class="line"> <span class="vi">@cmd_output</span> <span class="o"><<</span> <span class="n">stderr</span><span class="p">.</span><span class="nf">read</span></span> + :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31 diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index b8bba36439a..87849230dbe 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -1,22 +1,22 @@ require 'spec_helper' describe BlobHelper do - describe 'highlight' do - let(:blob_name) { 'test.lisp' } - let(:no_context_content) { ":type \"assem\"))" } - let(:blob_content) { "(make-pathname :defaults name\n#{no_context_content}" } - let(:split_content) { blob_content.split("\n") } - let(:multiline_content) do - %q( - def test(input): - """This is line 1 of a multi-line comment. - This is line 2. - """ - ) - end + let(:blob_name) { 'test.lisp' } + let(:no_context_content) { ":type \"assem\"))" } + let(:blob_content) { "(make-pathname :defaults name\n#{no_context_content}" } + let(:split_content) { blob_content.split("\n") } + let(:multiline_content) do + %q( + def test(input): + """This is line 1 of a multi-line comment. + This is line 2. + """ + ) + end + describe '#highlight' do it 'should return plaintext for unknown lexer context' do - result = highlight(blob_name, no_context_content, nowrap: true, continue: false) + result = helper.highlight(blob_name, no_context_content, nowrap: true) expect(result).to eq('<span id="LC1" class="line">:type "assem"))</span>') end @@ -24,28 +24,17 @@ describe BlobHelper do expected = %Q[<span id="LC1" class="line"><span class="p">(</span><span class="nb">make-pathname</span> <span class="ss">:defaults</span> <span class="nv">name</span></span> <span id="LC2" class="line"><span class="ss">:type</span> <span class="s">"assem"</span><span class="p">))</span></span>] - expect(highlight(blob_name, blob_content, nowrap: true, continue: false)).to eq(expected) - end - - it 'should highlight continued blocks' do - # Both lines have LC1 as ID since formatter doesn't support continue at the moment - expected = [ - '<span id="LC1" class="line"><span class="p">(</span><span class="nb">make-pathname</span> <span class="ss">:defaults</span> <span class="nv">name</span></span>', - '<span id="LC1" class="line"><span class="ss">:type</span> <span class="s">"assem"</span><span class="p">))</span></span>' - ] - - result = split_content.map{ |content| highlight(blob_name, content, nowrap: true, continue: true) } - expect(result).to eq(expected) + expect(helper.highlight(blob_name, blob_content, nowrap: true)).to eq(expected) end it 'should highlight multi-line comments' do - result = highlight(blob_name, multiline_content, nowrap: true, continue: false) + result = helper.highlight(blob_name, multiline_content, nowrap: true) html = Nokogiri::HTML(result) lines = html.search('.s') expect(lines.count).to eq(3) expect(lines[0].text).to eq('"""This is line 1 of a multi-line comment.') - expect(lines[1].text).to eq(' This is line 2.') - expect(lines[2].text).to eq(' """') + expect(lines[1].text).to eq(' This is line 2.') + expect(lines[2].text).to eq(' """') end context 'diff highlighting' do @@ -59,9 +48,23 @@ describe BlobHelper do end it 'should highlight each line properly' do - result = highlight(blob_name, blob_content, nowrap: true, continue: false) + result = helper.highlight(blob_name, blob_content, nowrap: true) expect(result).to eq(expected) end end end + + describe "#highlighter" do + it 'should highlight continued blocks' do + # Both lines have LC1 as ID since formatter doesn't support continue at the moment + expected = [ + '<span id="LC1" class="line"><span class="p">(</span><span class="nb">make-pathname</span> <span class="ss">:defaults</span> <span class="nv">name</span></span>', + '<span id="LC1" class="line"><span class="ss">:type</span> <span class="s">"assem"</span><span class="p">))</span></span>' + ] + + highlighter = helper.highlighter(blob_name, blob_content, nowrap: true) + result = split_content.map{ |content| highlighter.highlight(content) } + expect(result).to eq(expected) + end + end end diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 7c96a74e581..955d2852cfd 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -4,10 +4,12 @@ describe DiffHelper do include RepoHelpers let(:project) { create(:project) } + let(:repository) { project.repository } let(:commit) { project.commit(sample_commit.id) } let(:diffs) { commit.diffs } let(:diff) { diffs.first } - let(:diff_file) { Gitlab::Diff::File.new(diff) } + let(:diff_refs) { [commit.parent, commit] } + let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs) } describe 'diff_hard_limit_enabled?' do it 'should return true if param is provided' do @@ -44,55 +46,41 @@ describe DiffHelper do describe 'safe_diff_files' do it 'should return all files from a commit that is smaller than safe limits' do - expect(safe_diff_files(diffs).length).to eq(2) + expect(safe_diff_files(diffs, diff_refs).length).to eq(2) end it 'should return only the first file if the diff line count in the 2nd file takes the total beyond safe limits' do allow(diffs[1].diff).to receive(:lines).and_return([""] * 4999) #simulate 4999 lines - expect(safe_diff_files(diffs).length).to eq(1) + expect(safe_diff_files(diffs, diff_refs).length).to eq(1) end it 'should return all files from a commit that is beyond safe limit for numbers of lines if force diff is true' do allow(controller).to receive(:params) { { force_show_diff: true } } allow(diffs[1].diff).to receive(:lines).and_return([""] * 4999) #simulate 4999 lines - expect(safe_diff_files(diffs).length).to eq(2) + expect(safe_diff_files(diffs, diff_refs).length).to eq(2) end it 'should return only the first file if the diff line count in the 2nd file takes the total beyond hard limits' do allow(controller).to receive(:params) { { force_show_diff: true } } allow(diffs[1].diff).to receive(:lines).and_return([""] * 49999) #simulate 49999 lines - expect(safe_diff_files(diffs).length).to eq(1) + expect(safe_diff_files(diffs, diff_refs).length).to eq(1) end it 'should return only a safe number of file diffs if a commit touches more files than the safe limits' do large_diffs = diffs * 100 #simulate 200 diffs - expect(safe_diff_files(large_diffs).length).to eq(100) + expect(safe_diff_files(large_diffs, diff_refs).length).to eq(100) end it 'should return all file diffs if a commit touches more files than the safe limits but force diff is true' do allow(controller).to receive(:params) { { force_show_diff: true } } large_diffs = diffs * 100 #simulate 200 diffs - expect(safe_diff_files(large_diffs).length).to eq(200) + expect(safe_diff_files(large_diffs, diff_refs).length).to eq(200) end it 'should return a limited file diffs if a commit touches more files than the hard limits and force diff is true' do allow(controller).to receive(:params) { { force_show_diff: true } } very_large_diffs = diffs * 1000 #simulate 2000 diffs - expect(safe_diff_files(very_large_diffs).length).to eq(1000) - end - end - - describe 'parallel_diff' do - it 'should return an array of arrays containing the parsed diff' do - expect(parallel_diff(diff_file, 0)). - to match_array(parallel_diff_result_array) - end - end - - describe 'generate_line_code' do - it 'should generate correct line code' do - expect(generate_line_code(diff_file.file_path, diff_file.diff_lines.first)). - to eq('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6') + expect(safe_diff_files(very_large_diffs, diff_refs).length).to eq(1000) end end @@ -126,39 +114,11 @@ describe DiffHelper do expect(diff_line_content(diff_file.diff_lines.first.text)). to eq('@@ -6,12 +6,18 @@ module Popen') expect(diff_line_content(diff_file.diff_lines.first.type)).to eq('match') - expect(diff_line_content(diff_file.diff_lines.first.new_pos)).to eq(6) + expect(diff_file.diff_lines.first.new_pos).to eq(6) end - end - def parallel_diff_result_array - [ - ["match", 6, "@@ -6,12 +6,18 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", "match", 6, "@@ -6,12 +6,18 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6"], - [nil, 6, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", nil, 6, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6"], [nil, 7, " def popen(cmd, path=nil)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7", nil, 7, " def popen(cmd, path=nil)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"], - [nil, 8, " unless cmd.is_a?(Array)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8", nil, 8, " unless cmd.is_a?(Array)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"], - ["old", 9, "- raise <span class='idiff'></span>"System commands must be given as an array of strings"", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9", "new", 9, "+ raise <span class='idiff'>RuntimeError, </span>"System commands must be given as an array of strings"", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], - [nil, 10, " end", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10", nil, 10, " end", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"], - [nil, 11, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11", nil, 11, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11"], - [nil, 12, " path ||= Dir.pwd", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12", nil, 12, " path ||= Dir.pwd", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12"], - ["old", 13, "- vars = { "PWD" => path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13", "old", nil, " ", nil], - ["old", 14, "- options = { chdir: path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13", "new", 13, "+", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_13"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14", "new", 14, "+ vars = {", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15", "new", 15, "+ "PWD" => path", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16", "new", 16, "+ }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17", "new", 17, "+", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18", "new", 18, "+ options = {", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19", "new", 19, "+ chdir: path", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20", "new", 20, "+ }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20"], - [nil, 15, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21", nil, 21, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21"], - [nil, 16, " unless File.directory?(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22", nil, 22, " unless File.directory?(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22"], - [nil, 17, " FileUtils.mkdir_p(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23", nil, 23, " FileUtils.mkdir_p(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23"], - ["match", 19, "@@ -19,6 +25,7 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", "match", 25, "@@ -19,6 +25,7 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25"], - [nil, 19, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", nil, 25, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25"], - [nil, 20, " @cmd_output = """, "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26", nil, 26, " @cmd_output = """, "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26"], - [nil, 21, " @cmd_status = 0", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27", nil, 27, " @cmd_status = 0", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28", "new", 28, "+", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28"], - [nil, 22, " Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29", nil, 29, " Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29"], - [nil, 23, " @cmd_output << stdout.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30", nil, 30, " @cmd_output << stdout.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30"], - [nil, 24, " @cmd_output << stderr.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31", nil, 31, " @cmd_output << stderr.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31"] - ] + it 'should return safe HTML' do + expect(diff_line_content(diff_file.diff_lines.first.text)).to be_html_safe + end end end diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index c7cdf8691d6..0d9694f2c13 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Diff::File, lib: true do let(:project) { create(:project) } let(:commit) { project.commit(sample_commit.id) } let(:diff) { commit.diffs.first } - let(:diff_file) { Gitlab::Diff::File.new(diff) } + let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent, commit]) } describe :diff_lines do let(:diff_lines) { diff_file.diff_lines } diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb new file mode 100644 index 00000000000..b84a57f357a --- /dev/null +++ b/spec/lib/gitlab/diff/highlight_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe Gitlab::Diff::Highlight, lib: true do + include RepoHelpers + + let(:project) { create(:project) } + let(:commit) { project.commit(sample_commit.id) } + let(:diff) { commit.diffs.first } + let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent, commit]) } + + describe '#highlight' do + let(:diff_lines) { Gitlab::Diff::Highlight.new(diff_file).highlight } + + it 'should return Gitlab::Diff::Line elements' do + expect(diff_lines.first).to be_an_instance_of(Gitlab::Diff::Line) + end + + it 'should not modify "match" lines' do + expect(diff_lines[0].text).to eq('@@ -6,12 +6,18 @@ module Popen') + expect(diff_lines[22].text).to eq('@@ -19,6 +25,7 @@ module Popen') + end + + it 'should highlight unchanged lines' do + code = %Q{ <span id="LC7" class="line"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span>\n} + + expect(diff_lines[2].text).to eq(code) + end + + it 'should highlight removed lines' do + code = %Q{-<span id="LC9" class="line"> <span class="k">raise</span> <span class="s2">"System commands must be given as an array of strings"</span></span>\n} + + expect(diff_lines[4].text).to eq(code) + end + + it 'should highlight added lines' do + code = %Q{+<span id="LC9" class="line"> <span class="k">raise</span> <span class="no"><span class='idiff'>RuntimeError</span></span><span class="p"><span class='idiff'>,</span></span><span class='idiff'> </span><span class="s2">"System commands must be given as an array of strings"</span></span>\n} + + expect(diff_lines[5].text).to eq(code) + end + end +end diff --git a/spec/lib/gitlab/diff/inline_diff_marker_spec.rb b/spec/lib/gitlab/diff/inline_diff_marker_spec.rb new file mode 100644 index 00000000000..6f3276a8b53 --- /dev/null +++ b/spec/lib/gitlab/diff/inline_diff_marker_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe Gitlab::Diff::InlineDiffMarker, lib: true do + describe '#inline_diffs' do + let(:raw) { "abc 'def'" } + let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">'def'</span>} } + let(:inline_diffs) { [2..5] } + + let(:subject) { Gitlab::Diff::InlineDiffMarker.new(raw, rich).mark(inline_diffs) } + + it 'marks the inline diffs' do + expect(subject).to eq(%{<span class="abc">ab<span class='idiff'>c</span></span><span class="space"><span class='idiff'> </span></span><span class="def"><span class='idiff'>'d</span>ef'</span>}) + end + end +end diff --git a/spec/lib/gitlab/diff/inline_diff_spec.rb b/spec/lib/gitlab/diff/inline_diff_spec.rb new file mode 100644 index 00000000000..056917df893 --- /dev/null +++ b/spec/lib/gitlab/diff/inline_diff_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe Gitlab::Diff::InlineDiff, lib: true do + describe '#inline_diffs' do + let(:diff) do + <<eos + class Test +- def initialize(test = true) ++ def initialize(test = false) + @test = test + end + end +eos + end + + let(:subject) { Gitlab::Diff::InlineDiff.new(diff.lines).inline_diffs } + + it 'finds all inline diffs' do + expect(subject[0]).to be_nil + expect(subject[1]).to eq([25..27]) + expect(subject[2]).to eq([25..28]) + expect(subject[3]).to be_nil + expect(subject[4]).to be_nil + expect(subject[5]).to be_nil + end + end +end diff --git a/spec/lib/gitlab/diff/parallel_diff_spec.rb b/spec/lib/gitlab/diff/parallel_diff_spec.rb new file mode 100644 index 00000000000..1c5bbc47120 --- /dev/null +++ b/spec/lib/gitlab/diff/parallel_diff_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Gitlab::Diff::ParallelDiff, lib: true do + include RepoHelpers + + let(:project) { create(:project) } + let(:repository) { project.repository } + let(:commit) { project.commit(sample_commit.id) } + let(:diffs) { commit.diffs } + let(:diff) { diffs.first } + let(:diff_refs) { [commit.parent, commit] } + let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs) } + subject { described_class.new(diff_file) } + + let(:parallel_diff_result_array) { YAML.load_file("#{Rails.root}/spec/fixtures/parallel_diff_result.yml") } + + describe '#parallelize' do + it 'should return an array of arrays containing the parsed diff' do + expect(subject.parallelize).to match_array(parallel_diff_result_array) + end + end +end diff --git a/spec/lib/gitlab/diff/parser_spec.rb b/spec/lib/gitlab/diff/parser_spec.rb index ba577bd28e5..fe0dea77909 100644 --- a/spec/lib/gitlab/diff/parser_spec.rb +++ b/spec/lib/gitlab/diff/parser_spec.rb @@ -86,7 +86,7 @@ eos it { expect(line.type).to eq(nil) } it { expect(line.old_pos).to eq(24) } it { expect(line.new_pos).to eq(31) } - it { expect(line.text).to eq(' @cmd_output << stderr.read') } + it { expect(line.text).to eq(' @cmd_output << stderr.read') } end end end diff --git a/spec/lib/gitlab/highlight_spec.rb b/spec/lib/gitlab/highlight_spec.rb new file mode 100644 index 00000000000..1620eb6c60a --- /dev/null +++ b/spec/lib/gitlab/highlight_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::Highlight, lib: true do + include RepoHelpers + + let(:project) { create(:project) } + let(:commit) { project.commit(sample_commit.id) } + + describe '.highlight_lines' do + let(:lines) do + Gitlab::Highlight.highlight_lines(project.repository, commit.id, 'files/ruby/popen.rb') + end + + it 'should properly highlight all the lines' do + expect(lines[4]).to eq(%Q{<span id="LC5" class="line"> <span class="kp">extend</span> <span class="nb">self</span></span>\n}) + expect(lines[21]).to eq(%Q{<span id="LC22" class="line"> <span class="k">unless</span> <span class="no">File</span><span class="p">.</span><span class="nf">directory?</span><span class="p">(</span><span class="n">path</span><span class="p">)</span></span>\n}) + expect(lines[26]).to eq(%Q{<span id="LC27" class="line"> <span class="vi">@cmd_status</span> <span class="o">=</span> <span class="mi">0</span></span>\n}) + end + end + +end diff --git a/spec/lib/gitlab/inline_diff_spec.rb b/spec/lib/gitlab/inline_diff_spec.rb deleted file mode 100644 index c690c195112..00000000000 --- a/spec/lib/gitlab/inline_diff_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -require 'spec_helper' - -describe Gitlab::InlineDiff, lib: true do - describe '#processing' do - let(:diff) do - <<eos ---- a/test.rb -+++ b/test.rb -@@ -1,6 +1,6 @@ - class Test - def cleanup_string(input) - return nil if input.nil? -- input.sub(/[\\r\\n].+/,'').sub(/\\\\[rn].+/, '').strip -+ input.to_s.sub(/[\\r\\n].+/,'').sub(/\\\\[rn].+/, '').strip - end - end -eos - end - - let(:expected) do - ["--- a/test.rb\n", - "+++ b/test.rb\n", - "@@ -1,6 +1,6 @@\n", - " class Test\n", - " def cleanup_string(input)\n", - " return nil if input.nil?\n", - "- input.#!idiff-start!##!idiff-finish!#sub(/[\\r\\n].+/,'').sub(/\\\\[rn].+/, '').strip\n", - "+ input.#!idiff-start!#to_s.#!idiff-finish!#sub(/[\\r\\n].+/,'').sub(/\\\\[rn].+/, '').strip\n", - " end\n", - " end\n"] - end - - let(:subject) { Gitlab::InlineDiff.processing(diff.lines) } - - it 'should retain backslashes' do - expect(subject).to eq(expected) - end - end -end diff --git a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb index 449cecaa789..de9fed2b7dd 100644 --- a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb +++ b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb @@ -6,7 +6,7 @@ describe MergeRequests::MergeWhenBuildSucceedsService do let(:mr_merge_if_green_enabled) do create(:merge_request, merge_when_build_succeeds: true, merge_user: user, - source_branch: "source_branch", target_branch: project.default_branch, + source_branch: "master", target_branch: 'feature', source_project: project, target_project: project, state: "opened") end |