Merge pull request #5308 from Popl7/parallel-diffs-side-by-side

diff view on commit with parallel diff view

Merge pull request #5308 from Popl7/parallel-diffs-side-by-side
diff view on commit with parallel diff view
Dmitriy Zaporozhets
2 parents c5718036 856d4088
Showing 9 changed files with 165 additions and 2 deletions Show diff stats
app/assets/stylesheets/sections/commits.scss
app/assets/stylesheets/sections/notes.scss
app/helpers/commits_helper.rb
app/views/projects/commits/_diffs.html.haml
app/views/projects/commits/_parallel_view.html.haml
app/views/projects/commits/_text_file.html.haml
app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml
features/project/commits/commits.feature
features/steps/project/project_browse_commits.rb
@@ -70,7 +70,7 @@
         font-size: 12px;
       }
     }
-    .old_line, .new_line {
+    .old_line, .new_line, .diff_line {
       margin: 0px;
       padding: 0px;
       border: none;
@@ -92,6 +92,15 @@
           text-decoration: underline;
         }
       }
+      &.new {
+        background: #CFD;
+      }
+      &.old {
+        background: #FDD;
+      }
+    }
+    .diff_line {
+      padding: 0;
     }
     .line_holder {
       &.old .old_line,
@@ -122,6 +131,11 @@
         color: #ccc;
         background: #fafafa;
       }
