Commit be4359ef authored by Marin Jankovski's avatar Marin Jankovski

Merge branch 'refactor-diff-yeild' into 'parallel_diff_refactor'

Refactor diffs

This refactoring done on top of Marin improvements to side-by-side diff.
Main target of this refactoring is to reduce mess and increase test coverage.

1. Use objects:
  * Gitlab::Diff::Line for line change in diff
  * Gitlab::Diff::File for file changes (diff + blob info)
2. Avoid `yield` magic
3. Move logic from views/helper to models/libs
4. Move diff views under separate dir `app/views/projects/diffs`

See merge request !1072
parents 205358b1 93dc8855
...@@ -31,7 +31,7 @@ class Projects::EditTreeController < Projects::BaseTreeController ...@@ -31,7 +31,7 @@ class Projects::EditTreeController < Projects::BaseTreeController
diffy = Diffy::Diff.new(@blob.data, @content, diff: '-U 3', diffy = Diffy::Diff.new(@blob.data, @content, diff: '-U 3',
include_diff_info: true) include_diff_info: true)
@diff = Gitlab::DiffParser.new(diffy.diff.scan(/.*\n/)) @diff_lines = Gitlab::Diff::Parser.new.parse(diffy.diff.scan(/.*\n/))
render layout: false render layout: false
end end
......
...@@ -16,87 +16,6 @@ module CommitsHelper ...@@ -16,87 +16,6 @@ module CommitsHelper
commit_person_link(commit, options.merge(source: :committer)) commit_person_link(commit, options.merge(source: :committer))
end end
def each_diff_line(diff, index)
Gitlab::DiffParser.new(diff.diff.lines.to_a, diff.new_path)
.each do |full_line, type, line_code, line_new, line_old|
yield(full_line, type, line_code, line_new, line_old)
end
end
def parallel_diff_line(diff, index)
Gitlab::DiffParser.new(diff.diff.lines.to_a, diff.new_path)
.each do |full_line, type, line_code, line_new, line_old, raw_line, next_type, next_line|
yield(full_line, type, line_code, line_new, line_old, raw_line, next_type, next_line)
end
end
def parallel_diff(diff, index)
lines = []
skip_next = false
# Building array of lines
#
# [left_type, left_line_number, left_line_content, right_line_type, right_line_number, right_line_content]
#
parallel_diff_line(diff, index) do |full_line, type, line_code, line_new, line_old, raw_line, next_type, next_line|
line = [type, line_old, full_line, next_type, line_new]
if type == 'match' || type.nil?
# line in the right panel is the same as in the left one
line = [type, line_old, full_line, type, line_new, full_line]
lines.push(line)
elsif type == 'old'
if next_type == 'new'
# Left side has text removed, right side has text added
line.push(next_line)
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
line.pop # remove the newline
line.push(nil) # no line number on the right panel
line.push("&nbsp;") # empty line on the right panel
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, "&nbsp;", type, line_new, full_line]
lines.push(line)
end
end
end
lines
end
def each_diff_line_near(diff, index, expected_line_code)
max_number_of_lines = 16
prev_match_line = nil
prev_lines = []
each_diff_line(diff, index) do |full_line, type, line_code, line_new, line_old|
line = [full_line, type, line_code, line_new, line_old]
if line_code != expected_line_code
if type == "match"
prev_lines.clear
prev_match_line = line
else
prev_lines.push(line)
prev_lines.shift if prev_lines.length >= max_number_of_lines
end
else
yield(prev_match_line) if !prev_match_line.nil?
prev_lines.each { |ln| yield(ln) }
yield(line)
break
end
end
end
def image_diff_class(diff) def image_diff_class(diff)
if diff.deleted_file if diff.deleted_file
"deleted" "deleted"
...@@ -202,10 +121,6 @@ module CommitsHelper ...@@ -202,10 +121,6 @@ module CommitsHelper
end end
end end
def diff_file_mode_changed?(diff)
diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode
end
def unfold_bottom_class(bottom) def unfold_bottom_class(bottom)
(bottom) ? 'js-unfold-bottom' : '' (bottom) ? 'js-unfold-bottom' : ''
end end
......
module DiffHelper module DiffHelper
def safe_diff_files(diffs) def allowed_diff_size
if diff_hard_limit_enabled? if diff_hard_limit_enabled?
diffs.first(Commit::DIFF_HARD_LIMIT_FILES) Commit::DIFF_HARD_LIMIT_FILES
else else
diffs.first(Commit::DIFF_SAFE_FILES) Commit::DIFF_SAFE_FILES
end
end
def safe_diff_files(diffs)
diffs.first(allowed_diff_size).map do |diff|
Gitlab::Diff::File.new(diff)
end end
end end
def show_diff_size_warninig?(diffs) def show_diff_size_warninig?(diffs)
safe_diff_files(diffs).size < diffs.size diffs.size > allowed_diff_size
end end
def diff_hard_limit_enabled? def diff_hard_limit_enabled?
...@@ -19,4 +25,65 @@ module DiffHelper ...@@ -19,4 +25,65 @@ module DiffHelper
false false
end end
end end
def generate_line_code(file_path, line)
Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos)
end
def parallel_diff(diff_file, index)
lines = []
skip_next = false
# Building array of lines
#
# [left_type, left_line_number, left_line_content, right_line_type, right_line_number, right_line_content]
#
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_type = next_line.type
next_line = next_line.text
end
line = [type, line_old, full_line, next_type, line_new]
if type == 'match' || type.nil?
# line in the right panel is the same as in the left one
line = [type, line_old, full_line, type, line_new, full_line]
lines.push(line)
elsif type == 'old'
if next_type == 'new'
# Left side has text removed, right side has text added
line.push(next_line)
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
line.pop # remove the newline
line.push(nil) # no line number on the right panel
line.push("&nbsp;") # empty line on the right panel
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, "&nbsp;", type, line_new, full_line]
lines.push(line)
end
end
end
lines
end
end end
...@@ -209,9 +209,10 @@ class Note < ActiveRecord::Base ...@@ -209,9 +209,10 @@ class Note < ActiveRecord::Base
noteable.diffs.each do |mr_diff| noteable.diffs.each do |mr_diff|
next unless mr_diff.new_path == self.diff.new_path next unless mr_diff.new_path == self.diff.new_path
Gitlab::DiffParser.new(mr_diff.diff.lines.to_a, mr_diff.new_path). lines = Gitlab::Diff::Parser.new.parse(mr_diff.diff.lines.to_a)
each do |full_line, type, line_code, line_new, line_old|
if full_line == diff_line lines.each do |line|
if line.text == diff_line
return true return true
end end
end end
...@@ -232,6 +233,14 @@ class Note < ActiveRecord::Base ...@@ -232,6 +233,14 @@ class Note < ActiveRecord::Base
diff.new_path if diff diff.new_path if diff
end end
def file_path
if diff.new_path.present?
diff.new_path
elsif diff.old_path.present?
diff.old_path
end
end
def diff_old_line def diff_old_line
line_code.split('_')[1].to_i line_code.split('_')[1].to_i
end end
...@@ -240,19 +249,49 @@ class Note < ActiveRecord::Base ...@@ -240,19 +249,49 @@ class Note < ActiveRecord::Base
line_code.split('_')[2].to_i line_code.split('_')[2].to_i
end end
def generate_line_code(line)
Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos)
end
def diff_line def diff_line
return @diff_line if @diff_line return @diff_line if @diff_line
if diff if diff
Gitlab::DiffParser.new(diff.diff.lines.to_a, diff.new_path) diff_lines.each do |line|
.each do |full_line, type, line_code, line_new, line_old| if generate_line_code(line) == self.line_code
@diff_line = full_line if line_code == self.line_code @diff_line = line.text
end end
end
end end
@diff_line @diff_line
end end
def truncated_diff_lines
max_number_of_lines = 16
prev_match_line = nil
prev_lines = []
diff_lines.each do |line|
if generate_line_code(line) != self.line_code
if line.type == "match"
prev_lines.clear
prev_match_line = line
else
prev_lines.push(line)
prev_lines.shift if prev_lines.length >= max_number_of_lines
end
else
prev_lines << line
return prev_lines
end
end
end
def diff_lines
@diff_lines ||= Gitlab::Diff::Parser.new.parse(diff.diff.lines.to_a)
end
def discussion_id def discussion_id
@discussion_id ||= Note.build_discussion_id(noteable_type, noteable_id || commit_id, line_code) @discussion_id ||= Note.build_discussion_id(noteable_type, noteable_id || commit_id, line_code)
end end
......
= render "commit_box" = render "commit_box"
= render "projects/commits/diffs", diffs: @diffs, project: @project = render "projects/diffs/diffs", diffs: @diffs, project: @project
= render "projects/notes/notes_with_form" = render "projects/notes/notes_with_form"
...@@ -18,7 +18,7 @@ ...@@ -18,7 +18,7 @@
- else - else
%ul.well-list= render Commit.decorate(@commits), project: @project %ul.well-list= render Commit.decorate(@commits), project: @project
= render "projects/commits/diffs", diffs: @diffs, project: @project = render "projects/diffs/diffs", diffs: @diffs, project: @project
- else - else
.light-well .light-well
......
.row .row
.col-md-8 .col-md-8
= render 'projects/commits/diff_stats', diffs: diffs = render 'projects/diffs/stats', diffs: diffs
.col-md-4 .col-md-4
%ul.nav.nav-tabs %ul.nav.nav-tabs
%li.pull-right{class: params[:view] == 'parallel' ? 'active' : ''} %li.pull-right{class: params[:view] == 'parallel' ? 'active' : ''}
...@@ -11,12 +11,13 @@ ...@@ -11,12 +11,13 @@
- params_copy[:view] = 'inline' - params_copy[:view] = 'inline'
= link_to "Inline Diff", url_for(params_copy), {id: "commit-diff-viewtype"} = link_to "Inline Diff", url_for(params_copy), {id: "commit-diff-viewtype"}
- if show_diff_size_warninig?(diffs) - if show_diff_size_warninig?(diffs)
= render 'projects/commits/diff_warning', diffs: diffs = render 'projects/diffs/warning', diffs: diffs
.files .files
- safe_diff_files(diffs).each_with_index do |diff, i| - safe_diff_files(diffs).each_with_index do |diff_file, index|
= render 'projects/commits/diff_file', diff: diff, i: i, project: project = render 'projects/diffs/file', diff_file: diff_file, i: index, project: project
- if @diff_timeout - if @diff_timeout
.alert.alert-danger .alert.alert-danger
......
- file = project.repository.blob_for_diff(@commit, diff) - blob = project.repository.blob_for_diff(@commit, diff_file.diff)
- return unless file - return unless blob
- blob_diff_path = diff_project_blob_path(project, - blob_diff_path = diff_project_blob_path(project, tree_join(@commit.id, diff_file.file_path))
tree_join(@commit.id, diff.new_path))
.diff-file{id: "diff-#{i}", data: {blob_diff_path: blob_diff_path }} .diff-file{id: "diff-#{i}", data: {blob_diff_path: blob_diff_path }}
.diff-header{id: "file-path-#{hexdigest(diff.new_path || diff.old_path)}"} .diff-header{id: "file-path-#{hexdigest(diff_file.new_path || diff_file.old_path)}"}
- if diff.deleted_file - if diff_file.deleted_file
%span= diff.old_path %span= diff_file.old_path
.diff-btn-group .diff-btn-group
- if @commit.parent_ids.present? - if @commit.parent_ids.present?
= view_file_btn(@commit.parent_id, diff, project) = view_file_btn(@commit.parent_id, diff_file, project)
- else - else
%span= diff.new_path %span= diff_file.new_path
- if diff_file_mode_changed?(diff) - if diff_file.mode_changed?
%span.file-mode= "#{diff.a_mode}#{diff.b_mode}" %span.file-mode= "#{diff.a_mode}#{diff.b_mode}"
.diff-btn-group .diff-btn-group
...@@ -26,23 +25,23 @@ ...@@ -26,23 +25,23 @@
&nbsp; &nbsp;
- if @merge_request && @merge_request.source_project - if @merge_request && @merge_request.source_project
= link_to project_edit_tree_path(@merge_request.source_project, tree_join(@merge_request.source_branch, diff.new_path), from_merge_request_id: @merge_request.id), { class: 'btn btn-small' } do = link_to project_edit_tree_path(@merge_request.source_project, tree_join(@merge_request.source_branch, diff_file.new_path), from_merge_request_id: @merge_request.id), { class: 'btn btn-small' } do
Edit Edit
&nbsp; &nbsp;
= view_file_btn(@commit.id, diff, project) = view_file_btn(@commit.id, diff_file, project)
.diff-content .diff-content
-# Skipp all non non-supported blobs -# Skipp all non non-supported blobs
- return unless file.respond_to?('text?') - return unless blob.respond_to?('text?')
- if file.text? - if blob.text?
- if params[:view] == 'parallel' - if params[:view] == 'parallel'
= render "projects/commits/parallel_view", diff: diff, project: project, file: file, index: i = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i
- else - else
= render "projects/commits/text_file", diff: diff, index: i = render "projects/diffs/text_file", diff_file: diff_file, index: i
- elsif file.image? - elsif blob.image?
- old_file = project.repository.prev_blob_for_diff(@commit, diff) - old_file = project.repository.prev_blob_for_diff(@commit, diff_file)
= render "projects/commits/image", diff: diff, old_file: old_file, file: file, index: i = render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i
- else - else
.nothing-here-block No preview for this file type .nothing-here-block No preview for this file type
- diff = diff_file.diff
- if diff.renamed_file || diff.new_file || diff.deleted_file - if diff.renamed_file || diff.new_file || diff.deleted_file
.image .image
%span.wrap %span.wrap
......
/ Side-by-side diff view / Side-by-side diff view
%div.text-file %div.text-file
%table %table
- parallel_diff(diff, index).each do |line| - parallel_diff(diff_file, index).each do |line|
- type_left = line[0] - type_left = line[0]
- line_number_left = line[1] - line_number_left = line[1]
- line_content_left = line[2] - line_content_left = line[2]
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
%tr.line_holder.parallel %tr.line_holder.parallel
- if type_left == 'match' - if type_left == 'match'
= render "projects/commits/diffs/match_line_parallel", { line: line_content_left, = render "projects/diffs/match_line_parallel", { line: line_content_left,
line_old: line_number_left, line_new: line_number_right } line_old: line_number_left, line_new: line_number_right }
- elsif type_left == 'old' || type_left.nil? - elsif type_left == 'old' || type_left.nil?
%td.old_line{class: "#{type_left}"} %td.old_line{class: "#{type_left}"}
...@@ -21,6 +21,6 @@ ...@@ -21,6 +21,6 @@
= link_to raw(line_number_right) = link_to raw(line_number_right)
%td.line_content.parallel{class: "noteable_line #{type_right == 'new' ? 'new' : nil}"}= raw line_content_right %td.line_content.parallel{class: "noteable_line #{type_right == 'new' ? 'new' : nil}"}= raw line_content_right
- if diff.diff.blank? && diff_file_mode_changed?(diff) - if diff_file.diff.diff.blank? && diff_file.mode_changed?
.file-mode-changed .file-mode-changed
File mode changed File mode changed
- too_big = diff.diff.lines.count > Commit::DIFF_SAFE_LINES - too_big = diff_file.diff_lines.count > Commit::DIFF_SAFE_LINES
- if too_big - if too_big
%a.supp_diff_link Changes suppressed. Click to show %a.supp_diff_link Changes suppressed. Click to show
%table.text-file{class: "#{'hide' if too_big}"} %table.text-file{class: "#{'hide' if too_big}"}
- last_line = 0 - last_line = 0
- each_diff_line(diff, index) do |line, type, line_code, line_new, line_old, raw_line| - diff_file.diff_lines.each_with_index do |line, index|
- last_line = line_new - type = line.type
- last_line = line.new_pos
- line_code = generate_line_code(diff_file.file_path, line)
- line_old = line.old_pos
%tr.line_holder{ id: line_code, class: "#{type}" } %tr.line_holder{ id: line_code, class: "#{type}" }
- if type == "match" - if type == "match"
= render "projects/commits/diffs/match_line", {line: line, = render "projects/diffs/match_line", {line: line.text,
line_old: line_old, line_new: line_new, bottom: false} line_old: line_old, line_new: line.new_pos, bottom: false}
- else - else
%td.old_line %td.old_line
= link_to raw(type == "new" ? "&nbsp;" : line_old), "##{line_code}", id: line_code = link_to raw(type == "new" ? "&nbsp;" : line_old), "##{line_code}", id: line_code
- if @comments_allowed - if @comments_allowed
= link_to_new_diff_note(line_code) = link_to_new_diff_note(line_code)
%td.new_line{data: {linenumber: line_new}} %td.new_line{data: {linenumber: line.new_pos}}
= link_to raw(type == "old" ? "&nbsp;" : line_new) , "##{line_code}", id: line_code = link_to raw(type == "old" ? "&nbsp;" : 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) %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line.text)
- if @reply_allowed - if @reply_allowed
- comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at) - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at)
- unless comments.empty? - unless comments.empty?
= render "projects/notes/diff_notes_with_reply", notes: comments, line: line = render "projects/notes/diff_notes_with_reply", notes: comments, line: line.text
- if last_line > 0 - if last_line > 0
= render "projects/commits/diffs/match_line", {line: "", = render "projects/diffs/match_line", {line: "",
line_old: last_line, line_new: last_line, bottom: true} line_old: last_line, line_new: last_line, bottom: true}
- if diff.diff.blank? && diff_file_mode_changed?(diff) - if diff_file.diff.blank? && diff_file.mode_changed?
.file-mode-changed .file-mode-changed
File mode changed File mode changed
...@@ -14,6 +14,6 @@ ...@@ -14,6 +14,6 @@
= link_to "Email patch", project_merge_request_path(@project, @merge_request, format: :patch), class: "btn btn-warning btn-small" = link_to "Email patch", project_merge_request_path(@project, @merge_request, format: :patch), class: "btn btn-warning btn-small"
%p %p
To preserve performance only To preserve performance only
%strong #{safe_diff_files(diffs).size} of #{diffs.size} %strong #{allowed_diff_size} of #{diffs.size}
files displayed. files displayed.
%table.text-file
- each_diff_line(diff, 1) do |line, type, line_code, line_new, line_old, raw_line|
%tr.line_holder{ id: line_code, class: "#{type}" }
- if type == "match"
%td.old_line= "..."
%td.new_line= "..."
%td.line_content.matched= line
- else
%td.old_line
= link_to raw(type == "new" ? "&nbsp;" : line_old), "##{line_code}", id: line_code
%td.new_line= link_to raw(type == "old" ? "&nbsp;" : line_new) , "##{line_code}", id: line_code
%td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line)
...@@ -9,18 +9,17 @@ ...@@ -9,18 +9,17 @@
= raw render_markup(@blob.name, @content) = raw render_markup(@blob.name, @content)
- else - else
.file-content.code .file-content.code
- unless @diff.empty? - unless @diff_lines.empty?
%table.text-file %table.text-file
- @diff.each do |line, type, line_code, line_new, line_old, raw_line| - @diff_lines.each do |line|
%tr.line_holder{ id: line_code, class: "#{type}" } %tr.line_holder{ class: "#{line.type}" }
- if type == "match" - if line.type == "match"
%td.old_line= "..." %td.old_line= "..."
%td.new_line= "..." %td.new_line= "..."
%td.line_content.matched= line %td.line_content.matched= line.text
- else - else
%td.old_line %td.old_line
= link_to raw(type == "new" ? "&nbsp;" : line_old), "##{line_code}", id: line_code %td.new_line
%td.new_line= link_to raw(type == "old" ? "&nbsp;" : line_new) , "##{line_code}", id: line_code %td.line_content{class: "#{line.type}"}= raw diff_line_content(line.text)
%td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line)
- else - else
.nothing-here-block No changes. .nothing-here-block No changes.
...@@ -75,7 +75,7 @@ ...@@ -75,7 +75,7 @@
%h4 Changes %h4 Changes
- if @diffs.present? - if @diffs.present?
= render "projects/commits/diffs", diffs: @diffs, project: @project = render "projects/diffs/diffs", diffs: @diffs, project: @project
- elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE
.bs-callout.bs-callout-danger .bs-callout.bs-callout-danger
%h4 This comparison includes more than #{MergeRequestDiff::COMMITS_SAFE_SIZE} commits. %h4 This comparison includes more than #{MergeRequestDiff::COMMITS_SAFE_SIZE} commits.
......
- if @merge_request_diff.collected? - if @merge_request_diff.collected?
= render "projects/commits/diffs", diffs: @merge_request.diffs, project: @merge_request.source_project = render "projects/diffs/diffs", diffs: @merge_request.diffs, project: @merge_request.source_project
- elsif @merge_request_diff.empty? - elsif @merge_request_diff.empty?
.nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch} .nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch}
- else - else
......
- note = @project.notes.new(@comments_target.merge({ line_code: line_code }))
= link_to "",
"javascript:;",
class: "add-diff-note js-add-diff-note-button",
data: { noteable_type: note.noteable_type,
noteable_id: note.noteable_id,
commit_id: note.commit_id,
line_code: note.line_code,
discussion_id: note.discussion_id },
title: "Add a comment to this line"
...@@ -11,16 +11,17 @@ ...@@ -11,16 +11,17 @@
%br/ %br/
.diff-content .diff-content
%table %table
- each_diff_line_near(diff, note.diff_file_index, note.line_code) do |line, type, line_code, line_new, line_old| - note.truncated_diff_lines.each do |line|
- line_code = generate_line_code(note.file_path, line)
%tr.line_holder{ id: line_code } %tr.line_holder{ id: line_code }
- if type == "match" - if line.type == "match"
%td.old_line= "..." %td.old_line= "..."
%td.new_line= "..." %td.new_line= "..."
%td.line_content.matched= line %td.line_content.matched= line.text
- else - else
%td.old_line= raw(type == "new" ? "&nbsp;" : line_old) %td.old_line= raw(line.type == "new" ? "&nbsp;" : line.old_pos)
%td.new_line= raw(type == "old" ? "&nbsp;" : line_new) %td.new_line= raw(line.type == "old" ? "&nbsp;" : line.new_pos)
%td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw "#{line} &nbsp;" %td.line_content{class: "noteable_line #{line.type} #{line_code}", "line_code" => line_code}= raw "#{line.text} &nbsp;"
- if line_code == note.line_code - if line_code == note.line_code
= render "projects/notes/diff_notes_with_reply", notes: discussion_notes = render "projects/notes/diff_notes_with_reply", notes: discussion_notes
module Gitlab
module Diff
class File
attr_reader :diff
delegate :new_file, :deleted_file, :renamed_file,
:old_path, :new_path, to: :diff, prefix: false
def initialize(diff)
@diff = diff
end
# Array of Gitlab::DIff::Line objects
def diff_lines
@lines ||= parser.parse(raw_diff.lines)
end
def mode_changed?
!!(diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode)
end
def parser
Gitlab::Diff::Parser.new
end
def raw_diff
diff.diff
end
def next_line(index)
diff_lines[index + 1]
end
def prev_line(index)
if index > 0
diff_lines[index - 1]
end
end
def file_path
if diff.new_path.present?
diff.new_path
elsif diff.old_path.present?
diff.old_path
end
end
end
end
end
module Gitlab
module Diff
class Line
attr_reader :type, :text, :index, :old_pos, :new_pos
def initialize(text, type, index, old_pos, new_pos)
@text, @type, @index = text, type, index
@old_pos, @new_pos = old_pos, new_pos
end
end
end
end
module Gitlab
module Diff
class LineCode
def self.generate(file_path, new_line_position, old_line_position)
"#{Digest::SHA1.hexdigest(file_path)}_#{old_line_position}_#{new_line_position}"
end
end
end
end
module Gitlab
module Diff
class Parser
include Enumerable
def parse(lines)
@lines = lines,
lines_obj = []
line_obj_index = 0
line_old = 1
line_new = 1
type = nil
lines_arr = ::Gitlab::InlineDiff.processing lines
lines_arr.each do |line|
raw_line = line.dup
next if filename?(line)
full_line = html_escape(line.gsub(/\n/, ''))
full_line = ::Gitlab::InlineDiff.replace_markers full_line
if line.match(/^@@ -/)
type = "match"
line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0
line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0
next if line_old == 1 && line_new == 1 #top of file
lines_obj << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new)
line_obj_index += 1
next
else
type = identification_type(line)
lines_obj << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new)
line_obj_index += 1
end
if line[0] == "+"
line_new += 1
elsif line[0] == "-"
line_old += 1
else
line_new += 1
line_old += 1
end
end
lines_obj
end
def empty?
@lines.empty?
end
private
def filename?(line)
line.start_with?('--- /dev/null', '+++ /dev/null', '--- a', '+++ b',
'--- /tmp/diffy', '+++ /tmp/diffy')
end
def identification_type(line)
if line[0] == "+"
"new"
elsif line[0] == "-"
"old"
else
nil
end
end
def html_escape str
replacements = { '&' => '&amp;', '>' => '&gt;', '<' => '&lt;', '"' => '&quot;', "'" => '&#39;' }
str.gsub(/[&"'><]/, replacements)
end
end
end
end
module Gitlab
class DiffParser
include Enumerable
attr_reader :lines, :new_path
def initialize(lines, new_path = '')
@lines = lines
@new_path = new_path
end
def each
line_old = 1
line_new = 1
type = nil
lines_arr = ::Gitlab::InlineDiff.processing lines
lines_arr.each_cons(2) do |line, next_line|
raw_line = line.dup
next if filename?(line)
full_line = html_escape(line.gsub(/\n/, ''))
full_line = ::Gitlab::InlineDiff.replace_markers full_line
next_line = html_escape(next_line.gsub(/\n/, ''))
next_line = ::Gitlab::InlineDiff.replace_markers next_line
if line.match(/^@@ -/)
type = "match"
line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0
line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0
next if line_old == 1 && line_new == 1 #top of file
yield(full_line, type, nil, line_new, line_old)
next
else
type = identification_type(line)
next_type = identification_type(next_line)
line_code = generate_line_code(new_path, line_new, line_old)
yield(full_line, type, line_code, line_new, line_old, raw_line, next_type, next_line)
end
if line[0] == "+"
line_new += 1
elsif line[0] == "-"
line_old += 1
else
line_new += 1
line_old += 1
end
end
end
def empty?
@lines.empty?
end
private
def filename?(line)
line.start_with?('--- /dev/null', '+++ /dev/null', '--- a', '+++ b',
'--- /tmp/diffy', '+++ /tmp/diffy')
end
def identification_type(line)
if line[0] == "+"
"new"
elsif line[0] == "-"
"old"
else
nil
end
end
def generate_line_code(path, line_new, line_old)
"#{Digest::SHA1.hexdigest(path)}_#{line_old}_#{line_new}"
end
def html_escape str
replacements = { '&' => '&amp;', '>' => '&gt;', '<' => '&lt;', '"' => '&quot;', "'" => '&#39;' }
str.gsub(/[&"'><]/, replacements)
end
end
end
require 'spec_helper'
describe Gitlab::Diff::File do
include RepoHelpers
let(:project) { create(:project) }
let(:commit) { project.repository.commit(sample_commit.id) }
let(:diff) { commit.diffs.first }
let(:diff_file) { Gitlab::Diff::File.new(diff) }
describe :diff_lines do
let(:diff_lines) { diff_file.diff_lines }
it { diff_lines.size.should == 30 }
it { diff_lines.first.should be_kind_of(Gitlab::Diff::Line) }
end
describe :mode_changed? do
it { diff_file.mode_changed?.should be_false }
end
end
require 'spec_helper'
describe Gitlab::Diff::Parser do
include RepoHelpers
let(:project) { create(:project) }
let(:commit) { project.repository.commit(sample_commit.id) }
let(:diff) { commit.diffs.first }
let(:parser) { Gitlab::Diff::Parser.new }
describe :parse do
let(:diff) do
<<eos
--- a/files/ruby/popen.rb
+++ b/files/ruby/popen.rb
@@ -6,12 +6,18 @@ module Popen
def popen(cmd, path=nil)
unless cmd.is_a?(Array)
- raise "System commands must be given as an array of strings"
+ raise RuntimeError, "System commands must be given as an array of strings"
end
path ||= Dir.pwd
- vars = { "PWD" => path }
- options = { chdir: path }
+
+ vars = {
+ "PWD" => path
+ }
+
+ options = {
+ chdir: path
+ }
unless File.directory?(path)
FileUtils.mkdir_p(path)
@@ -19,6 +25,7 @@ module Popen
@cmd_output = ""
@cmd_status = 0
+
Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
@cmd_output << stdout.read
@cmd_output << stderr.read
eos
end
before do
@lines = parser.parse(diff.lines)
end
it { @lines.size.should == 30 }
describe 'lines' do
describe 'first line' do
let(:line) { @lines.first }
it { line.type.should == 'match' }
it { line.old_pos.should == 6 }
it { line.new_pos.should == 6 }
it { line.text.should == '@@ -6,12 +6,18 @@ module Popen' }
end
describe 'removal line' do
let(:line) { @lines[10] }
it { line.type.should == 'old' }
it { line.old_pos.should == 14 }
it { line.new_pos.should == 13 }
it { line.text.should == '- options = { chdir: path }' }
end
describe 'addition line' do
let(:line) { @lines[16] }
it { line.type.should == 'new' }
it { line.old_pos.should == 15 }
it { line.new_pos.should == 18 }
it { line.text.should == '+ options = {' }
end
describe 'unchanged line' do
let(:line) { @lines.last }
it { line.type.should == nil }
it { line.old_pos.should == 24 }
it { line.new_pos.should == 31 }
it { line.text.should == ' @cmd_output &lt;&lt; stderr.read' }
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment