Skip to content
Snippets Groups Projects
Commit 3d0efa8e authored by Minsik Yoon's avatar Minsik Yoon Committed by 윤민식
Browse files

Add ignore white space option in merge request diff

fix this issue(https://gitlab.com/gitlab-org/gitlab-ce/issues/1393).

Add ignore whitespace optoin to Commits Compare view
parent 23ed732e
No related branches found
No related tags found
No related merge requests found
Showing
with 80 additions and 8 deletions
Loading
@@ -36,6 +36,7 @@ v 8.2.0 (unreleased)
Loading
@@ -36,6 +36,7 @@ v 8.2.0 (unreleased)
- MR target branch is now visible on a list view when it is different from project's default one - MR target branch is now visible on a list view when it is different from project's default one
- Improve Continuous Integration graphs page - Improve Continuous Integration graphs page
- Make color of "Accept Merge Request" button consistent with current build status - Make color of "Accept Merge Request" button consistent with current build status
- Add ignore white space option in merge request diff and commit and compare view
   
v 8.1.4 v 8.1.4
- Fix bug where manually merged branches in a MR would end up with an empty diff (Stan Hu) - Fix bug where manually merged branches in a MR would end up with an empty diff (Stan Hu)
Loading
Loading
Loading
@@ -12,9 +12,10 @@ class Projects::CompareController < Projects::ApplicationController
Loading
@@ -12,9 +12,10 @@ class Projects::CompareController < Projects::ApplicationController
def show def show
base_ref = Addressable::URI.unescape(params[:from]) base_ref = Addressable::URI.unescape(params[:from])
@ref = head_ref = Addressable::URI.unescape(params[:to]) @ref = head_ref = Addressable::URI.unescape(params[:to])
diff_options = { ignore_whitespace_change: true } if params[:w] == '1'
   
compare_result = CompareService.new. compare_result = CompareService.new.
execute(@project, head_ref, @project, base_ref) execute(@project, head_ref, @project, base_ref, diff_options)
   
if compare_result if compare_result
@commits = Commit.decorate(compare_result.commits, @project) @commits = Commit.decorate(compare_result.commits, @project)
Loading
Loading
Loading
@@ -40,7 +40,7 @@ class MergeRequest < ActiveRecord::Base
Loading
@@ -40,7 +40,7 @@ class MergeRequest < ActiveRecord::Base
after_create :create_merge_request_diff after_create :create_merge_request_diff
after_update :update_merge_request_diff after_update :update_merge_request_diff
   
delegate :commits, :diffs, to: :merge_request_diff, prefix: nil delegate :commits, :diffs, :diffs_no_whitespace, to: :merge_request_diff, prefix: nil
   
# When this attribute is true some MR validation is ignored # When this attribute is true some MR validation is ignored
# It allows us to close or modify broken merge requests # It allows us to close or modify broken merge requests
Loading
Loading
Loading
@@ -19,7 +19,7 @@ class MergeRequestDiff < ActiveRecord::Base
Loading
@@ -19,7 +19,7 @@ class MergeRequestDiff < ActiveRecord::Base
# Prevent store of diff if commits amount more then 500 # Prevent store of diff if commits amount more then 500
COMMITS_SAFE_SIZE = 500 COMMITS_SAFE_SIZE = 500
   
attr_reader :commits, :diffs attr_reader :commits, :diffs, :diffs_no_whitespace
   
belongs_to :merge_request belongs_to :merge_request
   
Loading
@@ -47,6 +47,20 @@ class MergeRequestDiff < ActiveRecord::Base
Loading
@@ -47,6 +47,20 @@ class MergeRequestDiff < ActiveRecord::Base
@diffs ||= (load_diffs(st_diffs) || []) @diffs ||= (load_diffs(st_diffs) || [])
end end
   
def diffs_no_whitespace
# Get latest sha of branch from source project
source_sha = merge_request.source_project.commit(source_branch).sha
compare_result = Gitlab::CompareResult.new(
Gitlab::Git::Compare.new(
merge_request.target_project.repository.raw_repository,
merge_request.target_branch,
source_sha,
), { ignore_whitespace_change: true }
)
@diffs_no_whitespace ||= load_diffs(dump_commits(compare_result.diffs))
end
def commits def commits
@commits ||= load_commits(st_commits || []) @commits ||= load_commits(st_commits || [])
end end
Loading
Loading
Loading
@@ -3,7 +3,7 @@ require 'securerandom'
Loading
@@ -3,7 +3,7 @@ require 'securerandom'
# Compare 2 branches for one repo or between repositories # Compare 2 branches for one repo or between repositories
# and return Gitlab::CompareResult object that responds to commits and diffs # and return Gitlab::CompareResult object that responds to commits and diffs
class CompareService class CompareService
def execute(source_project, source_branch, target_project, target_branch) def execute(source_project, source_branch, target_project, target_branch, diff_options = {})
source_commit = source_project.commit(source_branch) source_commit = source_project.commit(source_branch)
return unless source_commit return unless source_commit
   
Loading
@@ -25,7 +25,7 @@ class CompareService
Loading
@@ -25,7 +25,7 @@ class CompareService
target_project.repository.raw_repository, target_project.repository.raw_repository,
target_branch, target_branch,
source_sha, source_sha,
) ), diff_options
) )
end end
end end
- if @merge_request_diff.collected? - if @merge_request_diff.collected?
= render "projects/diffs/diffs", diffs: @merge_request.diffs, project: @merge_request.project = render "projects/diffs/diffs", diffs: params[:w] == '1' ? @merge_request.diffs_no_whitespace : @merge_request.diffs, project: @merge_request.project
- 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
Loading
Loading
Loading
@@ -38,3 +38,15 @@ To check out a particular merge request:
Loading
@@ -38,3 +38,15 @@ To check out a particular merge request:
``` ```
$ git checkout origin/merge-requests/1 $ git checkout origin/merge-requests/1
``` ```
## Ignore whitespace changes in Merge Request diff view
![MR diff](merge_requests/merge_request_diff.png)
It you add `w=1` option to URL, you can see diff without whitespace changes.
![MR diff without whitespace](merge_requests/merge_request_diff_without_whitespace.png)
It is also working on commits compare view.
![Commit Compare](merge_requests/commit_compare.png)
doc/workflow/merge_requests/commit_compare.png