+      &.parallel {
+        display: table-cell;
+        overflow: hidden;
+        width: 50%;
+      }
     }
   }
   .image {
@@ -131,6 +131,11 @@ ul.notes {
       text-align: center;
       padding: 10px 0;
     }
+    &.notes_line2 {
+      text-align: center;
+      padding: 10px 0;
+      border-left: 1px solid #ddd !important;
+    }
     &.notes_content {
       background-color: $white;
       border-width: 1px 0;
@@ -358,3 +363,7 @@ ul.notes {
 .js-note-attachment-delete {
   display: none;
 }
+
+.parallel-comment {
+  padding: 6px;
+}
 \ No newline at end of file
@@ -105,6 +105,10 @@ module CommitsHelper
     branches.sort.map { |branch| link_to(branch, project_tree_path(project, branch)) }.join(", ").html_safe
   end
  
+  def get_old_file(project, commit, diff)
+    project.repository.blob_at(commit.parent_id, diff.old_path) if commit.parent_id
+  end
+
   protected
  
   # Private: Returns a link to a person. If the person has a matching user and
@@ -30,6 +30,10 @@
       %strong.cgreen #{@commit.stats.additions} additions
       and
       %strong.cred #{@commit.stats.deletions} deletions
+  - if params[:view] == 'parallel'
+    = link_to "Unified Diff", url_for(view: 'unified'), {id: "commit-diff-viewtype", class: 'btn btn-tiny pull-right'}
+  - else
+    = link_to "Parallel Diff", url_for(view: 'parallel'), {id: "commit-diff-viewtype", class: 'btn btn-tiny pull-right'}
 .file-stats
   = render "projects/commits/diff_head", diffs: diffs
  
@@ -62,7 +66,10 @@
           -# Skipp all non non-supported blobs
           - next unless file.respond_to?('text?')
           - if file.text?
-            = render "projects/commits/text_file", diff: diff, index: i
+            - if params[:view] == 'parallel'
+              = render "projects/commits/parallel_view", diff: diff, project: project, file: file, index: i            
+            - else
+              = render "projects/commits/text_file", diff: diff, index: i
           - elsif file.image?
             - old_file = project.repository.blob_at(@commit.parent_id, diff.old_path) if @commit.parent_id
             = render "projects/commits/image", diff: diff, old_file: old_file, file: file, index: i
@@ -0,0 +1,75 @@
+/ Parallel diff view
+- old_file = get_old_file(project, @commit, diff)
+- deleted_lines = {}
+- added_lines = {}
+- each_diff_line(diff, index) do |line, type, line_code, line_new, line_old, raw_line|
+  - if type == "old"
+    - deleted_lines[line_old] = { line_code: line_code, type: type, line: line }
+  - elsif type == "new"
+    - added_lines[line_new]   = { line_code: line_code, type: type, line: line }
+
+- max_length = old_file.sloc + added_lines.length if old_file
+- max_length ||= file.sloc
+- offset1 = 0
+- offset2 = 0
+
+%div.text-file-parallel
+  %table{ style: "table-layout: fixed;" }
+    - max_length.times do |line_index|
+      - line_index1 = line_index - offset1
+      - line_index2 = line_index - offset2
+      - deleted_line = deleted_lines[line_index1 + 1]
+      - added_line = added_lines[line_index2 + 1]
+      - old_line = old_file.lines[line_index1] if old_file
+      - new_line = file.lines[line_index2]
+
+      - if deleted_line && added_line
+      - elsif deleted_line
+        - new_line = nil
+        - offset2 += 1
+      - elsif added_line
+        - old_line = nil
+        - offset1 += 1
+
+      %tr.line_holder.parallel
+        - if line_index == 0 && diff.new_file
+          %td.line_content.parallel= "File was created"
+          %td.old_line= ""
+        - elsif deleted_line
+          %td.line_content{class: "parallel noteable_line old #{deleted_line[:line_code]}", "line_code" => deleted_line[:line_code] }= old_line
+          %td.old_line.old
+            = line_index1 + 1
+            - if @comments_allowed
+              =# render "projects/notes/diff_note_link", line_code: deleted_line[:line_code]
+        - elsif old_line
+          %td.line_content.parallel= old_line
+          %td.old_line= line_index1 + 1
+        - else
+          %td.line_content.parallel= ""
+          %td.old_line= ""
+
+        %td.diff_line= ""
+
+        - if diff.deleted_file && line_index == 0
+          %td.new_line= ""
+          %td.line_content.parallel= "File was deleted"
+        - elsif added_line
+          %td.new_line.new
+            = line_index2 + 1
+            - if @comments_allowed
+              =# render "projects/notes/diff_note_link", line_code: added_line[:line_code]
+          %td.line_content{class: "parallel noteable_line new #{added_line[:line_code]}", "line_code" => added_line[:line_code] }= new_line
+        - elsif new_line
+          %td.new_line= line_index2 + 1
+          %td.line_content.parallel= new_line
+        - else
+          %td.new_line= ""
+          %td.line_content.parallel= ""
+
+      - if @reply_allowed
+        - comments1 = []
+        - comments2 = []
+        - comments1 = @line_notes.select { |n| n.line_code == deleted_line[:line_code] }.sort_by(&:created_at) if deleted_line
+        - comments2 = @line_notes.select { |n| n.line_code == added_line[:line_code] }.sort_by(&:created_at) if added_line
+        - unless comments1.empty? && comments2.empty?
+          = render "projects/notes/diff_notes_with_reply_parallel", notes1: comments1, notes2: comments2, line1: deleted_line, line2: added_line
 \ No newline at end of file
@@ -21,3 +21,4 @@
       - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at)
       - unless comments.empty?
         = render "projects/notes/diff_notes_with_reply", notes: comments, line: line
+
@@ -0,0 +1,34 @@
+- note1 = notes1.first # example note
+- note2 = notes2.first # example note
+%tr.notes_holder
+  -# Check if line want not changed since comment was left
+  /- if !defined?(line1) || line1 == note1.diff_line
+  - if note1
+    %td.notes_content
+      %ul.notes{ rel: note1.discussion_id }
+        = render notes1
+      = render "projects/notes/discussion_reply_button", note: note1
+    %td.notes_line2
+      %span.btn.disabled.parallel-comment
+        %i.icon-comment
+        = notes1.count
+  - else
+    %td= ""
+    %td= ""
+
+  %td= ""
+
+  -# Check if line want not changed since comment was left
+  /- if !defined?(line2) || line2 == note2.diff_line
+  - if note2
+    %td.notes_line
+      %span.btn.disabled.parallel-comment
+        %i.icon-comment
+        = notes2.count
+    %td.notes_content
+      %ul.notes{ rel: note2.discussion_id }
+        = render notes2
+      = render "projects/notes/discussion_reply_button", note: note2
+  - else
+    %td= ""
+    %td= ""
@@ -14,6 +14,12 @@ Feature: Project Browse commits
   Scenario: I browse commit from list
     Given I click on commit link
     Then I see commit info
+    And I see parallel diff button
+
+  Scenario: I browse commit with parallel diff view
+    Given I click on commit link
+    And I click parallel diff button
+    Then I see unified diff button
  
   Scenario: I compare refs
     Given I visit compare refs page
@@ -88,4 +88,17 @@ class ProjectBrowseCommits &lt; Spinach::FeatureSteps
     links[0]['href'].should =~ %r{blob/bc3735004cb45cec5e0e4fa92710897a910a5957}
     links[1]['href'].should =~ %r{blob/cc1ba255d6c5ffdce87a357ba7ccc397a4f4026b}
   end
+
+  Given 'I click parallel diff button' do
+    click_link "Parallel Diff"
+  end
+
+  Then 'I see parallel diff button' do
+    page.should have_content "Parallel Diff"
+  end
+
+  Then 'I see unified diff button' do
+    page.should have_content "Unified Diff"
+  end
+
 end
...	...	@@ -70,7 +70,7 @@
70	70	font-size: 12px;
71	71	}
72	72	}
73		- .old_line, .new_line {
	73	+ .old_line, .new_line, .diff_line {
74	74	margin: 0px;
75	75	padding: 0px;
76	76	border: none;
...	...	@@ -92,6 +92,15 @@
92	92	text-decoration: underline;
93	93	}
94	94	}
	95	+ &.new {
	96	+ background: #CFD;
	97	+ }
	98	+ &.old {
	99	+ background: #FDD;
	100	+ }
	101	+ }
	102	+ .diff_line {
	103	+ padding: 0;
95	104	}
96	105	.line_holder {
97	106	&.old .old_line,
...	...	@@ -122,6 +131,11 @@
122	131	color: #ccc;
123	132	background: #fafafa;
124	133	}
	134	+ &.parallel {
	135	+ display: table-cell;
	136	+ overflow: hidden;
	137	+ width: 50%;
	138	+ }
