Commit 6b9c730e authored by Rubén Dávila's avatar Rubén Dávila

More refactoring from last code review. #3945

* Use commit objects instead of IDs when generating diffs
* Use proper references when generating MR's source and target
* Update broken specs
parent 70bc3224
...@@ -72,7 +72,7 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -72,7 +72,7 @@ class Projects::CommitController < Projects::ApplicationController
@diffs = commit.diffs @diffs = commit.diffs
end end
@diff_refs = [commit.parent_id || commit.id, commit.id] @diff_refs = [commit.parent || commit, commit]
@notes_count = commit.notes.count @notes_count = commit.notes.count
@statuses = ci_commit.statuses if ci_commit @statuses = ci_commit.statuses if ci_commit
......
...@@ -20,9 +20,9 @@ class Projects::CompareController < Projects::ApplicationController ...@@ -20,9 +20,9 @@ class Projects::CompareController < Projects::ApplicationController
if compare_result if compare_result
@commits = Commit.decorate(compare_result.commits, @project) @commits = Commit.decorate(compare_result.commits, @project)
@diffs = compare_result.diffs @diffs = compare_result.diffs
@diff_refs = [base_ref, head_ref]
@commit = @project.commit(head_ref) @commit = @project.commit(head_ref)
@first_commit = @project.commit(base_ref) @first_commit = @project.commit(base_ref)
@diff_refs = [@first_commit, @commit]
@line_notes = [] @line_notes = []
end end
end end
......
...@@ -59,7 +59,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -59,7 +59,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def diffs def diffs
@commit = @merge_request.last_commit @commit = @merge_request.last_commit
@first_commit = @merge_request.first_commit @first_commit = @merge_request.first_commit
@diff_refs = [@merge_request.last_commit.parent_id, @merge_request.source_sha]
@comments_allowed = @reply_allowed = true @comments_allowed = @reply_allowed = true
@comments_target = { @comments_target = {
...@@ -104,8 +103,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -104,8 +103,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@commit = @merge_request.last_commit @commit = @merge_request.last_commit
@first_commit = @merge_request.first_commit @first_commit = @merge_request.first_commit
@diffs = @merge_request.compare_diffs @diffs = @merge_request.compare_diffs
# We need to use #source_branch because #source_sha requires an existent MergeRequestDiff object.
@diff_refs = [@merge_request.target_sha, @merge_request.source_branch]
@ci_commit = @merge_request.ci_commit @ci_commit = @merge_request.ci_commit
@statuses = @ci_commit.statuses if @ci_commit @statuses = @ci_commit.statuses if @ci_commit
......
...@@ -19,13 +19,13 @@ module DiffHelper ...@@ -19,13 +19,13 @@ module DiffHelper
end end
end end
def safe_diff_files(diffs, diff_refs, repository) def safe_diff_files(diffs, diff_refs)
lines = 0 lines = 0
safe_files = [] safe_files = []
diffs.first(allowed_diff_size).each do |diff| diffs.first(allowed_diff_size).each do |diff|
lines += diff.diff.lines.count lines += diff.diff.lines.count
break if lines > allowed_diff_lines break if lines > allowed_diff_lines
safe_files << Gitlab::Diff::File.new(diff, diff_refs, repository) safe_files << Gitlab::Diff::File.new(diff, diff_refs)
end end
safe_files safe_files
end end
......
...@@ -511,4 +511,8 @@ class MergeRequest < ActiveRecord::Base ...@@ -511,4 +511,8 @@ class MergeRequest < ActiveRecord::Base
def broken? def broken?
self.commits.blank? || branch_missing? || cannot_be_merged? self.commits.blank? || branch_missing? || cannot_be_merged?
end end
def diff_range
[last_commit.parent, first_commit]
end
end end
- if diff_view == 'parallel' - if diff_view == 'parallel'
- fluid_layout true - fluid_layout true
- diff_files = safe_diff_files(diffs, diff_refs, repository) - diff_files = safe_diff_files(diffs, diff_refs)
.gray-content-block.middle-block.oneline-block .gray-content-block.middle-block.oneline-block
.inline-parallel-buttons .inline-parallel-buttons
......
...@@ -38,7 +38,7 @@ ...@@ -38,7 +38,7 @@
= render "projects/merge_requests/show/commits" = render "projects/merge_requests/show/commits"
#diffs.diffs.tab-pane.active #diffs.diffs.tab-pane.active
- if @diffs.present? - if @diffs.present?
= render "projects/diffs/diffs", diffs: @diffs, project: @project, diff_refs: @diff_refs = render "projects/diffs/diffs", diffs: @diffs, project: @project, diff_refs: @merge_request.diff_range
- elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE
.alert.alert-danger .alert.alert-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/diffs/diffs", diffs: params[:w] == '1' ? @merge_request.diffs_no_whitespace : @merge_request.diffs, project: @merge_request.project, diff_refs: @diff_refs = 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_range
- 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
......
module Gitlab module Gitlab
module Diff module Diff
class File class File
attr_reader :diff, :repository, :new_ref, :old_ref attr_reader :diff, :new_ref, :old_ref
delegate :new_file, :deleted_file, :renamed_file, delegate :new_file, :deleted_file, :renamed_file,
:old_path, :new_path, to: :diff, prefix: false :old_path, :new_path, to: :diff, prefix: false
def initialize(diff, diff_refs, repository) def initialize(diff, diff_refs)
@diff = diff @diff = diff
@repository = repository
@old_ref, @new_ref = diff_refs @old_ref, @new_ref = diff_refs
end end
......
...@@ -3,8 +3,7 @@ module Gitlab ...@@ -3,8 +3,7 @@ module Gitlab
class Highlight class Highlight
attr_reader :diff_file attr_reader :diff_file
delegate :repository, :old_path, :new_path, :old_ref, :new_ref, delegate :old_path, :new_path, :old_ref, :new_ref, to: :diff_file, prefix: :diff
to: :diff_file, prefix: :diff
def initialize(diff_file) def initialize(diff_file)
@diff_file = diff_file @diff_file = diff_file
...@@ -141,11 +140,11 @@ module Gitlab ...@@ -141,11 +140,11 @@ module Gitlab
end end
def old_lines def old_lines
@old_lines ||= Gitlab::Highlight.highlight_lines(diff_repository, diff_old_ref, diff_old_path) @old_lines ||= self.class.process_file(*processing_args(:old))
end end
def new_lines def new_lines
@new_lines ||= Gitlab::Highlight.highlight_lines(diff_repository, diff_new_ref, diff_new_path) @new_lines ||= self.class.process_file(*processing_args(:new))
end end
def longest_common_suffix(a, b) def longest_common_suffix(a, b)
...@@ -200,6 +199,16 @@ module Gitlab ...@@ -200,6 +199,16 @@ module Gitlab
offset offset
end end
private
def processing_args(version)
ref = send("diff_#{version}_ref")
path = send("diff_#{version}_path")
[ref.project.repository, ref.id, path]
end
end end
end end
end end
--- ---
- - match - :left:
- 6 :type: match
- "@@ -6,12 +6,18 @@ module Popen" :number: 6
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 :text: "@@ -6,12 +6,18 @@ module Popen"
- match :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6
- 6 :right:
- "@@ -6,12 +6,18 @@ module Popen" :type: match
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 :number: 6
- - :text: "@@ -6,12 +6,18 @@ module Popen"
- 6 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6
- |2 - :left:
<span id="LC6" class="line"></span> :type:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 :number: 6
- :text: |2
- 6 <span id="LC6" class="line"></span>
- |2 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6
<span id="LC6" class="line"></span> :right:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 :type:
- - :number: 6
- 7 :text: |2
- |2 <span id="LC6" class="line"></span>
<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_6_6
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7 - :left:
- :type:
- 7 :number: 7
- |2 :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> <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>
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7
- - :right:
- 8 :type:
- |2 :number: 7
<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> :text: |2
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8 <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
- 8 - :left:
- |2 :type:
<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> :number: 8
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8 :text: |2
- - old <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>
- 9 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8
- | :right:
-<span id="LC9" class="line"> <span class="k">raise</span> <span class="s2">&quot;System commands must be given as an array of strings&quot;</span></span> :type:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9 :number: 8
- new :text: |2
- 9 <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
+<span id="LC9" class="line"> <span class="k">raise</span> <span class="no">RuntimeError</span><span class="p">,</span> <span class="s2">&quot;System commands must be given as an array of strings&quot;</span></span> - :left:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9 :type: old
- - :number: 9
- 10 :text: |
- |2 -<span id="LC9" class="line"> <span class="k">raise</span> <span class="s2">&quot;System commands must be given as an array of strings&quot;</span></span>
<span id="LC10" class="line"> <span class="k">end</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10 :right:
- :type: new
- 10 :number: 9
- |2 :text: |
<span id="LC10" class="line"> <span class="k">end</span></span> +<span id="LC9" class="line"> <span class="k">raise</span> <span class="no">RuntimeError</span><span class="p">,</span> <span class="s2">&quot;System commands must be given as an array of strings&quot;</span></span>
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9
- - - :left:
- 11 :type:
- |2 :number: 10
<span id="LC11" class="line"></span> :text: |2
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11 <span id="LC10" class="line"> <span class="k">end</span></span>
- :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10
- 11 :right:
- |2 :type:
<span id="LC11" class="line"></span> :number: 10
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11 :text: |2
- - <span id="LC10" class="line"> <span class="k">end</span></span>
- 12 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10
- |2 - :left:
<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> :type:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12 :number: 11
- :text: |2
- 12 <span id="LC11" class="line"></span>
- |2 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11
<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> :right:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12 :type:
- - old :number: 11
- 13 :text: |2
- | <span id="LC11" class="line"></span>
-<span id="LC13" class="line"> <span class="n">vars</span> <span class="o">=</span> <span class="p">{</span> <span class="s2">&quot;PWD&quot;</span> <span class="o">=&gt;</span> <span class="n">path</span> <span class="p">}</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13 - :left:
- old :type:
- :number: 12
- "&nbsp;" :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>
- - old :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12
- 14 :right:
- | :type:
-<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> :number: 12
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13 :text: |2
- new <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>
- 13 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12
- | - :left:
+<span id="LC13" class="line"></span> :type: old
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_13 :number: 13
- - :text: |
- -<span id="LC13" class="line"> <span class="n">vars</span> <span class="o">=</span> <span class="p">{</span> <span class="s2">&quot;PWD&quot;</span> <span class="o">=&gt;</span> <span class="n">path</span> <span class="p">}</span></span>
- "&nbsp;" :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14 :right:
- new :type: old
- 14 :number:
- | :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: old
- :number: 14
- "&nbsp;" :text: |
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15 -<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>
- new :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13
- 15 :right:
- | :type: new
+<span id="LC15" class="line"> <span class="s2">&quot;PWD&quot;</span> <span class="o">=&gt;</span> <span class="n">path</span></span> :number: 13
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15 :text: |
- - +<span id="LC13" class="line"></span>
- :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_13
- "&nbsp;" - :left:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16 :type:
- new :number:
- 16 :text: ''
- | :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14
+<span id="LC16" class="line"> <span class="p">}</span></span> :right:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16 :type: new
- - :number: 14
- :text: |
- "&nbsp;" +<span id="LC14" class="line"> <span class="n">vars</span> <span class="o">=</span> <span class="p">{</span></span>
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14
- new - :left:
- 17 :type:
- | :number:
+<span id="LC17" class="line"></span> :text: ''
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15
- - :right:
- :type: new
- "&nbsp;" :number: 15
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18 :text: |
- new +<span id="LC15" class="line"> <span class="s2">&quot;PWD&quot;</span> <span class="o">=&gt;</span> <span class="n">path</span></span>
- 18 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15
- | - :left:
+<span id="LC18" class="line"> <span class="n">options</span> <span class="o">=</span> <span class="p">{</span></span> :type:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18 :number:
- - :text: ''
- :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16
- "&nbsp;" :right:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19 :type: new
- new :number: 16
- 19 :text: |
- | +<span id="LC16" class="line"> <span class="p">}</span></span>
+<span id="LC19" class="line"> <span class="ss">chdir: </span><span class="n">path</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19 - :left:
- - :type:
- :number:
- "&nbsp;" :text: ''
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17
- new :right:
- 20 :type: new
- | :number: 17
+<span id="LC20" class="line"> <span class="p">}</span></span> :text: |
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20 +<span id="LC17" class="line"></span>
- - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17
- 15 - :left:
- |2 :type:
<span id="LC21" class="line"></span> :number:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21 :text: ''
- :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18
- 21 :right:
- |2 :type: new
<span id="LC21" class="line"></span> :number: 18
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21 :text: |
- - +<span id="LC18" class="line"> <span class="n">options</span> <span class="o">=</span> <span class="p">{</span></span>
- 16 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18
- |2 - :left:
<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> :type:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22 :number:
- :text: ''
- 22 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19
- |2 :right:
<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> :type: new
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22 :number: 19
- - :text: |
- 17 +<span id="LC19" class="line"> <span class="ss">chdir: </span><span class="n">path</span></span>
- |2 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19
<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> - :left:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23 :type:
- :number:
- 23 :text: ''
- |2 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20
<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> :right:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23 :type: new
- - match :number: 20
- 19 :text: |
- "@@ -19,6 +25,7 @@ module Popen" +<span id="LC20" class="line"> <span class="p">}</span></span>
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20
- match - :left:
- 25 :type:
- "@@ -19,6 +25,7 @@ module Popen" :number: 15
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 :text: |2
- - <span id="LC21" class="line"></span>
- 19 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21
- |2 :right:
<span id="LC25" class="line"></span> :type:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 :number: 21
- :text: |2
- 25 <span id="LC21" class="line"></span>
- |2 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21
<span id="LC25" class="line"></span> - :left:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 :type:
- - :number: 16
- 20 :text: |2
- |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>
<span id="LC26" class="line"> <span class="vi">@cmd_output</span> <span class="o">=</span> <span class="s2">&quot;&quot;</span></span> :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26 :right:
- :type:
- 26 :number: 22
- |2 :text: |2
<span id="LC26" class="line"> <span class="vi">@cmd_output</span> <span class="o">=</span> <span class="s2">&quot;&quot;</span></span> <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>
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22
- - - :left:
- 21 :type:
- |2 :number: 17
<span id="LC27" class="line"> <span class="vi">@cmd_status</span> <span class="o">=</span> <span class="mi">0</span></span> :text: |2
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27 <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
- 27 :right:
- |2 :type:
<span id="LC27" class="line"> <span class="vi">@cmd_status</span> <span class="o">=</span> <span class="mi">0</span></span> :number: 23
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27 :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
- "&nbsp;" - :left:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28 :type: match
- new :number: 19
- 28 :text: "@@ -19,6 +25,7 @@ module Popen"
- | :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25
+<span id="LC28" class="line"></span> :right:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28 :type: match
- - :number: 25
- 22 :text: "@@ -19,6 +25,7 @@ module Popen"
- |2 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25
<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> - :left:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29 :type:
- :number: 19
- 29 :text: |2
- |2 <span id="LC25" class="line"></span>
<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_19_25
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29 :right:
- - :type:
- 23 :number: 25
- |2 :text: |2
<span id="LC30" class="line"> <span class="vi">@cmd_output</span> <span class="o">&lt;&lt;</span> <span class="n">stdout</span><span class="p">.</span><span class="nf">read</span></span> <span id="LC25" class="line"></span>
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25
- - :left:
- 30 :type:
- |2 :number: 20
<span id="LC30" class="line"> <span class="vi">@cmd_output</span> <span class="o">&lt;&lt;</span> <span class="n">stdout</span><span class="p">.</span><span class="nf">read</span></span> :text: |2
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30 <span id="LC26" class="line"> <span class="vi">@cmd_output</span> <span class="o">=</span> <span class="s2">&quot;&quot;</span></span>
- - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26
- 24 :right:
- |2 :type:
<span id="LC31" class="line"> <span class="vi">@cmd_output</span> <span class="o">&lt;&lt;</span> <span class="n">stderr</span><span class="p">.</span><span class="nf">read</span></span> :number: 26
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31 :text: |2
- <span id="LC26" class="line"> <span class="vi">@cmd_output</span> <span class="o">=</span> <span class="s2">&quot;&quot;</span></span>
- 31 :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26
- |2 - :left:
<span id="LC31" class="line"> <span class="vi">@cmd_output</span> <span class="o">&lt;&lt;</span> <span class="n">stderr</span><span class="p">.</span><span class="nf">read</span></span> :type:
- 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31 :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">&lt;&lt;</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">&lt;&lt;</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">&lt;&lt;</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">&lt;&lt;</span> <span class="n">stderr</span><span class="p">.</span><span class="nf">read</span></span>
:line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31
...@@ -8,8 +8,8 @@ describe DiffHelper do ...@@ -8,8 +8,8 @@ describe DiffHelper do
let(:commit) { project.commit(sample_commit.id) } let(:commit) { project.commit(sample_commit.id) }
let(:diffs) { commit.diffs } let(:diffs) { commit.diffs }
let(:diff) { diffs.first } let(:diff) { diffs.first }
let(:diff_refs) { [commit.parent.id, commit.id] } let(:diff_refs) { [commit.parent, commit] }
let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs, repository) } let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs) }
describe 'diff_hard_limit_enabled?' do describe 'diff_hard_limit_enabled?' do
it 'should return true if param is provided' do it 'should return true if param is provided' do
...@@ -46,41 +46,41 @@ describe DiffHelper do ...@@ -46,41 +46,41 @@ describe DiffHelper do
describe 'safe_diff_files' do describe 'safe_diff_files' do
it 'should return all files from a commit that is smaller than safe limits' do it 'should return all files from a commit that is smaller than safe limits' do
expect(safe_diff_files(diffs, diff_refs, repository).length).to eq(2) expect(safe_diff_files(diffs, diff_refs).length).to eq(2)
end end
it 'should return only the first file if the diff line count in the 2nd file takes the total beyond safe limits' do 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 allow(diffs[1].diff).to receive(:lines).and_return([""] * 4999) #simulate 4999 lines
expect(safe_diff_files(diffs, diff_refs, repository).length).to eq(1) expect(safe_diff_files(diffs, diff_refs).length).to eq(1)
end end
it 'should return all files from a commit that is beyond safe limit for numbers of lines if force diff is true' do 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(controller).to receive(:params) { { force_show_diff: true } }
allow(diffs[1].diff).to receive(:lines).and_return([""] * 4999) #simulate 4999 lines allow(diffs[1].diff).to receive(:lines).and_return([""] * 4999) #simulate 4999 lines
expect(safe_diff_files(diffs, diff_refs, repository).length).to eq(2) expect(safe_diff_files(diffs, diff_refs).length).to eq(2)
end end
it 'should return only the first file if the diff line count in the 2nd file takes the total beyond hard limits' do 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(controller).to receive(:params) { { force_show_diff: true } }
allow(diffs[1].diff).to receive(:lines).and_return([""] * 49999) #simulate 49999 lines allow(diffs[1].diff).to receive(:lines).and_return([""] * 49999) #simulate 49999 lines
expect(safe_diff_files(diffs, diff_refs, repository).length).to eq(1) expect(safe_diff_files(diffs, diff_refs).length).to eq(1)
end end
it 'should return only a safe number of file diffs if a commit touches more files than the safe limits' do 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 large_diffs = diffs * 100 #simulate 200 diffs
expect(safe_diff_files(large_diffs, diff_refs, repository).length).to eq(100) expect(safe_diff_files(large_diffs, diff_refs).length).to eq(100)
end end
it 'should return all file diffs if a commit touches more files than the safe limits but force diff is true' do 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 } } allow(controller).to receive(:params) { { force_show_diff: true } }
large_diffs = diffs * 100 #simulate 200 diffs large_diffs = diffs * 100 #simulate 200 diffs
expect(safe_diff_files(large_diffs, diff_refs, repository).length).to eq(200) expect(safe_diff_files(large_diffs, diff_refs).length).to eq(200)
end end
it 'should return a limited file diffs if a commit touches more files than the hard limits and force diff is true' do 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 } } allow(controller).to receive(:params) { { force_show_diff: true } }
very_large_diffs = diffs * 1000 #simulate 2000 diffs very_large_diffs = diffs * 1000 #simulate 2000 diffs
expect(safe_diff_files(very_large_diffs, diff_refs, repository).length).to eq(1000) expect(safe_diff_files(very_large_diffs, diff_refs).length).to eq(1000)
end end
end end
...@@ -128,7 +128,11 @@ describe DiffHelper do ...@@ -128,7 +128,11 @@ describe DiffHelper do
expect(diff_line_content(diff_file.diff_lines.first.text)). expect(diff_line_content(diff_file.diff_lines.first.text)).
to eq('@@ -6,12 +6,18 @@ module Popen') 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.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
it 'should return safe HTML' do
expect(diff_line_content(diff_file.diff_lines.first.text)).to be_html_safe
end end
end end
......
...@@ -6,7 +6,7 @@ describe Gitlab::Diff::File, lib: true do ...@@ -6,7 +6,7 @@ describe Gitlab::Diff::File, lib: true do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:commit) { project.commit(sample_commit.id) } let(:commit) { project.commit(sample_commit.id) }
let(:diff) { commit.diffs.first } let(:diff) { commit.diffs.first }
let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent.id, commit.id], project.repository) } let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent, commit]) }
describe :diff_lines do describe :diff_lines do
let(:diff_lines) { diff_file.diff_lines } let(:diff_lines) { diff_file.diff_lines }
......
...@@ -6,7 +6,7 @@ describe Gitlab::Diff::Highlight, lib: true do ...@@ -6,7 +6,7 @@ describe Gitlab::Diff::Highlight, lib: true do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:commit) { project.commit(sample_commit.id) } let(:commit) { project.commit(sample_commit.id) }
let(:diff) { commit.diffs.first } let(:diff) { commit.diffs.first }
let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent.id, commit.id], project.repository) } let(:diff_file) { Gitlab::Diff::File.new(diff, [commit.parent, commit]) }
describe '.process_diff_lines' do describe '.process_diff_lines' do
context 'when processing Gitlab::Diff::Line objects' do context 'when processing Gitlab::Diff::Line objects' do
......
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