Refactor Parallel Diff feature and add scrollbars

Also now removed lines relating to unused comments feature

Refactor Parallel Diff feature and add scrollbars
Also now removed lines relating to unused comments feature
George Dewar
1 parent ad7cd8fb
Showing 4 changed files with 147 additions and 71 deletions Show diff stats
app/assets/stylesheets/sections/diff.scss
app/helpers/commits_helper.rb
app/models/diff_line.rb
app/views/projects/commits/_parallel_view.html.haml
@@ -62,6 +62,29 @@
         font-size: 12px;
       }
     }
+
+    .text-file-parallel div {
+      display: inline-block;
+      padding-bottom: 16px;
+    }
+    .diff-side {
+      overflow-x: scroll;
+      width: 508px;
+      height: 700px;
+    }
+    .diff-side.diff-side-left{
+      overflow-y:hidden;
+    }
+    .diff-side table, td.diff-middle table {
+      height: 700px;
+    }
+    .diff-middle {
+      width: 114px;
+      vertical-align: top;
+      height: 700px;
+      overflow: hidden
+    }
+
     .old_line, .new_line, .diff_line {
       margin: 0px;
       padding: 0px;
@@ -125,8 +148,6 @@
       }
       &.parallel {
         display: table-cell;
-        overflow: hidden;
-        width: 50%;
       }
     }
   }
@@ -105,8 +105,80 @@ 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
+  def parallel_diff_lines(project, commit, diff, file)
+    old_file = project.repository.blob_at(commit.parent_id, diff.old_path) if commit.parent_id
+    deleted_lines = {}
+    added_lines = {}
+    each_diff_line(diff, 0) do |line, type, line_code, line_new, line_old|
+      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 }
+      end
+    end
+    max_length = old_file ? old_file.sloc + added_lines.length : file.sloc
+
+    offset1 = 0
+    offset2 = 0
+    old_lines = []
+    new_lines = []
+
+    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
+      end
+
+      old_lines[line_index] = DiffLine.new
+      new_lines[line_index] = DiffLine.new
+
+      # old
+      if line_index == 0 && diff.new_file
+        old_lines[line_index].type = :file_created
+        old_lines[line_index].content = 'File was created'
+      elsif deleted_line
+        old_lines[line_index].type = :deleted
+        old_lines[line_index].content = old_line
+        old_lines[line_index].num = line_index1 + 1
+        old_lines[line_index].code = deleted_line[:line_code]
+      elsif old_line
+        old_lines[line_index].type = :no_change
+        old_lines[line_index].content = old_line
+        old_lines[line_index].num = line_index1 + 1
+      else
+        old_lines[line_index].type = :added
+      end
+
+      # new
+      if line_index == 0 && diff.deleted_file
+        new_lines[line_index].type = :file_deleted
+        new_lines[line_index].content = "File was deleted"
+      elsif added_line
+        new_lines[line_index].type = :added
+        new_lines[line_index].num = line_index2 + 1
+        new_lines[line_index].content = new_line
+        new_lines[line_index].code = added_line[:line_code]
+      elsif new_line
+        new_lines[line_index].type = :no_change
+        new_lines[line_index].num = line_index2 + 1
+        new_lines[line_index].content = new_line
+      else
+        new_lines[line_index].type = :deleted
+      end
+    end
+
+    return old_lines, new_lines
   end
   protected
@@ -0,0 +1,3 @@
+class DiffLine
+  attr_accessor :type, :content, :num, :code
+end
 / Side-by-side 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
+- old_lines, new_lines = parallel_diff_lines(project, @commit, diff, file)
+- num_lines = old_lines.length
 %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]
