summaryrefslogtreecommitdiff
path: root/lib/gitlab/diff
diff options
context:
space:
mode:
Diffstat (limited to 'lib/gitlab/diff')
-rw-r--r--lib/gitlab/diff/file.rb79
-rw-r--r--lib/gitlab/diff/file_collection/base.rb25
-rw-r--r--lib/gitlab/diff/file_collection/merge_request_diff.rb3
-rw-r--r--lib/gitlab/diff/highlight.rb6
-rw-r--r--lib/gitlab/diff/line.rb4
-rw-r--r--lib/gitlab/diff/position.rb30
-rw-r--r--lib/gitlab/diff/position_tracer.rb216
7 files changed, 230 insertions, 133 deletions
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index c6bf25b5874..2aef7fdaa35 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -1,16 +1,17 @@
module Gitlab
module Diff
class File
- attr_reader :diff, :repository, :diff_refs
+ attr_reader :diff, :repository, :diff_refs, :fallback_diff_refs
- delegate :new_file, :deleted_file, :renamed_file,
- :old_path, :new_path, :a_mode, :b_mode,
+ delegate :new_file?, :deleted_file?, :renamed_file?,
+ :old_path, :new_path, :a_mode, :b_mode, :mode_changed?,
:submodule?, :too_large?, :collapsed?, to: :diff, prefix: false
- def initialize(diff, repository:, diff_refs: nil)
+ def initialize(diff, repository:, diff_refs: nil, fallback_diff_refs: nil)
@diff = diff
@repository = repository
@diff_refs = diff_refs
+ @fallback_diff_refs = fallback_diff_refs
end
def position(line)
@@ -49,24 +50,60 @@ module Gitlab
line_code(line) if line
end
+ def old_sha
+ diff_refs&.base_sha
+ end
+
+ def new_sha
+ diff_refs&.head_sha
+ end
+
+ def content_sha
+ return old_content_sha if deleted_file?
+ return @content_sha if defined?(@content_sha)
+
+ refs = diff_refs || fallback_diff_refs
+ @content_sha = refs&.head_sha
+ end
+
def content_commit
- return unless diff_refs
+ return @content_commit if defined?(@content_commit)
+
+ sha = content_sha
+ @content_commit = repository.commit(sha) if sha
+ end
+
+ def old_content_sha
+ return if new_file?
+ return @old_content_sha if defined?(@old_content_sha)
- repository.commit(deleted_file ? old_ref : new_ref)
+ refs = diff_refs || fallback_diff_refs
+ @old_content_sha = refs&.base_sha
end
def old_content_commit
- return unless diff_refs
+ return @old_content_commit if defined?(@old_content_commit)
- repository.commit(old_ref)
+ sha = old_content_sha
+ @old_content_commit = repository.commit(sha) if sha
end
- def old_ref
- diff_refs.try(:base_sha)
+ def blob
+ return @blob if defined?(@blob)
+
+ sha = content_sha
+ return @blob = nil unless sha
+
+ repository.blob_at(sha, file_path)
end
- def new_ref
- diff_refs.try(:head_sha)
+ def old_blob
+ return @old_blob if defined?(@old_blob)
+
+ sha = old_content_sha
+ return @old_blob = nil unless sha
+
+ @old_blob = repository.blob_at(sha, old_path)
end
attr_writer :highlighted_diff_lines
@@ -85,10 +122,6 @@ module Gitlab
@parallel_diff_lines ||= Gitlab::Diff::ParallelDiff.new(self).parallelize
end
- def mode_changed?
- a_mode && b_mode && a_mode != b_mode
- end
-
def raw_diff
diff.diff.to_s
end
@@ -117,20 +150,8 @@ module Gitlab
diff_lines.count(&:removed?)
end
- def old_blob(commit = old_content_commit)
- return unless commit
-
- repository.blob_at(commit.id, old_path)
- end
-
- def blob(commit = content_commit)
- return unless commit
-
- repository.blob_at(commit.id, file_path)
- end
-
def file_identifier
- "#{file_path}-#{new_file}-#{deleted_file}-#{renamed_file}"
+ "#{file_path}-#{new_file?}-#{deleted_file?}-#{renamed_file?}"
end
end
end
diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb
index 2b9fc65b985..a6007ebf531 100644
--- a/lib/gitlab/diff/file_collection/base.rb
+++ b/lib/gitlab/diff/file_collection/base.rb
@@ -2,32 +2,41 @@ module Gitlab
module Diff
module FileCollection
class Base
- attr_reader :project, :diff_options, :diff_view, :diff_refs
+ attr_reader :project, :diff_options, :diff_refs, :fallback_diff_refs
delegate :count, :size, :real_size, to: :diff_files
def self.default_options
- ::Commit.max_diff_options.merge(ignore_whitespace_change: false, no_collapse: false)
+ ::Commit.max_diff_options.merge(ignore_whitespace_change: false, expanded: false)
end
- def initialize(diffable, project:, diff_options: nil, diff_refs: nil)
+ def initialize(diffable, project:, diff_options: nil, diff_refs: nil, fallback_diff_refs: nil)
diff_options = self.class.default_options.merge(diff_options || {})
- @diffable = diffable
- @diffs = diffable.raw_diffs(diff_options)
- @project = project
+ @diffable = diffable
+ @diffs = diffable.raw_diffs(diff_options)
+ @project = project
@diff_options = diff_options
- @diff_refs = diff_refs
+ @diff_refs = diff_refs
+ @fallback_diff_refs = fallback_diff_refs
end
def diff_files
@diff_files ||= @diffs.decorate! { |diff| decorate_diff!(diff) }
end
+ def diff_file_with_old_path(old_path)
+ diff_files.find { |diff_file| diff_file.old_path == old_path }
+ end
+
+ def diff_file_with_new_path(new_path)
+ diff_files.find { |diff_file| diff_file.new_path == new_path }
+ end
+
private
def decorate_diff!(diff)
- Gitlab::Diff::File.new(diff, repository: project.repository, diff_refs: diff_refs)
+ Gitlab::Diff::File.new(diff, repository: project.repository, diff_refs: diff_refs, fallback_diff_refs: fallback_diff_refs)
end
end
end
diff --git a/lib/gitlab/diff/file_collection/merge_request_diff.rb b/lib/gitlab/diff/file_collection/merge_request_diff.rb
index 0bd226ef050..9a58b500a2c 100644
--- a/lib/gitlab/diff/file_collection/merge_request_diff.rb
+++ b/lib/gitlab/diff/file_collection/merge_request_diff.rb
@@ -8,7 +8,8 @@ module Gitlab
super(merge_request_diff,
project: merge_request_diff.project,
diff_options: diff_options,
- diff_refs: merge_request_diff.diff_refs)
+ diff_refs: merge_request_diff.diff_refs,
+ fallback_diff_refs: merge_request_diff.fallback_diff_refs)
end
def diff_files
diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb
index 7db896522a9..ed2f541977a 100644
--- a/lib/gitlab/diff/highlight.rb
+++ b/lib/gitlab/diff/highlight.rb
@@ -3,7 +3,7 @@ module Gitlab
class Highlight
attr_reader :diff_file, :diff_lines, :raw_lines, :repository
- delegate :old_path, :new_path, :old_ref, :new_ref, to: :diff_file, prefix: :diff
+ delegate :old_path, :new_path, :old_sha, :new_sha, to: :diff_file, prefix: :diff
def initialize(diff_lines, repository: nil)
@repository = repository
@@ -61,12 +61,12 @@ module Gitlab
def old_lines
return unless diff_file
- @old_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_old_ref, diff_old_path)
+ @old_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_old_sha, diff_old_path)
end
def new_lines
return unless diff_file
- @new_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_new_ref, diff_new_path)
+ @new_lines ||= Gitlab::Highlight.highlight_lines(self.repository, diff_new_sha, diff_new_path)
end
end
end
diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb
index 0a15c6d9358..bd52ae47e9f 100644
--- a/lib/gitlab/diff/line.rb
+++ b/lib/gitlab/diff/line.rb
@@ -59,6 +59,10 @@ module Gitlab
type == 'match'
end
+ def discussable?
+ !['match', 'new-nonewline', 'old-nonewline'].include?(type)
+ end
+
def as_json(opts = nil)
{
type: type,
diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb
index fc728123c97..4d96778a2b2 100644
--- a/lib/gitlab/diff/position.rb
+++ b/lib/gitlab/diff/position.rb
@@ -12,20 +12,26 @@ module Gitlab
attr_reader :head_sha
def initialize(attrs = {})
+ if diff_file = attrs[:diff_file]
+ attrs[:diff_refs] = diff_file.diff_refs
+ attrs[:old_path] = diff_file.old_path
+ attrs[:new_path] = diff_file.new_path
+ end
+
+ if diff_refs = attrs[:diff_refs]
+ attrs[:base_sha] = diff_refs.base_sha
+ attrs[:start_sha] = diff_refs.start_sha
+ attrs[:head_sha] = diff_refs.head_sha
+ end
+
@old_path = attrs[:old_path]
@new_path = attrs[:new_path]
+ @base_sha = attrs[:base_sha]
+ @start_sha = attrs[:start_sha]
+ @head_sha = attrs[:head_sha]
+
@old_line = attrs[:old_line]
@new_line = attrs[:new_line]
-
- if attrs[:diff_refs]
- @base_sha = attrs[:diff_refs].base_sha
- @start_sha = attrs[:diff_refs].start_sha
- @head_sha = attrs[:diff_refs].head_sha
- else
- @base_sha = attrs[:base_sha]
- @start_sha = attrs[:start_sha]
- @head_sha = attrs[:head_sha]
- end
end
# `Gitlab::Diff::Position` objects are stored as serialized attributes in
@@ -129,11 +135,11 @@ module Gitlab
end
def diff_line(repository)
- @diff_line ||= diff_file(repository).line_for_position(self)
+ @diff_line ||= diff_file(repository)&.line_for_position(self)
end
def line_code(repository)
- @line_code ||= diff_file(repository).line_code_for_position(self)
+ @line_code ||= diff_file(repository)&.line_code_for_position(self)
end
private
diff --git a/lib/gitlab/diff/position_tracer.rb b/lib/gitlab/diff/position_tracer.rb
index e89ff238ec7..dcabb5f7fe5 100644
--- a/lib/gitlab/diff/position_tracer.rb
+++ b/lib/gitlab/diff/position_tracer.rb
@@ -3,21 +3,21 @@
module Gitlab
module Diff
class PositionTracer
- attr_accessor :repository
+ attr_accessor :project
attr_accessor :old_diff_refs
attr_accessor :new_diff_refs
attr_accessor :paths
- def initialize(repository:, old_diff_refs:, new_diff_refs:, paths: nil)
- @repository = repository
+ def initialize(project:, old_diff_refs:, new_diff_refs:, paths: nil)
+ @project = project
@old_diff_refs = old_diff_refs
@new_diff_refs = new_diff_refs
@paths = paths
end
- def trace(old_position)
+ def trace(ab_position)
return unless old_diff_refs&.complete? && new_diff_refs&.complete?
- return unless old_position.diff_refs == old_diff_refs
+ return unless ab_position.diff_refs == old_diff_refs
# Suppose we have an MR with source branch `feature` and target branch `master`.
# When the MR was created, the head of `master` was commit A, and the
@@ -44,14 +44,16 @@ module Gitlab
#
# For diff notes for diff A->B, the position looks like this:
# Position
- # base_sha - ID of commit A
+ # start_sha - ID of commit A
# head_sha - ID of commit B
+ # base_sha - ID of base commit of A and B
# old_path - path as of A (nil if file was newly created)
# new_path - path as of B (nil if file was deleted)
# old_line - line number as of A (nil if file was newly created)
# new_line - line number as of B (nil if file was deleted)
#
- # We can easily update `base_sha` and `head_sha` to hold the IDs of commits C and D,
+ # We can easily update `start_sha` and `head_sha` to hold the IDs of
+ # commits C and D, and can trivially determine `base_sha` based on those,
# but need to find the paths and line numbers as of C and D.
#
# If the file was unchanged or newly created in A->B, the path as of D can be found
@@ -68,107 +70,161 @@ module Gitlab
# by generating diff A->C ("base to base"), finding the diff file with
# `diff_file.old_path == position.old_path`, and taking `diff_file.new_path`.
# The path as of D can be found by taking diff C->D, finding the diff file
- # with that same `old_path` and taking `diff_file.new_path`.
+ # with `old_path` set to that `diff_file.new_path` and taking `diff_file.new_path`.
# The line number as of C can be found by using the LineMapper on diff A->C
# and providing the line number as of A.
# The line number as of D can be found by using the LineMapper on diff C->D
# and providing the line number as of C.
- results = nil
- results ||= trace_added_line(old_position) if old_position.added? || old_position.unchanged?
- results ||= trace_removed_line(old_position) if old_position.removed? || old_position.unchanged?
-
- return unless results
-
- file_diff, old_line, new_line = results
-
- new_position = Position.new(
- old_path: file_diff.old_path,
- new_path: file_diff.new_path,
- head_sha: new_diff_refs.head_sha,
- start_sha: new_diff_refs.start_sha,
- base_sha: new_diff_refs.base_sha,
- old_line: old_line,
- new_line: new_line
- )
-
- # If a position is found, but is not actually contained in the diff, for example
- # because it was an unchanged line in the context of a change that was undone,
- # we cannot return this as a successful trace.
- return unless new_position.diff_line(repository)
-
- new_position
+ if ab_position.added?
+ trace_added_line(ab_position)
+ elsif ab_position.removed?
+ trace_removed_line(ab_position)
+ else # unchanged
+ trace_unchanged_line(ab_position)
+ end
end
private
- def trace_added_line(old_position)
- file_path = old_position.new_path
-
- return unless diff_head_to_head
-
- file_head_to_head = diff_head_to_head.find { |diff_file| diff_file.old_path == file_path }
-
- file_path = file_head_to_head.new_path if file_head_to_head
-
- new_line = LineMapper.new(file_head_to_head).old_to_new(old_position.new_line)
-
- return unless new_line
-
- file_diff = new_diffs.find { |diff_file| diff_file.new_path == file_path }
- return unless file_diff
-
- old_line = LineMapper.new(file_diff).new_to_old(new_line)
-
- [file_diff, old_line, new_line]
+ def trace_added_line(ab_position)
+ b_path = ab_position.new_path
+ b_line = ab_position.new_line
+
+ bd_diff = bd_diffs.diff_file_with_old_path(b_path)
+
+ d_path = bd_diff&.new_path || b_path
+ d_line = LineMapper.new(bd_diff).old_to_new(b_line)
+
+ if d_line
+ cd_diff = cd_diffs.diff_file_with_new_path(d_path)
+
+ c_path = cd_diff&.old_path || d_path
+ c_line = LineMapper.new(cd_diff).new_to_old(d_line)
+
+ if c_line
+ # If the line is still in D but also in C, it has turned from an
+ # added line into an unchanged one.
+ new_position = position(cd_diff, c_line, d_line)
+ if valid_position?(new_position)
+ # If the line is still in the MR, we don't treat this as outdated.
+ { position: new_position, outdated: false }
+ else
+ # If the line is no longer in the MR, we unfortunately cannot show
+ # the current state on the CD diff, so we treat it as outdated.
+ ac_diff = ac_diffs.diff_file_with_new_path(c_path)
+
+ { position: position(ac_diff, nil, c_line), outdated: true }
+ end
+ else
+ # If the line is still in D and not in C, it is still added.
+ { position: position(cd_diff, nil, d_line), outdated: false }
+ end
+ else
+ # If the line is no longer in D, it has been removed from the MR.
+ { position: position(bd_diff, b_line, nil), outdated: true }
+ end
end
- def trace_removed_line(old_position)
- file_path = old_position.old_path
+ def trace_removed_line(ab_position)
+ a_path = ab_position.old_path
+ a_line = ab_position.old_line
- return unless diff_base_to_base
+ ac_diff = ac_diffs.diff_file_with_old_path(a_path)
- file_base_to_base = diff_base_to_base.find { |diff_file| diff_file.old_path == file_path }
+ c_path = ac_diff&.new_path || a_path
+ c_line = LineMapper.new(ac_diff).old_to_new(a_line)
- file_path = file_base_to_base.old_path if file_base_to_base
+ if c_line
+ cd_diff = cd_diffs.diff_file_with_old_path(c_path)
- old_line = LineMapper.new(file_base_to_base).old_to_new(old_position.old_line)
+ d_path = cd_diff&.new_path || c_path
+ d_line = LineMapper.new(cd_diff).old_to_new(c_line)
- return unless old_line
+ if d_line
+ # If the line is still in C but also in D, it has turned from a
+ # removed line into an unchanged one.
+ bd_diff = bd_diffs.diff_file_with_new_path(d_path)
- file_diff = new_diffs.find { |diff_file| diff_file.old_path == file_path }
- return unless file_diff
-
- new_line = LineMapper.new(file_diff).old_to_new(old_line)
+ { position: position(bd_diff, nil, d_line), outdated: true }
+ else
+ # If the line is still in C and not in D, it is still removed.
+ { position: position(cd_diff, c_line, nil), outdated: false }
+ end
+ else
+ # If the line is no longer in C, it has been removed outside of the MR.
+ { position: position(ac_diff, a_line, nil), outdated: true }
+ end
+ end
- [file_diff, old_line, new_line]
+ def trace_unchanged_line(ab_position)
+ a_path = ab_position.old_path
+ a_line = ab_position.old_line
+ b_path = ab_position.new_path
+ b_line = ab_position.new_line
+
+ ac_diff = ac_diffs.diff_file_with_old_path(a_path)
+
+ c_path = ac_diff&.new_path || a_path
+ c_line = LineMapper.new(ac_diff).old_to_new(a_line)
+
+ bd_diff = bd_diffs.diff_file_with_old_path(b_path)
+
+ d_line = LineMapper.new(bd_diff).old_to_new(b_line)
+
+ cd_diff = cd_diffs.diff_file_with_old_path(c_path)
+
+ if c_line && d_line
+ # If the line is still in C and D, it is still unchanged.
+ new_position = position(cd_diff, c_line, d_line)
+ if valid_position?(new_position)
+ # If the line is still in the MR, we don't treat this as outdated.
+ { position: new_position, outdated: false }
+ else
+ # If the line is no longer in the MR, we unfortunately cannot show
+ # the current state on the CD diff or any change on the BD diff,
+ # so we treat it as outdated.
+ { position: nil, outdated: true }
+ end
+ elsif d_line # && !c_line
+ # If the line is still in D but no longer in C, it has turned from
+ # an unchanged line into an added one.
+ # We don't treat this as outdated since the line is still in the MR.
+ { position: position(cd_diff, nil, d_line), outdated: false }
+ else # !d_line && (c_line || !c_line)
+ # If the line is no longer in D, it has turned from an unchanged line
+ # into a removed one.
+ { position: position(bd_diff, b_line, nil), outdated: true }
+ end
end
- def diff_base_to_base
- @diff_base_to_base ||= diff_files(old_diff_refs.base_sha || old_diff_refs.start_sha, new_diff_refs.base_sha || new_diff_refs.start_sha)
+ def ac_diffs
+ @ac_diffs ||= compare(
+ old_diff_refs.base_sha || old_diff_refs.start_sha,
+ new_diff_refs.base_sha || new_diff_refs.start_sha,
+ straight: true
+ )
end
- def diff_head_to_head
- @diff_head_to_head ||= diff_files(old_diff_refs.head_sha, new_diff_refs.head_sha)
+ def bd_diffs
+ @bd_diffs ||= compare(old_diff_refs.head_sha, new_diff_refs.head_sha, straight: true)
end
- def new_diffs
- @new_diffs ||= diff_files(new_diff_refs.start_sha, new_diff_refs.head_sha, use_base: true)
+ def cd_diffs
+ @cd_diffs ||= compare(new_diff_refs.start_sha, new_diff_refs.head_sha)
end
- def diff_files(start_sha, head_sha, use_base: false)
- base_sha = self.repository.merge_base(start_sha, head_sha) || start_sha
+ def compare(start_sha, head_sha, straight: false)
+ compare = CompareService.new(project, head_sha).execute(project, start_sha, straight: straight)
+ compare.diffs(paths: paths)
+ end
- diffs = self.repository.raw_repository.diff(
- use_base ? base_sha : start_sha,
- head_sha,
- {},
- *paths
- )
+ def position(diff_file, old_line, new_line)
+ Position.new(diff_file: diff_file, old_line: old_line, new_line: new_line)
+ end
- diffs.decorate! do |diff|
- Gitlab::Diff::File.new(diff, repository: self.repository)
- end
+ def valid_position?(position)
+ !!position.diff_line(project.repository)
end
end
end