Commit 856d4088fbc7da5b5b583acc11ab674c4452b402
Committed by
Steven Thonus
1 parent
c938833b
Exists in
master
and in
4 other branches
diff view on commit with parallel diff view
TODO: fix comment forms to respect left and right columns
Showing
9 changed files
with
165 additions
and
2 deletions
Show diff stats
app/assets/stylesheets/sections/commits.scss
@@ -70,7 +70,7 @@ | @@ -70,7 +70,7 @@ | ||
70 | font-size: 12px; | 70 | font-size: 12px; |
71 | } | 71 | } |
72 | } | 72 | } |
73 | - .old_line, .new_line { | 73 | + .old_line, .new_line, .diff_line { |
74 | margin: 0px; | 74 | margin: 0px; |
75 | padding: 0px; | 75 | padding: 0px; |
76 | border: none; | 76 | border: none; |
@@ -92,6 +92,15 @@ | @@ -92,6 +92,15 @@ | ||
92 | text-decoration: underline; | 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 | .line_holder { | 105 | .line_holder { |
97 | &.old .old_line, | 106 | &.old .old_line, |
@@ -122,6 +131,11 @@ | @@ -122,6 +131,11 @@ | ||
122 | color: #ccc; | 131 | color: #ccc; |
123 | background: #fafafa; | 132 | background: #fafafa; |
124 | } | 133 | } |
134 | + &.parallel { | ||
135 | + display: table-cell; | ||
136 | + overflow: hidden; | ||
137 | + width: 50%; | ||
138 | + } | ||
125 | } | 139 | } |
126 | } | 140 | } |
127 | .image { | 141 | .image { |
app/assets/stylesheets/sections/notes.scss
@@ -131,6 +131,11 @@ ul.notes { | @@ -131,6 +131,11 @@ ul.notes { | ||
131 | text-align: center; | 131 | text-align: center; |
132 | padding: 10px 0; | 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 | &.notes_content { | 139 | &.notes_content { |
135 | background-color: $white; | 140 | background-color: $white; |
136 | border-width: 1px 0; | 141 | border-width: 1px 0; |
@@ -358,3 +363,7 @@ ul.notes { | @@ -358,3 +363,7 @@ ul.notes { | ||
358 | .js-note-attachment-delete { | 363 | .js-note-attachment-delete { |
359 | display: none; | 364 | display: none; |
360 | } | 365 | } |
366 | + | ||
367 | +.parallel-comment { | ||
368 | + padding: 6px; | ||
369 | +} | ||
361 | \ No newline at end of file | 370 | \ No newline at end of file |
app/helpers/commits_helper.rb
@@ -105,6 +105,10 @@ module CommitsHelper | @@ -105,6 +105,10 @@ 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 | ||
110 | + end | ||
111 | + | ||
108 | protected | 112 | protected |
109 | 113 | ||
110 | # Private: Returns a link to a person. If the person has a matching user and | 114 | # Private: Returns a link to a person. If the person has a matching user and |
app/views/projects/commits/_diffs.html.haml
@@ -30,6 +30,10 @@ | @@ -30,6 +30,10 @@ | ||
30 | %strong.cgreen #{@commit.stats.additions} additions | 30 | %strong.cgreen #{@commit.stats.additions} additions |
31 | and | 31 | and |
32 | %strong.cred #{@commit.stats.deletions} deletions | 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 | .file-stats | 37 | .file-stats |
34 | = render "projects/commits/diff_head", diffs: diffs | 38 | = render "projects/commits/diff_head", diffs: diffs |
35 | 39 | ||
@@ -62,7 +66,10 @@ | @@ -62,7 +66,10 @@ | ||
62 | -# Skipp all non non-supported blobs | 66 | -# Skipp all non non-supported blobs |
63 | - next unless file.respond_to?('text?') | 67 | - next unless file.respond_to?('text?') |
64 | - if file.text? | 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 | - elsif file.image? | 73 | - elsif file.image? |
67 | - old_file = project.repository.blob_at(@commit.parent_id, diff.old_path) if @commit.parent_id | 74 | - old_file = project.repository.blob_at(@commit.parent_id, diff.old_path) if @commit.parent_id |
68 | = render "projects/commits/image", diff: diff, old_file: old_file, file: file, index: i | 75 | = render "projects/commits/image", diff: diff, old_file: old_file, file: file, index: i |
@@ -0,0 +1,75 @@ | @@ -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 | \ No newline at end of file | 76 | \ No newline at end of file |
app/views/projects/commits/_text_file.html.haml
@@ -21,3 +21,4 @@ | @@ -21,3 +21,4 @@ | ||
21 | - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at) | 21 | - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at) |
22 | - unless comments.empty? | 22 | - unless comments.empty? |
23 | = render "projects/notes/diff_notes_with_reply", notes: comments, line: line | 23 | = render "projects/notes/diff_notes_with_reply", notes: comments, line: line |
24 | + |
app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml
0 → 100644
@@ -0,0 +1,34 @@ | @@ -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= "" |
features/project/commits/commits.feature
@@ -14,6 +14,12 @@ Feature: Project Browse commits | @@ -14,6 +14,12 @@ Feature: Project Browse commits | ||
14 | Scenario: I browse commit from list | 14 | Scenario: I browse commit from list |
15 | Given I click on commit link | 15 | Given I click on commit link |
16 | Then I see commit info | 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 | Scenario: I compare refs | 24 | Scenario: I compare refs |
19 | Given I visit compare refs page | 25 | Given I visit compare refs page |
features/steps/project/project_browse_commits.rb
@@ -88,4 +88,17 @@ class ProjectBrowseCommits < Spinach::FeatureSteps | @@ -88,4 +88,17 @@ class ProjectBrowseCommits < Spinach::FeatureSteps | ||
88 | links[0]['href'].should =~ %r{blob/bc3735004cb45cec5e0e4fa92710897a910a5957} | 88 | links[0]['href'].should =~ %r{blob/bc3735004cb45cec5e0e4fa92710897a910a5957} |
89 | links[1]['href'].should =~ %r{blob/cc1ba255d6c5ffdce87a357ba7ccc397a4f4026b} | 89 | links[1]['href'].should =~ %r{blob/cc1ba255d6c5ffdce87a357ba7ccc397a4f4026b} |
90 | end | 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 | end | 104 | end |