+  %div.diff-side.diff-side-left
+    %table
+      - old_lines.each do |line|
+
+        %tr.line_holder.parallel
+          - if line.type == :file_created
+            %td.line_content.parallel= "File was created"
+          - elsif line.type == :deleted
+            %td.line_content{class: "parallel noteable_line old #{line.code}", "line_code" => line.code }= line.content
+          - else line.type == :no_change
+            %td.line_content.parallel= line.content
+
+  %div.diff-middle
+    %table
+      - num_lines.times do |index|
+        %tr
+          - if old_lines[index].type == :deleted
+            %td.old_line.old= old_lines[index].num
+          - else
+            %td.old_line= old_lines[index].num
+
+          %td.diff_line=""
-      - if deleted_line && added_line
-      - elsif deleted_line
-        - new_line = nil
-        - offset2 += 1
-      - elsif added_line
-        - old_line = nil
-        - offset1 += 1
+          - if new_lines[index].type == :added
+            %td.new_line.new= new_lines[index].num
+          - else
+            %td.new_line= new_lines[index].num
-      %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= ""
+  %div.diff-side.diff-side-right
+    %table
+      - new_lines.each do |line|
-        %td.diff_line= ""
+        %tr.line_holder.parallel
+          - if line.type == :file_deleted
+            %td.line_content.parallel= "File was deleted"
+          - elsif line.type == :added
+            %td.line_content{class: "parallel noteable_line new #{line.code}", "line_code" => line.code }= line.content
+          - else line.type == :no_change
+            %td.line_content.parallel= line.content
-        - 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= ""
+:javascript
+  $('.diff-side-right').on('scroll', function(){
+    $('.diff-side-left, .diff-middle').scrollTop($(this).scrollTop());
+    $('.diff-side-left').scrollLeft($(this).scrollLeft());
+  });
-      - 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
+  $('.diff-side-left').on('scroll', function(){
+    $('.diff-side-right, .diff-middle').scrollTop($(this).scrollTop()); // might never be relevant
+    $('.diff-side-right').scrollLeft($(this).scrollLeft());
+  });
	@@ -62,6 +62,29 @@		@@ -62,6 +62,29 @@
62	font-size: 12px;	62	font-size: 12px;
63	}	63	}
64	}	64	}
		65	+
		66	+ .text-file-parallel div {
		67	+ display: inline-block;
		68	+ padding-bottom: 16px;
		69	+ }
		70	+ .diff-side {
		71	+ overflow-x: scroll;
		72	+ width: 508px;
		73	+ height: 700px;
		74	+ }
		75	+ .diff-side.diff-side-left{
		76	+ overflow-y:hidden;
		77	+ }
		78	+ .diff-side table, td.diff-middle table {
		79	+ height: 700px;
		80	+ }
		81	+ .diff-middle {
		82	+ width: 114px;
		83	+ vertical-align: top;
		84	+ height: 700px;
		85	+ overflow: hidden
		86	+ }
		87	+
65	.old_line, .new_line, .diff_line {	88	.old_line, .new_line, .diff_line {
66	margin: 0px;	89	margin: 0px;
67	padding: 0px;	90	padding: 0px;
	@@ -125,8 +148,6 @@		@@ -125,8 +148,6 @@
125	}	148	}
126	&.parallel {	149	&.parallel {
127	display: table-cell;	150	display: table-cell;
128	- overflow: hidden;
129	- width: 50%;
130	}	151	}
131	}	152	}
132	}	153	}
	@@ -105,8 +105,80 @@ module CommitsHelper		@@ -105,8 +105,80 @@ module CommitsHelper
105	branches.sort.map { \|branch\| link_to(branch, project_tree_path(project, branch)) }.join(", ").html_safe	105	branches.sort.map { \|branch\| link_to(branch, project_tree_path(project, branch)) }.join(", ").html_safe
106	end	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	108	+ def parallel_diff_lines(project, commit, diff, file)
		109	+ old_file = project.repository.blob_at(commit.parent_id, diff.old_path) if commit.parent_id
		110	+ deleted_lines = {}
		111	+ added_lines = {}
		112	+ each_diff_line(diff, 0) do \|line, type, line_code, line_new, line_old\|
		113	+ if type == "old"
		114	+ deleted_lines[line_old] = { line_code: line_code, type: type, line: line }
		115	+ elsif type == "new"
		116	+ added_lines[line_new] = { line_code: line_code, type: type, line: line }
		117	+ end
		118	+ end
		119	+ max_length = old_file ? old_file.sloc + added_lines.length : file.sloc
		120	+
		121	+ offset1 = 0
		122	+ offset2 = 0
		123	+ old_lines = []
		124	+ new_lines = []
		125	+
		126	+ max_length.times do \|line_index\|
		127	+ line_index1 = line_index - offset1
		128	+ line_index2 = line_index - offset2
		129	+ deleted_line = deleted_lines[line_index1 + 1]
		130	+ added_line = added_lines[line_index2 + 1]
		131	+ old_line = old_file.lines[line_index1] if old_file
		132	+ new_line = file.lines[line_index2]
		133	+
		134	+ if deleted_line && added_line
		135	+ elsif deleted_line
		136	+ new_line = nil
		137	+ offset2 += 1
		138	+ elsif added_line
		139	+ old_line = nil
		140	+ offset1 += 1
		141	+ end
		142	+
		143	+ old_lines[line_index] = DiffLine.new
		144	+ new_lines[line_index] = DiffLine.new
		145	+
		146	+ # old
		147	+ if line_index == 0 && diff.new_file
		148	+ old_lines[line_index].type = :file_created
		149	+ old_lines[line_index].content = 'File was created'
		150	+ elsif deleted_line
		151	+ old_lines[line_index].type = :deleted
		152	+ old_lines[line_index].content = old_line
		153	+ old_lines[line_index].num = line_index1 + 1
		154	+ old_lines[line_index].code = deleted_line[:line_code]
		155	+ elsif old_line
		156	+ old_lines[line_index].type = :no_change
		157	+ old_lines[line_index].content = old_line
		158	+ old_lines[line_index].num = line_index1 + 1
		159	+ else
		160	+ old_lines[line_index].type = :added
		161	+ end
		162	+
		163	+ # new
		164	+ if line_index == 0 && diff.deleted_file
		165	+ new_lines[line_index].type = :file_deleted
		166	+ new_lines[line_index].content = "File was deleted"
		167	+ elsif added_line
		168	+ new_lines[line_index].type = :added
		169	+ new_lines[line_index].num = line_index2 + 1
		170	+ new_lines[line_index].content = new_line
		171	+ new_lines[line_index].code = added_line[:line_code]
		172	+ elsif new_line
		173	+ new_lines[line_index].type = :no_change
		174	+ new_lines[line_index].num = line_index2 + 1
		175	+ new_lines[line_index].content = new_line
		176	+ else
		177	+ new_lines[line_index].type = :deleted
		178	+ end
		179	+ end
		180	+
		181	+ return old_lines, new_lines
