From b4cc04d7e17eeefe6d89bbb72661a43d7d9e3e2e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 25 Jan 2012 21:10:09 +0200 Subject: [PATCH] Commit diff fixes, per-line comments fixed --- app/helpers/commits_helper.rb | 63 ++++++++++++++++++++++++++-------- app/models/note.rb | 17 --------- app/views/commits/_text_file.html.haml | 51 ++++++++------------------- app/views/commits/show.html.haml | 2 +- spec/models/note_spec.rb | 16 +-------- 5 files changed, 66 insertions(+), 83 deletions(-) diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 4607e9daa..b56fce17b 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -7,16 +7,6 @@ module CommitsHelper end - def diff_line_class(line) - if line[0] == "+" - "new" - elsif line[0] == "-" - "old" - else - nil - end - end - def more_commits_link offset = params[:offset] || 0 limit = params[:limit] || 100 @@ -42,11 +32,56 @@ module CommitsHelper preserve out end - def build_line_code(line, index, line_new, line_old) - if diff_line_class(line) == "new" - "NEW_#{index}_#{line_new}" + def diff_line_class(line) + if line[0] == "+" + "new" + elsif line[0] == "-" + "old" else - "OLD_#{index}_#{line_old}" + nil + end + end + + def build_line_code(line, index, line_new, line_old) + "#{index}_#{line_old}_#{line_new}" + end + + def each_diff_line(diff_arr, index) + line_old = 1 + line_new = 1 + type = nil + + lines_arr = diff_arr + lines_arr.each do |line| + full_line = html_escape(line.gsub(/\n/, '')) + + next if line.match(/^--- \/dev\/null/) + next if line.match(/^--- a/) + next if line.match(/^\+\+\+ b/) + if line.match(/^@@ -/) + next if line_old == 1 && line_new == 1 + type = "match" + + line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0 + line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0 + + yield(nil, type, nil, nil, nil) + next + else + type = diff_line_class(line) + line_code = build_line_code(line, index, line_new, line_old) + yield(full_line, type, line_code, line_new, line_old) + end + + + if line[0] == "+" + line_new += 1 + elsif line[0] == "-" + line_old += 1 + else + line_new += 1 + line_old += 1 + end end end end diff --git a/app/models/note.rb b/app/models/note.rb index f499b62d3..01a550708 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -57,23 +57,6 @@ class Note < ActiveRecord::Base rescue nil end - - def line_file_id - @line_file_id ||= line_code.split("_")[1].to_i if line_code - end - - def line_type_id - @line_type_id ||= line_code.split("_").first if line_code - end - - def line_number - @line_number ||= line_code.split("_").last.to_i if line_code - end - - def for_line?(file_id, old_line, new_line) - line_file_id == file_id && - ((line_type_id == "NEW" && line_number == new_line) || (line_type_id == "OLD" && line_number == old_line )) - end end # == Schema Information # diff --git a/app/views/commits/_text_file.html.haml b/app/views/commits/_text_file.html.haml index 513efdf02..bff578c0e 100644 --- a/app/views/commits/_text_file.html.haml +++ b/app/views/commits/_text_file.html.haml @@ -1,38 +1,17 @@ %table - - line_old = 0 - - line_new = 0 - - diff_str = diff.diff - - lines_arr = diff_str.lines.to_a - - lines_arr.each do |line| - - next if line.match(/^--- \/dev\/null/) - - next if line.match(/^--- a/) - - next if line.match(/^\+\+\+ b/) - - if line.match(/^@@ -/) - - unless line_old.zero? && line_new.zero? - %tr.line_holder - %td.old_line= "..." - %td.new_line= "..." - %td.line_content   - - - line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0 - - line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0 - - next - - - full_line = html_escape(line.gsub(/\n/, '')) + - each_diff_line(diff.diff.lines.to_a, index) do |line, type, line_code, line_new, line_old| %tr.line_holder - %td.old_line - = link_to raw(diff_line_class(line) == "new" ? " " : line_old), "#OLD#{index}-#{line_old}", :id => "OLD#{index}-#{line_old}" - %td.new_line - = link_to raw(diff_line_class(line) == "old" ? " " : line_new) , "#NEW#{index}-#{line_new}", :id => "NEW#{index}-#{line_new}" - %td.line_content{:class => "#{diff_line_class(full_line)} #{build_line_code(line, index, line_new, line_old)}", "line_code" => build_line_code(line, index, line_new, line_old)}= raw "#{full_line}  " - - comments = @line_notes.select { |n| n.for_line?(index, line_old, line_new) }.sort_by(&:created_at).reverse - - unless comments.empty? - - comments.each do |note| - = render "notes/per_line_show", :note => note - - if line[0] == "+" - - line_new += 1 - - elsif line[0] == "-" - - line_old += 1 - - else - - line_new += 1 - - line_old += 1 + - if type == "match" + %td.old_line= "..." + %td.new_line= "..." + %td.line_content   + - else + %td.old_line= link_to raw(type == "new" ? " " : line_old), "##{line_code}", :id => line_code + %td.new_line= link_to raw(type == "old" ? " " : line_new) , "##{line_code}", :id => line_code + %td.line_content{:class => "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw "#{line}  " + + - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at).reverse + - unless comments.empty? + - comments.each do |note| + = render "notes/per_line_show", :note => note + - @line_notes.reject!{ |n| n == note } diff --git a/app/views/commits/show.html.haml b/app/views/commits/show.html.haml index 3e847e78d..6010f01f6 100644 --- a/app/views/commits/show.html.haml +++ b/app/views/commits/show.html.haml @@ -29,7 +29,7 @@ :javascript $(document).ready(function(){ - $(".line_content").live("dblclick", function(e) { + $(".noteable_line").live("dblclick", function(e) { var form = $(".per_line_form"); $(this).parent().after(form); form.find("#note_line_code").val($(this).attr("line_code")); diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 75503fd9a..44a0ee194 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -42,27 +42,13 @@ describe Note do :project => project, :noteable_id => commit.id, :noteable_type => "Commit", - :line_code => "OLD_1_23" + :line_code => "0_16_1" end it "should save a valid note" do @note.noteable_id.should == commit.id @note.target.id.should == commit.id end - - it { @note.line_type_id.should == "OLD" } - it { @note.line_file_id.should == 1 } - it { @note.line_number.should == 23 } - - it { @note.for_line?(1, 23, 34).should be_true } - it { @note.for_line?(1, 23, nil).should be_true } - it { @note.for_line?(1, 23, 0).should be_true } - it { @note.for_line?(1, 23, 23).should be_true } - - it { @note.for_line?(1, nil, 34).should be_false } - it { @note.for_line?(1, 24, nil).should be_false } - it { @note.for_line?(1, 24, 0).should be_false } - it { @note.for_line?(1, 24, 23).should be_false } end describe :authorization do -- 2.11.0