125	139	}
126	140	}
127	141	.image {
...	...
...	...	@@ -131,6 +131,11 @@ ul.notes {
131	131	text-align: center;
132	132	padding: 10px 0;
133	133	}
	134	+ &.notes_line2 {
	135	+ text-align: center;
	136	+ padding: 10px 0;
	137	+ border-left: 1px solid #ddd !important;
	138	+ }
134	139	&.notes_content {
135	140	background-color: $white;
136	141	border-width: 1px 0;
...	...	@@ -358,3 +363,7 @@ ul.notes {
358	363	.js-note-attachment-delete {
359	364	display: none;
360	365	}
	366	+
	367	+.parallel-comment {
	368	+ padding: 6px;
	369	+}
361	370	\ No newline at end of file
...	...
...	...	@@ -105,6 +105,10 @@ module CommitsHelper
105	105	branches.sort.map { \|branch\| link_to(branch, project_tree_path(project, branch)) }.join(", ").html_safe
106	106	end
107	107
	108	+ def get_old_file(project, commit, diff)
	109	+ project.repository.blob_at(commit.parent_id, diff.old_path) if commit.parent_id
	110	+ end
	111	+
108	112	protected
109	113
110	114	# Private: Returns a link to a person. If the person has a matching user and
...	...
...	...	@@ -30,6 +30,10 @@
30	30	%strong.cgreen #{@commit.stats.additions} additions
31	31	and
32	32	%strong.cred #{@commit.stats.deletions} deletions
	33	+ - if params[:view] == 'parallel'
	34	+ = link_to "Unified Diff", url_for(view: 'unified'), {id: "commit-diff-viewtype", class: 'btn btn-tiny pull-right'}
	35	+ - else
	36	+ = link_to "Parallel Diff", url_for(view: 'parallel'), {id: "commit-diff-viewtype", class: 'btn btn-tiny pull-right'}
33	37	.file-stats
34	38	= render "projects/commits/diff_head", diffs: diffs
35	39
...	...	@@ -62,7 +66,10 @@
62	66	-# Skipp all non non-supported blobs
63	67	- next unless file.respond_to?('text?')
64	68	- if file.text?
65		- = render "projects/commits/text_file", diff: diff, index: i
	69	+ - if params[:view] == 'parallel'
	70	+ = render "projects/commits/parallel_view", diff: diff, project: project, file: file, index: i
	71	+ - else
	72	+ = render "projects/commits/text_file", diff: diff, index: i
66	73	- elsif file.image?
67	74	- old_file = project.repository.blob_at(@commit.parent_id, diff.old_path) if @commit.parent_id
68	75	= render "projects/commits/image", diff: diff, old_file: old_file, file: file, index: i
...	...
...	...	@@ -0,0 +1,75 @@
	1	+/ Parallel diff view
	2	+- old_file = get_old_file(project, @commit, diff)
	3	+- deleted_lines = {}
	4	+- added_lines = {}
	5	+- each_diff_line(diff, index) do \|line, type, line_code, line_new, line_old, raw_line\|
	6	+ - if type == "old"
	7	+ - deleted_lines[line_old] = { line_code: line_code, type: type, line: line }
	8	+ - elsif type == "new"
	9	+ - added_lines[line_new] = { line_code: line_code, type: type, line: line }
	10	+
	11	+- max_length = old_file.sloc + added_lines.length if old_file
	12	+- max_length \|\|= file.sloc
	13	+- offset1 = 0
	14	+- offset2 = 0
	15	+
	16	+%div.text-file-parallel
	17	+ %table{ style: "table-layout: fixed;" }
	18	+ - max_length.times do \|line_index\|
	19	+ - line_index1 = line_index - offset1
	20	+ - line_index2 = line_index - offset2
	21	+ - deleted_line = deleted_lines[line_index1 + 1]
	22	+ - added_line = added_lines[line_index2 + 1]
	23	+ - old_line = old_file.lines[line_index1] if old_file
	24	+ - new_line = file.lines[line_index2]
	25	+
	26	+ - if deleted_line && added_line
	27	+ - elsif deleted_line
	28	+ - new_line = nil
	29	+ - offset2 += 1
	30	+ - elsif added_line
	31	+ - old_line = nil
	32	+ - offset1 += 1
	33	+
	34	+ %tr.line_holder.parallel
	35	+ - if line_index == 0 && diff.new_file
	36	+ %td.line_content.parallel= "File was created"
	37	+ %td.old_line= ""
	38	+ - elsif deleted_line
	39	+ %td.line_content{class: "parallel noteable_line old #{deleted_line[:line_code]}", "line_code" => deleted_line[:line_code] }= old_line
	40	+ %td.old_line.old
	41	+ = line_index1 + 1
	42	+ - if @comments_allowed
	43	+ =# render "projects/notes/diff_note_link", line_code: deleted_line[:line_code]
	44	+ - elsif old_line
	45	+ %td.line_content.parallel= old_line
	46	+ %td.old_line= line_index1 + 1
	47	+ - else
	48	+ %td.line_content.parallel= ""
	49	+ %td.old_line= ""
	50	+
	51	+ %td.diff_line= ""
	52	+
	53	+ - if diff.deleted_file && line_index == 0
	54	+ %td.new_line= ""
	55	+ %td.line_content.parallel= "File was deleted"
	56	+ - elsif added_line
	57	+ %td.new_line.new
	58	+ = line_index2 + 1
	59	+ - if @comments_allowed
	60	+ =# render "projects/notes/diff_note_link", line_code: added_line[:line_code]
	61	+ %td.line_content{class: "parallel noteable_line new #{added_line[:line_code]}", "line_code" => added_line[:line_code] }= new_line
	62	+ - elsif new_line
	63	+ %td.new_line= line_index2 + 1
	64	+ %td.line_content.parallel= new_line
	65	+ - else
	66	+ %td.new_line= ""
	67	+ %td.line_content.parallel= ""
	68	+
	69	+ - if @reply_allowed
	70	+ - comments1 = []
	71	+ - comments2 = []
	72	+ - comments1 = @line_notes.select { \|n\| n.line_code == deleted_line[:line_code] }.sort_by(&:created_at) if deleted_line
	73	+ - comments2 = @line_notes.select { \|n\| n.line_code == added_line[:line_code] }.sort_by(&:created_at) if added_line
	74	+ - unless comments1.empty? && comments2.empty?
	75	+ = render "projects/notes/diff_notes_with_reply_parallel", notes1: comments1, notes2: comments2, line1: deleted_line, line2: added_line
0	76	\ No newline at end of file
...	...
...	...	@@ -21,3 +21,4 @@
21	21	- comments = @line_notes.select { \|n\| n.line_code == line_code }.sort_by(&:created_at)
22	22	- unless comments.empty?
23	23	= render "projects/notes/diff_notes_with_reply", notes: comments, line: line
	24	+
...	...
...	...	@@ -0,0 +1,34 @@
	1	+- note1 = notes1.first # example note
	2	+- note2 = notes2.first # example note
	3	+%tr.notes_holder
	4	+ -# Check if line want not changed since comment was left
	5	+ /- if !defined?(line1) \|\| line1 == note1.diff_line
	6	+ - if note1
	7	+ %td.notes_content
	8	+ %ul.notes{ rel: note1.discussion_id }
	9	+ = render notes1
	10	+ = render "projects/notes/discussion_reply_button", note: note1
	11	+ %td.notes_line2
	12	+ %span.btn.disabled.parallel-comment
	13	+ %i.icon-comment
	14	+ = notes1.count
	15	+ - else
	16	+ %td= ""
	17	+ %td= ""
	18	+
	19	+ %td= ""
	20	+
	21	+ -# Check if line want not changed since comment was left
	22	+ /- if !defined?(line2) \|\| line2 == note2.diff_line
	23	+ - if note2
	24	+ %td.notes_line
	25	+ %span.btn.disabled.parallel-comment
	26	+ %i.icon-comment
	27	+ = notes2.count
	28	+ %td.notes_content
	29	+ %ul.notes{ rel: note2.discussion_id }
	30	+ = render notes2
	31	+ = render "projects/notes/discussion_reply_button", note: note2
	32	+ - else
	33	+ %td= ""
	34	+ %td= ""
...	...
...	...	@@ -14,6 +14,12 @@ Feature: Project Browse commits
14	14	Scenario: I browse commit from list
15	15	Given I click on commit link
16	16	Then I see commit info
	17	+ And I see parallel diff button
	18	+
	19	+ Scenario: I browse commit with parallel diff view
	20	+ Given I click on commit link
	21	+ And I click parallel diff button
	22	+ Then I see unified diff button
17	23
18	24	Scenario: I compare refs
19	25	Given I visit compare refs page
...	...
...	...	@@ -88,4 +88,17 @@ class ProjectBrowseCommits < Spinach::FeatureSteps
88	88	links[0]['href'].should =~ %r{blob/bc3735004cb45cec5e0e4fa92710897a910a5957}
89	89	links[1]['href'].should =~ %r{blob/cc1ba255d6c5ffdce87a357ba7ccc397a4f4026b}
90	90	end
	91	+
	92	+ Given 'I click parallel diff button' do
	93	+ click_link "Parallel Diff"
	94	+ end
	95	+
	96	+ Then 'I see parallel diff button' do
	97	+ page.should have_content "Parallel Diff"
	98	+ end
	99	+
	100	+ Then 'I see unified diff button' do
	101	+ page.should have_content "Unified Diff"
	102	+ end
	103	+
91	104	end
...	...