87.5 KiB

doc/workflow/merge_requests/merge_request_diff.png

118 KiB

doc/workflow/merge_requests/merge_request_diff_without_whitespace.png

96.6 KiB

Loading
@@ -2,8 +2,8 @@ module Gitlab
Loading
@@ -2,8 +2,8 @@ module Gitlab
class CompareResult class CompareResult
attr_reader :commits, :diffs attr_reader :commits, :diffs
   
def initialize(compare) def initialize(compare, diff_options = {})
@commits, @diffs = compare.commits, compare.diffs @commits, @diffs = compare.commits, compare.diffs(nil, diff_options)
end end
end end
end end
Loading
@@ -23,6 +23,22 @@ describe Projects::CompareController do
Loading
@@ -23,6 +23,22 @@ describe Projects::CompareController do
expect(assigns(:commits).length).to be >= 1 expect(assigns(:commits).length).to be >= 1
end end
   
it 'compare should show some diffs with ignore whitespace change option' do
get(:show,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
from: '08f22f25',
to: '66eceea0',
w: 1)
expect(response).to be_success
expect(assigns(:diffs).length).to be >= 1
expect(assigns(:commits).length).to be >= 1
# without whitespace option, there are more than 2 diff_splits
diff_splits = assigns(:diffs)[0].diff.split("\n")
expect(diff_splits.length).to be <= 2
end
describe 'non-existent refs' do describe 'non-existent refs' do
it 'invalid source ref' do it 'invalid source ref' do
get(:show, get(:show,
Loading
Loading
Loading
@@ -147,6 +147,34 @@ describe Projects::MergeRequestsController do
Loading
@@ -147,6 +147,34 @@ describe Projects::MergeRequestsController do
end end
end end
   
describe 'GET diffs with ignore_whitespace_change' do
def go(format: 'html')
get :diffs,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
id: merge_request.iid,
format: format,
w: 1
end
context 'as html' do
it 'renders the diff template' do
go
expect(response).to render_template('diffs')
end
end
context 'as json' do
it 'renders the diffs template to a string' do
go format: 'json'
expect(response).to render_template('projects/merge_requests/show/_diffs')
expect(JSON.parse(response.body)).to have_key('html')
end
end
end
describe 'GET commits' do describe 'GET commits' do
def go(format: 'html') def go(format: 'html')
get :commits, get :commits,
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment