diff --git a/app/assets/javascripts/merge_conflict_resolver.js.coffee b/app/assets/javascripts/merge_conflict_resolver.js.coffee index 04fbb8666bf17f14ac01a94142b2bb0f0faa028c..47ff8b7825b4078b5acce35e41ed561afa36db54 100644 --- a/app/assets/javascripts/merge_conflict_resolver.js.coffee +++ b/app/assets/javascripts/merge_conflict_resolver.js.coffee @@ -21,9 +21,9 @@ class window.MergeConflictResolver extends Vue fetchData: -> $.ajax - url : '/emojis' + url : './conflicts.json' success: (response) => - @handleResponse window.mergeConflictsData + @handleResponse response handleResponse: (data) -> @@ -81,6 +81,8 @@ class window.MergeConflictResolver extends Vue headHeaderText = 'HEAD//our changes' originHeaderText = 'origin//their changes' + data.shortCommitSha = data.commit_sha.slice 0, 7 + data.commitMessage = data.commit_message @updateResolutionsData data @@ -88,7 +90,6 @@ class window.MergeConflictResolver extends Vue for file in data.files file.parallelLines = { left: [], right: [] } file.inlineLines = [] - file.shortCommitSha = file.commit_sha.slice 0, 7 currentLineType = 'old' for section in file.sections @@ -96,41 +97,42 @@ class window.MergeConflictResolver extends Vue if conflict # FIXME: Make these lines better - file.parallelLines.left.push { isHeader: yes, id, text: headHeaderText, section: 'head', isHead: yes, isSelected: no, isUnselected: no } - file.parallelLines.right.push { isHeader: yes, id, text: originHeaderText, section: 'origin', isOrigin: yes, isSelected: no, isUnselected: no } + file.parallelLines.left.push { isHeader: yes, id, richText: headHeaderText, section: 'head', isHead: yes, isSelected: no, isUnselected: no } + file.parallelLines.right.push { isHeader: yes, id, richText: originHeaderText, section: 'origin', isOrigin: yes, isSelected: no, isUnselected: no } - file.inlineLines.push { isHeader: yes, id, text: headHeaderText, type: 'old', section: 'head', isHead: yes, isSelected: no, isUnselected: no } + file.inlineLines.push { isHeader: yes, id, richText: headHeaderText, type: 'old', section: 'head', isHead: yes, isSelected: no, isUnselected: no } for line in lines if line.type in ['new', 'old'] and currentLineType isnt line.type currentLineType = line.type # FIXME: Find a better way to add a new line - file.inlineLines.push { lineType: 'emptyLine', text: '<span> </span>' } + file.inlineLines.push { lineType: 'emptyLine', richText: '<span> </span>' } # FIXME: Make these lines better line.conflict = conflict line.id = id - line.isHead = line.type is 'old' - line.isOrigin = line.type is 'new' + line.isHead = line.type is 'new' + line.isOrigin = line.type is 'old' line.isSelected = no line.isUnselected = no + line.richText = line.rich_text file.inlineLines.push line if conflict if line.type is 'old' - line = { lineType: 'conflict', lineNumber: line.old_line, text: line.text, section: 'head', id, isSelected: no, isUnselected: no, isHead: yes } + line = { lineType: 'conflict', lineNumber: line.old_line, richText: line.rich_text, section: 'origin', id, isSelected: no, isUnselected: no, isOrigin: yes } file.parallelLines.left.push line else if line.type is 'new' - line = { lineType: 'conflict', lineNumber: line.new_line, text: line.text, section: 'origin', id, isSelected: no, isUnselected: no, isOrigin: yes } + line = { lineType: 'conflict', lineNumber: line.new_line, richText: line.rich_text, section: 'head', id, isSelected: no, isUnselected: no, isHead: yes } file.parallelLines.right.push line else console.log 'unhandled line type...', line else - file.parallelLines.left.push { lineType: 'context', lineNumber: line.old_line, text: line.text } - file.parallelLines.right.push { lineType: 'context', lineNumber: line.new_line, text: line.text } + file.parallelLines.left.push { lineType: 'context', lineNumber: line.old_line, richText: line.rich_text } + file.parallelLines.right.push { lineType: 'context', lineNumber: line.new_line, richText: line.rich_text } if conflict - file.inlineLines.push { isHeader: yes, id, type: 'new', text: originHeaderText, section: 'origin', isOrigin: yes, isSelected: no, isUnselected: no } + file.inlineLines.push { isHeader: yes, id, type: 'new', richText: originHeaderText, section: 'origin', isOrigin: yes, isSelected: no, isUnselected: no } return data diff --git a/app/assets/javascripts/merge_conflicts_data.coffee b/app/assets/javascripts/merge_conflicts_data.coffee index a302424f421d6fbfa8f53e4e74772b0f82d17aeb..7d89a87cf14628d1afb1ae7509f9f8c7c6dd90f2 100644 --- a/app/assets/javascripts/merge_conflicts_data.coffee +++ b/app/assets/javascripts/merge_conflicts_data.coffee @@ -1,9 +1,9 @@ window.mergeConflictsData = { "target_branch": "master", "source_branch": "mc-ui", + "commit_sha": "b5fa56eb3f2cea5e21c68b43c7c22b5b96e0e7b3", "files": [ { - "commit_sha": "b5fa56eb3f2cea5e21c68b43c7c22b5b96e0e7b3", "old_path": "lib/component.js", "new_path": "lib/component.js", "sections": [ diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 4fa7d97101308d19766bcd15cac029344a4956cf..689c9627762a367f0b909d7fbf134b8403f6f276 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -17,7 +17,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController before_action :define_widget_vars, only: [:merge, :cancel_merge_when_build_succeeds, :merge_check] before_action :define_commit_vars, only: [:diffs] before_action :define_diff_comment_vars, only: [:diffs] - before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds] + before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts] # Allow read any merge_request before_action :authorize_read_merge_request! @@ -130,10 +130,38 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end - def conflicts + return render_404 unless @merge_request.cannot_be_merged? + respond_to do |format| format.html { render 'show' } + format.json do + rugged = @merge_request.project.repository.rugged + their_commit = rugged.branches[@merge_request.target_branch].target + our_commit = rugged.lookup(@merge_request.diff_head_sha) + merge = rugged.merge_commits(our_commit, their_commit) + commit_message = "Merge branch '#{@merge_request.target_branch}' into '#{@merge_request.source_branch}'\n\n# Conflicts:" + + files = merge.conflicts.map do |conflict| + their_path = conflict[:theirs][:path] + our_path = conflict[:ours][:path] + + raise 'path mismatch!' unless their_path == our_path + + commit_message << "\n# #{our_path}" + merge_file = merge.merge_file(our_path) + + Gitlab::Conflict::File.new(merge_file, conflict, @merge_request.target_branch, @merge_request.source_branch, @merge_request.project.repository) + end + + render json: { + target_branch: @merge_request.target_branch, + source_branch: @merge_request.source_branch, + commit_sha: @merge_request.diff_head_sha, + commit_message: commit_message, + files: files + } + end end end diff --git a/app/views/projects/merge_requests/show/_conflicts.html.haml b/app/views/projects/merge_requests/show/_conflicts.html.haml index 9aa4e17a64928bf5ed56f700f166763a58bd1027..d40f231cf53c5b29a6009dd350e134ab0de17130 100644 --- a/app/views/projects/merge_requests/show/_conflicts.html.haml +++ b/app/views/projects/merge_requests/show/_conflicts.html.haml @@ -28,7 +28,7 @@ .file-title %span {{file.new_path}} .file-actions - %a.btn.btn-sm View file @{{file.shortCommitSha}} + %a.btn.btn-sm View file @{{conflictsData.shortCommitShab}} .diff-content.diff-wrap-lines .diff-wrap-lines.code.file-content.js-syntax-highlight.white @@ -57,7 +57,7 @@ .file-title %span {{file.new_path}} .file-actions - %a.btn.btn-sm View file @{{file.shortCommitSha}} + %a.btn.btn-sm View file @{{conflictsData.shortCommitSha}} .diff-content.diff-wrap-lines .diff-wrap-lines.code.file-content.js-syntax-highlight.white @@ -75,13 +75,13 @@ %td.diff-line-num.new_line %a {{line.new_line}} %td.line_content - {{{line.text}}} + {{{line.rich_text}}} %template{"v-if" => "line.isHeader"} %td.diff-line-num.header %td.diff-line-num.header %td.line_content.header - %strong {{{line.text}}} + %strong {{{line.richText}}} %button.btn{"@click" => "handleSelected(line.id, line.section)"} Use this .content-block.oneline-block.files-changed @@ -92,7 +92,7 @@ .commit-message-container.form-group .max-width-marker - %textarea.form-control.js-commit-message{"placeholder" => 'Your commit message', ":disabled" => "!allResolved"} + %textarea.form-control.js-commit-message{":disabled" => "!allResolved"} {{{conflictsData.commitMessage}}} %button{type: 'button', class: 'btn btn-success js-submit-button', ":disabled" => "!allResolved"} Commit conflict resolution diff --git a/lib/gitlab/conflict/file.rb b/lib/gitlab/conflict/file.rb new file mode 100644 index 0000000000000000000000000000000000000000..84d3e6f4e036d2a3972a92ebd2b961196b7059f5 --- /dev/null +++ b/lib/gitlab/conflict/file.rb @@ -0,0 +1,91 @@ +module Gitlab + module Conflict + class File + CONTEXT_LINES = 3 + + attr_reader :merge_file, :their_path, :their_ref, :our_path, :our_ref, :repository + + def initialize(merge_file, conflict, their_ref, our_ref, repository) + @merge_file = merge_file + @their_path = conflict[:theirs][:path] + @our_path = conflict[:ours][:path] + @their_ref = their_ref + @our_ref = our_ref + @repository = repository + end + + # Array of Gitlab::Diff::Line objects + def lines + @lines ||= Gitlab::Conflict::Parser.new.parse(merge_file[:data], their_path, our_path) + end + + def highlighted_lines + return @highlighted_lines if @highlighted_lines + + their_highlight = Gitlab::Highlight.highlight_lines(repository, their_ref, their_path) + our_highlight = Gitlab::Highlight.highlight_lines(repository, our_ref, our_path) + + @highlighted_lines = lines.map do |line| + line = line.dup + if line.type == 'old' + line.rich_text = their_highlight[line.old_line - 1].delete("\n") + else + line.rich_text = our_highlight[line.new_line - 1].delete("\n") + end + line + end + end + + def sections + return @sections if @sections + + chunked_lines = highlighted_lines.chunk { |line| line.type.nil? } + match_line = nil + + @sections = chunked_lines.flat_map.with_index do |(no_conflict, lines), i| + section = nil + + if no_conflict + conflict_before = i > 0 + conflict_after = chunked_lines.peek + + if conflict_before && conflict_after + if lines.length > CONTEXT_LINES * 2 + tail_lines = lines.last(CONTEXT_LINES) + first_tail_line = tail_lines.first + match_line = Gitlab::Diff::Line.new('', + 'match', + first_tail_line.index, + first_tail_line.old_pos, + first_tail_line.new_pos) + + section = [ + { conflict: false, lines: lines.first(CONTEXT_LINES) }, + { conflict: false, lines: tail_lines.unshift(match_line) } + ] + end + elsif conflict_after + lines = lines.last(CONTEXT_LINES) + elsif conflict_before + lines = lines.first(CONTEXT_LINES) + end + end + + if match_line && !section + match_line.text = "@@ -#{match_line.old_pos},#{lines.last.old_pos} +#{match_line.new_pos},#{lines.last.new_pos} @@" + end + + section || { conflict: !no_conflict, lines: lines } + end + end + + def as_json(opts = nil) + { + old_path: their_path, + new_path: our_path, + sections: sections + } + end + end + end +end diff --git a/lib/gitlab/conflict/parser.rb b/lib/gitlab/conflict/parser.rb new file mode 100644 index 0000000000000000000000000000000000000000..a233c268070e82c2fc62f965be7df8e65a68110a --- /dev/null +++ b/lib/gitlab/conflict/parser.rb @@ -0,0 +1,59 @@ +module Gitlab + module Conflict + class Parser + class UnexpectedDelimiter < StandardError + end + + class MissingEndDelimiter < StandardError + end + + def parse(text, their_path, our_path) + return [] if text.blank? + + line_obj_index = 0 + line_old = 1 + line_new = 1 + type = nil + lines = [] + conflict_start = "<<<<<<< #{our_path}" + conflict_middle = '=======' + conflict_end = ">>>>>>> #{their_path}" + + text.each_line.map do |line| + full_line = line.delete("\n") + + if full_line == conflict_start + raise UnexpectedDelimiter unless type.nil? + + type = 'new' + elsif full_line == conflict_middle + raise UnexpectedDelimiter unless type == 'new' + + type = 'old' + elsif full_line == conflict_end + raise UnexpectedDelimiter unless type == 'old' + + type = nil + elsif line[0] == '\\' + type = 'nonewline' + lines << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) + else + lines << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) + line_old += 1 if type != 'new' + line_new += 1 if type != 'old' + + line_obj_index += 1 + end + end + + raise MissingEndDelimiter unless type == nil + + lines + end + + def empty? + @lines.empty? + end + end + end +end diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index cf097e0d0dec32f6f782efeca98be06e30328638..5196051f90db0aa65d7b7989db571300bb375e5b 100644 --- a/lib/gitlab/diff/line.rb +++ b/lib/gitlab/diff/line.rb @@ -3,6 +3,7 @@ module Gitlab class Line attr_reader :type, :index, :old_pos, :new_pos attr_accessor :text + attr_accessor :rich_text def initialize(text, type, index, old_pos, new_pos) @text, @type, @index = text, type, index @@ -46,6 +47,16 @@ module Gitlab def meta? type == 'match' || type == 'nonewline' end + + def as_json(opts = nil) + { + type: type, + old_line: old_line, + new_line: new_line, + text: text, + rich_text: rich_text || text + } + end end end end diff --git a/spec/lib/gitlab/conflict/file_spec.rb b/spec/lib/gitlab/conflict/file_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..318d0d249d6e87ffc19e51b22781e85ccba4c156 --- /dev/null +++ b/spec/lib/gitlab/conflict/file_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' + +describe Gitlab::Conflict::File, lib: true do + let(:project) { create(:project) } + let(:repository) { project.repository } + let(:rugged) { repository.rugged } + let(:their_ref) { their_commit.oid } + let(:their_commit) { rugged.branches['conflict-a'].target } + let(:our_ref) { our_commit.oid } + let(:our_commit) { rugged.branches['conflict-b'].target } + let(:index) { rugged.merge_commits(our_commit, their_commit) } + let(:conflict) { index.conflicts.last } + let(:merge_file) { index.merge_file('files/ruby/regex.rb') } + let(:conflict_file) { Gitlab::Conflict::File.new(merge_file, conflict, their_ref, our_ref, repository) } + + describe '#highlighted_lines' do + def html_to_text(html) + CGI.unescapeHTML(ActionView::Base.full_sanitizer.sanitize(html)) + end + + it 'returns lines with rich_text' do + conflict_file.highlighted_lines.each do |line| + expect(line).to have_attributes(rich_text: an_instance_of(String)) + end + end + + it 'returns lines with rich_text matching the text content of the line' do + conflict_file.highlighted_lines.each do |line| + expect(line.text).to eq(html_to_text(line.rich_text)) + end + end + end + + describe '#sections' do + it 'returns match lines when there is a gap between sections' do + section = conflict_file.sections[5] + match_line = section[:lines][0] + + expect(section[:conflict]).to be_falsey + expect(match_line.type).to eq('match') + expect(match_line.text).to eq('@@ -46,53 +46,53 @@') + end + + it 'does not return match lines when there is no gap between sections' do + conflict_file.sections.each_with_index do |section, i| + unless i == 5 + expect(section[:lines][0].type).not_to eq(5) + end + end + end + + it 'sets conflict to false for sections with only unchanged lines' do + conflict_file.sections.reject { |section| section[:conflict] }.each do |section| + without_match = section[:lines].reject { |line| line.type == 'match' } + + expect(without_match).to all(have_attributes(type: nil)) + end + end + + it 'only includes a maximum of CONTEXT_LINES (plus an optional match line) in context sections' do + conflict_file.sections.reject { |section| section[:conflict] }.each do |section| + without_match = section[:lines].reject { |line| line.type == 'match' } + + expect(without_match.length).to be <= Gitlab::Conflict::File::CONTEXT_LINES * 2 + end + end + + it 'sets conflict to true for sections with only changed lines' do + conflict_file.sections.select { |section| section[:conflict] }.each do |section| + section[:lines].each do |line| + expect(line.type).to be_in(['new', 'old']) + end + end + end + end +end diff --git a/spec/lib/gitlab/conflict/parser_spec.rb b/spec/lib/gitlab/conflict/parser_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..335da58e13c58984e8e1381662c8d71966a6e210 --- /dev/null +++ b/spec/lib/gitlab/conflict/parser_spec.rb @@ -0,0 +1,178 @@ +require 'spec_helper' + +describe Gitlab::Conflict::Parser, lib: true do + let(:parser) { Gitlab::Conflict::Parser.new } + + describe '#parse' do + context 'when the file has valid conflicts' do + let(:text) do + <<CONFLICT +module Gitlab + module Regexp + extend self + + def username_regexp + default_regexp + end + +<<<<<<< files/ruby/regex.rb + def project_name_regexp + /\A[a-zA-Z0-9][a-zA-Z0-9_\-\. ]*\z/ + end + + def name_regexp + /\A[a-zA-Z0-9_\-\. ]*\z/ +======= + def project_name_regex + %r{\A[a-zA-Z0-9][a-zA-Z0-9_\-\. ]*\z} + end + + def name_regex + %r{\A[a-zA-Z0-9_\-\. ]*\z} +>>>>>>> files/ruby/regex.rb + end + + def path_regexp + default_regexp + end + +<<<<<<< files/ruby/regex.rb + def archive_formats_regexp + /(zip|tar|7z|tar\.gz|tgz|gz|tar\.bz2|tbz|tbz2|tb2|bz2)/ +======= + def archive_formats_regex + %r{(zip|tar|7z|tar\.gz|tgz|gz|tar\.bz2|tbz|tbz2|tb2|bz2)} +>>>>>>> files/ruby/regex.rb + end + + def git_reference_regexp + # Valid git ref regexp, see: + # https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html + %r{ + (?! + (?# doesn't begins with) + \/| (?# rule #6) + (?# doesn't contain) + .*(?: + [\/.]\.| (?# rule #1,3) + \/\/| (?# rule #6) + @\{| (?# rule #8) + \\ (?# rule #9) + ) + ) + [^\000-\040\177~^:?*\[]+ (?# rule #4-5) + (?# doesn't end with) + (?<!\.lock) (?# rule #1) + (?<![\/.]) (?# rule #6-7) + }x + end + + protected + +<<<<<<< files/ruby/regex.rb + def default_regexp + /\A[.?]?[a-zA-Z0-9][a-zA-Z0-9_\-\.]*(?<!\.git)\z/ +======= + def default_regex + %r{\A[.?]?[a-zA-Z0-9][a-zA-Z0-9_\-\.]*(?<!\.git)\z} +>>>>>>> files/ruby/regex.rb + end + end +end +CONFLICT + end + + let(:lines) { parser.parse(text, 'files/ruby/regex.rb', 'files/ruby/regex.rb') } + + it 'sets our lines as new lines' do + expect(lines[8..13]).to all(have_attributes(type: 'new')) + expect(lines[26..27]).to all(have_attributes(type: 'new')) + expect(lines[56..57]).to all(have_attributes(type: 'new')) + end + + it 'sets their lines as old lines' do + expect(lines[14..19]).to all(have_attributes(type: 'old')) + expect(lines[28..29]).to all(have_attributes(type: 'old')) + expect(lines[58..59]).to all(have_attributes(type: 'old')) + end + + it 'sets non-conflicted lines as both' do + expect(lines[0..7]).to all(have_attributes(type: nil)) + expect(lines[20..25]).to all(have_attributes(type: nil)) + expect(lines[30..55]).to all(have_attributes(type: nil)) + expect(lines[60..62]).to all(have_attributes(type: nil)) + end + + it 'sets consecutive line numbers for index, old_pos, and new_pos' do + old_line_numbers = lines.select { |line| line.type != 'new' }.map(&:old_pos) + new_line_numbers = lines.select { |line| line.type != 'old' }.map(&:new_pos) + + expect(lines.map(&:index)).to eq(0.upto(62).to_a) + expect(old_line_numbers).to eq(1.upto(53).to_a) + expect(new_line_numbers).to eq(1.upto(53).to_a) + end + end + + context 'when the file contents include conflict delimiters' do + let(:path) { 'README.md' } + + def parse_text(text) + parser.parse(text, path, path) + end + + it 'raises UnexpectedDelimiter when there is a non-start delimiter first' do + expect { parse_text('=======') }. + to raise_error(Gitlab::Conflict::Parser::UnexpectedDelimiter) + + expect { parse_text('>>>>>>> README.md') }. + to raise_error(Gitlab::Conflict::Parser::UnexpectedDelimiter) + + expect { parse_text('>>>>>>> some-other-path.md') }. + not_to raise_error + end + + it 'raises UnexpectedDelimiter when a start delimiter is followed by a non-middle delimiter' do + start_text = "<<<<<<< README.md\n" + end_text = "\n=======\n>>>>>>> README.md" + + expect { parse_text(start_text + '>>>>>>> README.md' + end_text) }. + to raise_error(Gitlab::Conflict::Parser::UnexpectedDelimiter) + + expect { parse_text(start_text + start_text + end_text) }. + to raise_error(Gitlab::Conflict::Parser::UnexpectedDelimiter) + + expect { parse_text(start_text + '>>>>>>> some-other-path.md' + end_text) }. + not_to raise_error + end + + it 'raises UnexpectedDelimiter when a middle delimiter is followed by a non-end delimiter' do + start_text = "<<<<<<< README.md\n=======\n" + end_text = "\n>>>>>>> README.md" + + expect { parse_text(start_text + '=======' + end_text) }. + to raise_error(Gitlab::Conflict::Parser::UnexpectedDelimiter) + + expect { parse_text(start_text + start_text + end_text) }. + to raise_error(Gitlab::Conflict::Parser::UnexpectedDelimiter) + + expect { parse_text(start_text + '>>>>>>> some-other-path.md' + end_text) }. + not_to raise_error + end + + it 'raises MissingEndDelimiter when there is no end delimiter at the end' do + start_text = "<<<<<<< README.md\n=======\n" + + expect { parse_text(start_text) }. + to raise_error(Gitlab::Conflict::Parser::MissingEndDelimiter) + + expect { parse_text(start_text + '>>>>>>> some-other-path.md') }. + to raise_error(Gitlab::Conflict::Parser::MissingEndDelimiter) + end + end + + context 'when lines is blank' do + it { expect(parser.parse('', 'README.md', 'README.md')).to eq([]) } + it { expect(parser.parse(nil, 'README.md', 'README.md')).to eq([]) } + end + end +end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 1c0c66969e3bece8bd82183029bc4857a449c60f..976b1f362705c46069d94f91abc413b352625140 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -19,6 +19,8 @@ module TestEnv 'orphaned-branch' => '45127a9', 'binary-encoding' => '7b1cf43', 'gitattributes' => '5a62481', + 'conflict-a' => 'dfdd207', + 'conflict-b' => '4e75a62', 'expand-collapse-diffs' => '4842455', 'expand-collapse-files' => '025db92', 'expand-collapse-lines' => '238e82d',