110	end	182	end
111		183
112	protected	184	protected
@@ -0,0 +1,3 @@		@@ -0,0 +1,3 @@
	1	+class DiffLine
	2	+ attr_accessor :type, :content, :num, :code
	3	+end
1	/ Side-by-side diff view	1	/ Side-by-side 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	2	+- old_lines, new_lines = parallel_diff_lines(project, @commit, diff, file)
		3	+- num_lines = old_lines.length
15		4
16	%div.text-file-parallel	5	%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]	6	+ %div.diff-side.diff-side-left
		7	+ %table
		8	+ - old_lines.each do \|line\|
		9	+
		10	+ %tr.line_holder.parallel
		11	+ - if line.type == :file_created
		12	+ %td.line_content.parallel= "File was created"
		13	+ - elsif line.type == :deleted
		14	+ %td.line_content{class: "parallel noteable_line old #{line.code}", "line_code" => line.code }= line.content
		15	+ - else line.type == :no_change
		16	+ %td.line_content.parallel= line.content
		17	+
		18	+ %div.diff-middle
		19	+ %table
		20	+ - num_lines.times do \|index\|
		21	+ %tr
		22	+ - if old_lines[index].type == :deleted
		23	+ %td.old_line.old= old_lines[index].num
		24	+ - else
		25	+ %td.old_line= old_lines[index].num
		26	+
		27	+ %td.diff_line=""
25		28
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	29	+ - if new_lines[index].type == :added
		30	+ %td.new_line.new= new_lines[index].num
		31	+ - else
		32	+ %td.new_line= new_lines[index].num
33		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= ""	34	+ %div.diff-side.diff-side-right
		35	+ %table
		36	+ - new_lines.each do \|line\|
50		37
51	- %td.diff_line= ""	38	+ %tr.line_holder.parallel
		39	+ - if line.type == :file_deleted
		40	+ %td.line_content.parallel= "File was deleted"
		41	+ - elsif line.type == :added
		42	+ %td.line_content{class: "parallel noteable_line new #{line.code}", "line_code" => line.code }= line.content
		43	+ - else line.type == :no_change
		44	+ %td.line_content.parallel= line.content
52		45
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= ""	46	+:javascript
		47	+ $('.diff-side-right').on('scroll', function(){
		48	+ $('.diff-side-left, .diff-middle').scrollTop($(this).scrollTop());
		49	+ $('.diff-side-left').scrollLeft($(this).scrollLeft());
		50	+ });
68		51
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
76	\ No newline at end of file	52	\ No newline at end of file
		53	+ $('.diff-side-left').on('scroll', function(){
		54	+ $('.diff-side-right, .diff-middle').scrollTop($(this).scrollTop()); // might never be relevant
		55	+ $('.diff-side-right').scrollLeft($(this).scrollLeft());
		56	+ });