From 856d4088fbc7da5b5b583acc11ab674c4452b402 Mon Sep 17 00:00:00 2001 From: Steven Thonus Date: Wed, 9 Oct 2013 22:22:37 +0200 Subject: [PATCH] diff view on commit with parallel diff view TODO: fix comment forms to respect left and right columns --- app/assets/stylesheets/sections/commits.scss | 16 +++- app/assets/stylesheets/sections/notes.scss | 9 +++ app/helpers/commits_helper.rb | 4 + app/views/projects/commits/_diffs.html.haml | 9 ++- .../projects/commits/_parallel_view.html.haml | 75 +++++++++++++++++++ .../projects/commits/_text_file.html.haml | 1 + .../_diff_notes_with_reply_parallel.html.haml | 34 +++++++++ features/project/commits/commits.feature | 6 ++ .../steps/project/project_browse_commits.rb | 13 ++++ 9 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 app/views/projects/commits/_parallel_view.html.haml create mode 100644 app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml diff --git a/app/assets/stylesheets/sections/commits.scss b/app/assets/stylesheets/sections/commits.scss index 769e95d591b..81f7ace66bf 100644 --- a/app/assets/stylesheets/sections/commits.scss +++ b/app/assets/stylesheets/sections/commits.scss @@ -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 { diff --git a/app/assets/stylesheets/sections/notes.scss b/app/assets/stylesheets/sections/notes.scss index 94b9ca3b181..fc84d157755 100644 --- a/app/assets/stylesheets/sections/notes.scss +++ b/app/assets/stylesheets/sections/notes.scss @@ -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 diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index c340eb30be1..654788a4c19 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -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 diff --git a/app/views/projects/commits/_diffs.html.haml b/app/views/projects/commits/_diffs.html.haml index f23403a50c4..aec7f15cdb7 100644 --- a/app/views/projects/commits/_diffs.html.haml +++ b/app/views/projects/commits/_diffs.html.haml @@ -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 diff --git a/app/views/projects/commits/_parallel_view.html.haml b/app/views/projects/commits/_parallel_view.html.haml new file mode 100644 index 00000000000..6585bd140a1 --- /dev/null +++ b/app/views/projects/commits/_parallel_view.html.haml @@ -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 diff --git a/app/views/projects/commits/_text_file.html.haml b/app/views/projects/commits/_text_file.html.haml index c724213878a..c827d96d855 100644 --- a/app/views/projects/commits/_text_file.html.haml +++ b/app/views/projects/commits/_text_file.html.haml @@ -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 + diff --git a/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml b/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml new file mode 100644 index 00000000000..936dbb354cd --- /dev/null +++ b/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml @@ -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= "" diff --git a/features/project/commits/commits.feature b/features/project/commits/commits.feature index fe470f5ac99..97113871a0a 100644 --- a/features/project/commits/commits.feature +++ b/features/project/commits/commits.feature @@ -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 diff --git a/features/steps/project/project_browse_commits.rb b/features/steps/project/project_browse_commits.rb index 650bc3a16f7..17c51dc9a96 100644 --- a/features/steps/project/project_browse_commits.rb +++ b/features/steps/project/project_browse_commits.rb @@ -88,4 +88,17 @@ class ProjectBrowseCommits < 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