Merge pull request #9757 from yms9654/diff-option

Add ignore white space option in merge request diff

13 files changed, 80 insertions, 8 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 9da838ed281..c766e8ea98a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -37,6 +37,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
   - Improve Continuous Integration graphs page
   - 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
   - Fix bug where manually merged branches in a MR would end up with an empty diff (Stan Hu)
diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb
index 71aaad1fad6..55134e11d15 100644
--- a/app/controllers/projects/compare_controller.rb
+++ b/app/controllers/projects/compare_controller.rb
@@ -12,9 +12,10 @@ class Projects::CompareController < Projects::ApplicationController
   def show
     base_ref = Addressable::URI.unescape(params[:from])
     @ref = head_ref = Addressable::URI.unescape(params[:to])
+    diff_options = { ignore_whitespace_change: true } if params[:w] == '1'
 
     compare_result = CompareService.new.
-      execute(@project, head_ref, @project, base_ref)
+      execute(@project, head_ref, @project, base_ref, diff_options)
 
     if compare_result
       @commits = Commit.decorate(compare_result.commits, @project)
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 85f37e49e62..e81d65c2330 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -40,7 +40,7 @@ class MergeRequest < ActiveRecord::Base
   after_create :create_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
   # It allows us to close or modify broken merge requests
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 6575d0bc81f..c499a4b5b4c 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -19,7 +19,7 @@ class MergeRequestDiff < ActiveRecord::Base
   # Prevent store of diff if commits amount more then 500
   COMMITS_SAFE_SIZE = 500
 
-  attr_reader :commits, :diffs
+  attr_reader :commits, :diffs, :diffs_no_whitespace
 
   belongs_to :merge_request
 
@@ -47,6 +47,20 @@ class MergeRequestDiff < ActiveRecord::Base
     @diffs ||= (load_diffs(st_diffs) || [])
   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
     @commits ||= load_commits(st_commits || [])
   end
diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb
index bfe6a3dc4be..ec581658fc1 100644
--- a/app/services/compare_service.rb
+++ b/app/services/compare_service.rb
@@ -3,7 +3,7 @@ require 'securerandom'
 # Compare 2 branches for one repo or between repositories
 # and return Gitlab::CompareResult object that responds to commits and diffs
 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)
     return unless source_commit
 
@@ -25,7 +25,7 @@ class CompareService
         target_project.repository.raw_repository,
         target_branch,
         source_sha,
-      )
+      ), diff_options
     )
   end
 end
diff --git a/app/views/projects/merge_requests/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml
index 626970f39be..d9cfc3d7ae9 100644
--- a/app/views/projects/merge_requests/show/_diffs.html.haml
+++ b/app/views/projects/merge_requests/show/_diffs.html.haml
@@ -1,5 +1,5 @@
 - 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?
   .nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch}
 - else
diff --git a/doc/workflow/merge_requests.md b/doc/workflow/merge_requests.md
index 751e19da7f1..6d57b5d98cd 100644
--- a/doc/workflow/merge_requests.md
+++ b/doc/workflow/merge_requests.md
@@ -38,3 +38,15 @@ To check out a particular merge request:
 ```
 $ 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)
diff --git a/doc/workflow/merge_requests/commit_compare.png b/doc/workflow/merge_requests/commit_compare.png
new file mode 100644
index 00000000000..46b3a56a59b
--- /dev/null
+++ b/doc/workflow/merge_requests/commit_compare.png
diff --git a/doc/workflow/merge_requests/merge_request_diff.png b/doc/workflow/merge_requests/merge_request_diff.png
new file mode 100644
index 00000000000..ed08ae91bec
--- /dev/null
+++ b/doc/workflow/merge_requests/merge_request_diff.png
diff --git a/doc/workflow/merge_requests/merge_request_diff_without_whitespace.png b/doc/workflow/merge_requests/merge_request_diff_without_whitespace.png
new file mode 100644
index 00000000000..67d67a64d12
--- /dev/null
+++ b/doc/workflow/merge_requests/merge_request_diff_without_whitespace.png
diff --git a/lib/gitlab/compare_result.rb b/lib/gitlab/compare_result.rb
index d72391dade5..0d696a1ee28 100644
--- a/lib/gitlab/compare_result.rb
+++ b/lib/gitlab/compare_result.rb
@@ -2,8 +2,8 @@ module Gitlab
   class CompareResult
     attr_reader :commits, :diffs
 
-    def initialize(compare)
-      @commits, @diffs = compare.commits, compare.diffs
+    def initialize(compare, diff_options = {})
+      @commits, @diffs = compare.commits, compare.diffs(nil, diff_options)
     end
   end
 end
diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb
index 2a447248b70..be19f1abc53 100644
--- a/spec/controllers/projects/compare_controller_spec.rb
+++ b/spec/controllers/projects/compare_controller_spec.rb
@@ -23,6 +23,22 @@ describe Projects::CompareController do
     expect(assigns(:commits).length).to be >= 1
   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
     it 'invalid source ref' do
       get(:show,
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index b8db8591709..3e5e1fa87ae 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -147,6 +147,34 @@ describe Projects::MergeRequestsController do
     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
     def go(format: 'html')
       get :commits,