From 65c227e8c13a88d217a0b9c2f15bfa4aba9377ba Mon Sep 17 00:00:00 2001 From: Jan Bader Date: Tue, 30 Sep 2014 15:41:09 +0200 Subject: [PATCH 01/10] Use jQuery's .on() --- public/coffee/commit.coffee | 42 ++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/public/coffee/commit.coffee b/public/coffee/commit.coffee index 60ecaa17..2d3ffaf3 100644 --- a/public/coffee/commit.coffee +++ b/public/coffee/commit.coffee @@ -11,28 +11,28 @@ window.Commit = SIDE_BY_SIDE_CODE_WIDTH: 830 init: -> - $(".addCommentButton").click (e) => @onAddCommentMouseAction e + $(document).on "click", ".addCommentButton", (e) => @onAddCommentMouseAction e $("a.tipsyCommentCount").tipsy(gravity: "w") - $(".diffLine").dblclick (e) => @onAddCommentMouseAction e - $(".reply").live "click", (e) => @onAddCommentMouseAction e - $(".diffLine").hover(((e) => @selectLine(e)), ((e) => @clearSelectedLine())) - $(".commentForm").live "submit", (e) => @onCommentSubmit e - $(".commentPreview").click (e) => @onCommentPreview e - $(".commentEditForm").live "submit", (e) => @onCommentEditSubmit e - $("#approveButton").live "click", (e) => @onApproveClicked e - $("#disapproveButton").live "click", (e) => @onDisapproveClicked e - $(".delete").live "click", (e) => @onCommentDelete e - $(".edit").live "click", (e) => @onCommentEdit e - $("#sideBySideButton").live "click", => @toggleSideBySide true - $("#requestReviewButton").click (e) => @toggleReviewRequest() - $("#hideCommentButton").live "click", (e) => @toggleComments() - $(".diffCommentCount > a").live "click", (e) => @toggleSingleComment(e) - $("#requestInput button").click (e) => @submitReviewRequest() - $(".expandLink.all").click (e) => @expandContextAll(e) - $(".expandLink.below").click (e) => @expandContext(e, 10, "below") - $(".expandLink.above").click (e) => @expandContext(e, 10, "above") - $("#commit .file").on("mouseenter", ".contextExpander", @expandContextHoverIn) - $("#commit .file").on("mouseleave", ".contextExpander", @expandContextHoverOut) + $(document).on "dblclick", ".diffLine", (e) => @onAddCommentMouseAction e + $(document).on "click", ".reply", (e) => @onAddCommentMouseAction e + $(document).on "hover", ".diffLine", (((e) => @selectLine(e)), ((e) => @clearSelectedLine())) + $(document).on "submit", ".commentForm", (e) => @onCommentSubmit e + $(document).on "click", ".commentPreview", (e) => @onCommentPreview e + $(document).on "submit", ".commentEditForm", (e) => @onCommentEditSubmit e + $(document).on "click", "#approveButton", (e) => @onApproveClicked e + $(document).on "click", "#disapproveButton", (e) => @onDisapproveClicked e + $(document).on "click", ".delete", (e) => @onCommentDelete e + $(document).on "click", ".edit", (e) => @onCommentEdit e + $(document).on "click", "#sideBySideButton", => @toggleSideBySide true + $(document).on "click", "#requestReviewButton", (e) => @toggleReviewRequest() + $(document).on "click", "#hideCommentButton", (e) => @toggleComments() + $(document).on "click", ".diffCommentCount > a", (e) => @toggleSingleComment(e) + $(document).on "click", "#requestInput button", (e) => @submitReviewRequest() + $(document).on "click", ".expandLink.all", (e) => @expandContextAll(e) + $(document).on "click", ".expandLink.below", (e) => @expandContext(e, 10, "below") + $(document).on "click", ".expandLink.above", (e) => @expandContext(e, 10, "above") + $(document).on "mouseenter", "#commit .file .contextExpander", @expandContextHoverIn + $(document).on "mouseleave", "#commit .file .contextExpander", @expandContextHoverOut @currentlyScrollingTimer = null From 3ddbfe140cb7b95f8fc6349dbf12a32c80e85e8c Mon Sep 17 00:00:00 2001 From: Jan Bader Date: Tue, 30 Sep 2014 16:05:43 +0200 Subject: [PATCH 02/10] Add braces --- public/coffee/commit.coffee | 42 ++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/public/coffee/commit.coffee b/public/coffee/commit.coffee index 2d3ffaf3..ec35eff1 100644 --- a/public/coffee/commit.coffee +++ b/public/coffee/commit.coffee @@ -11,28 +11,28 @@ window.Commit = SIDE_BY_SIDE_CODE_WIDTH: 830 init: -> - $(document).on "click", ".addCommentButton", (e) => @onAddCommentMouseAction e + $(document).on("click", ".addCommentButton", (e) => @onAddCommentMouseAction e) $("a.tipsyCommentCount").tipsy(gravity: "w") - $(document).on "dblclick", ".diffLine", (e) => @onAddCommentMouseAction e - $(document).on "click", ".reply", (e) => @onAddCommentMouseAction e - $(document).on "hover", ".diffLine", (((e) => @selectLine(e)), ((e) => @clearSelectedLine())) - $(document).on "submit", ".commentForm", (e) => @onCommentSubmit e - $(document).on "click", ".commentPreview", (e) => @onCommentPreview e - $(document).on "submit", ".commentEditForm", (e) => @onCommentEditSubmit e - $(document).on "click", "#approveButton", (e) => @onApproveClicked e - $(document).on "click", "#disapproveButton", (e) => @onDisapproveClicked e - $(document).on "click", ".delete", (e) => @onCommentDelete e - $(document).on "click", ".edit", (e) => @onCommentEdit e - $(document).on "click", "#sideBySideButton", => @toggleSideBySide true - $(document).on "click", "#requestReviewButton", (e) => @toggleReviewRequest() - $(document).on "click", "#hideCommentButton", (e) => @toggleComments() - $(document).on "click", ".diffCommentCount > a", (e) => @toggleSingleComment(e) - $(document).on "click", "#requestInput button", (e) => @submitReviewRequest() - $(document).on "click", ".expandLink.all", (e) => @expandContextAll(e) - $(document).on "click", ".expandLink.below", (e) => @expandContext(e, 10, "below") - $(document).on "click", ".expandLink.above", (e) => @expandContext(e, 10, "above") - $(document).on "mouseenter", "#commit .file .contextExpander", @expandContextHoverIn - $(document).on "mouseleave", "#commit .file .contextExpander", @expandContextHoverOut + $(document).on("dblclick", ".diffLine", (e) => @onAddCommentMouseAction e) + $(document).on("click", ".reply", (e) => @onAddCommentMouseAction e) + $(document).on("hover", ".diffLine", (((e) => @selectLine(e)), ((e) => @clearSelectedLine()))) + $(document).on("submit", ".commentForm", (e) => @onCommentSubmit e) + $(document).on("click", ".commentPreview", (e) => @onCommentPreview e) + $(document).on("submit", ".commentEditForm", (e) => @onCommentEditSubmit e) + $(document).on("click", "#approveButton", (e) => @onApproveClicked e) + $(document).on("click", "#disapproveButton", (e) => @onDisapproveClicked e) + $(document).on("click", ".delete", (e) => @onCommentDelete e) + $(document).on("click", ".edit", (e) => @onCommentEdit e) + $(document).on("click", "#sideBySideButton", => @toggleSideBySide true) + $(document).on("click", "#requestReviewButton", (e) => @toggleReviewRequest()) + $(document).on("click", "#hideCommentButton", (e) => @toggleComments()) + $(document).on("click", ".diffCommentCount > a", (e) => @toggleSingleComment(e)) + $(document).on("click", "#requestInput button", (e) => @submitReviewRequest()) + $(document).on("click", ".expandLink.all", (e) => @expandContextAll(e)) + $(document).on("click", ".expandLink.below", (e) => @expandContext(e, 10, "below")) + $(document).on("click", ".expandLink.above", (e) => @expandContext(e, 10, "above")) + $(document).on("mouseenter", "#commit .file .contextExpander", @expandContextHoverIn) + $(document).on("mouseleave", "#commit .file .contextExpander", @expandContextHoverOut) @currentlyScrollingTimer = null From 479a156cc02e08311d2b72caf38cd6865fd753f0 Mon Sep 17 00:00:00 2001 From: Jan Bader Date: Tue, 30 Sep 2014 16:30:33 +0200 Subject: [PATCH 03/10] Use jQuery's on for expands --- public/coffee/commit.coffee | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/public/coffee/commit.coffee b/public/coffee/commit.coffee index ec35eff1..d622f3e2 100644 --- a/public/coffee/commit.coffee +++ b/public/coffee/commit.coffee @@ -15,7 +15,8 @@ window.Commit = $("a.tipsyCommentCount").tipsy(gravity: "w") $(document).on("dblclick", ".diffLine", (e) => @onAddCommentMouseAction e) $(document).on("click", ".reply", (e) => @onAddCommentMouseAction e) - $(document).on("hover", ".diffLine", (((e) => @selectLine(e)), ((e) => @clearSelectedLine()))) + $(document).on("mouseenter", ".diffLine", (e) => @selectLine(e)) + $(document).on("mouseleave", ".diffLine", (e) => @clearSelectedLine()) $(document).on("submit", ".commentForm", (e) => @onCommentSubmit e) $(document).on("click", ".commentPreview", (e) => @onCommentPreview e) $(document).on("submit", ".commentEditForm", (e) => @onCommentEditSubmit e) @@ -315,9 +316,9 @@ window.Commit = createContextExpander: (codeLines, attachDirection, top, bottom, incremental, refreshLine) -> renderedExpander = Snippets.contextExpander(top, bottom, codeLines.attr("diff-line-number"), incremental) contextExpander = $(renderedExpander) - contextExpander.find(".expandLink.all").click (e) => @expandContextAll(e) - contextExpander.find(".expandLink.above").click (e) => @expandContext(e, 10, "above") - contextExpander.find(".expandLink.below").click (e) => @expandContext(e, 10, "below") + contextExpander.on("click", ".expandLink.all", (e) => @expandContextAll(e)) + contextExpander.on("click", ".expandLink.above", (e) => @expandContext(e, 10, "above")) + contextExpander.on("click", ".expandLink.below", (e) => @expandContext(e, 10, "below")) codeLines.before(contextExpander) if attachDirection == "above" codeLines.after(contextExpander) if attachDirection == "below" expander = if attachDirection == "above" then codeLines.prev() else codeLines.next() From a83c377cc491ded6edd8ed0144af9cc2475349dc Mon Sep 17 00:00:00 2001 From: Jan Bader Date: Tue, 30 Sep 2014 16:48:47 +0200 Subject: [PATCH 04/10] Strip huge files from diff (Fixes #220) --- lib/git_diff_utils.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/git_diff_utils.rb b/lib/git_diff_utils.rb index 56d79a7f..7dc186d2 100644 --- a/lib/git_diff_utils.rb +++ b/lib/git_diff_utils.rb @@ -41,6 +41,8 @@ def self.get_tagged_commit_diffs(repo_name, commit, options = {}) data.special_case = "This is an empty file." elsif diff.renamed_file && diff.diff.empty? data.special_case = "File was renamed, but no other changes were made." + elsif diff.a_blob.data.length > 50000 || diff.b_blob.data.length > 50000 + data.special_case = "File is too big to show diff." else if options[:use_syntax_highlighting] || options[:warm_the_cache] begin From 5d84adc61c877891df22fdf13e139178113e738c Mon Sep 17 00:00:00 2001 From: Jan Bader Date: Tue, 30 Sep 2014 17:54:53 +0200 Subject: [PATCH 05/10] Fix trying to acces data of Nil --- lib/git_diff_utils.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/git_diff_utils.rb b/lib/git_diff_utils.rb index 7dc186d2..ae8b8bc5 100644 --- a/lib/git_diff_utils.rb +++ b/lib/git_diff_utils.rb @@ -23,6 +23,7 @@ def self.setup(redis) def self.get_tagged_commit_diffs(repo_name, commit, options = {}) repo = MetaRepo.instance.get_grit_repo(repo_name) begin + total_size = 0 GitDiffUtils.show(repo, commit).map do |diff| a_path = diff.a_path b_path = diff.b_path @@ -41,7 +42,9 @@ def self.get_tagged_commit_diffs(repo_name, commit, options = {}) data.special_case = "This is an empty file." elsif diff.renamed_file && diff.diff.empty? data.special_case = "File was renamed, but no other changes were made." - elsif diff.a_blob.data.length > 50000 || diff.b_blob.data.length > 50000 + elsif total_size > 1000000 + data.special_case = "Maximum diff size reached." + elsif !diff.a_blob.nil? && diff.a_blob.data.length > 200000 || !diff.b_blob.nil? && diff.b_blob.data.length > 200000 data.special_case = "File is too big to show diff." else if options[:use_syntax_highlighting] || options[:warm_the_cache] @@ -59,6 +62,7 @@ def self.get_tagged_commit_diffs(repo_name, commit, options = {}) # Diffs can be missing a_blob or b_blob if the change is an added or removed file. before, after = [diff.a_blob, diff.b_blob].map { |blob| blob ? blob.data : "" } end + total_size += [before.length, after.length].max raw_diff = GitDiffUtils.diff(diff.a_blob, diff.b_blob) unless options[:warm_the_cache] From 2e034a75133de66ffa72d0a95240dab6cea479b5 Mon Sep 17 00:00:00 2001 From: Jan Bader Date: Tue, 7 Oct 2014 16:01:20 +0200 Subject: [PATCH 06/10] Ignore files matching /\.(gen|properties|relations|functions|[dD]esigner)\.cs/) --- lib/git_diff_utils.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/git_diff_utils.rb b/lib/git_diff_utils.rb index ae8b8bc5..fee3cd2e 100644 --- a/lib/git_diff_utils.rb +++ b/lib/git_diff_utils.rb @@ -34,7 +34,9 @@ def self.get_tagged_commit_diffs(repo_name, commit, options = {}) if GitHelper.blob_binary?(diff.a_blob) || GitHelper.blob_binary?(diff.b_blob) data.binary = true data.special_case = "This is a binary file." - elsif data.submodule? + elsif !(a_path =~ /\.(gen|properties|relations|functions|[dD]esigner)\.cs/).nil? + data.special_case = "Generated File - not showing diff." + elsif data.submodule? data.special_case = data.submodule_special_case_message elsif diff.diff.nil? && (data.file_mode_before != data.file_mode_after) data.special_case = "File mode changed: #{data.file_mode_before} → #{data.file_mode_after}" From 220034822f0a7efa76e5ce10a9b305d20645ff8a Mon Sep 17 00:00:00 2001 From: Jan Bader Date: Tue, 7 Oct 2014 16:01:44 +0200 Subject: [PATCH 07/10] Increase FileSize limit to 2M --- lib/git_diff_utils.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/git_diff_utils.rb b/lib/git_diff_utils.rb index fee3cd2e..99a8d9f2 100644 --- a/lib/git_diff_utils.rb +++ b/lib/git_diff_utils.rb @@ -46,7 +46,7 @@ def self.get_tagged_commit_diffs(repo_name, commit, options = {}) data.special_case = "File was renamed, but no other changes were made." elsif total_size > 1000000 data.special_case = "Maximum diff size reached." - elsif !diff.a_blob.nil? && diff.a_blob.data.length > 200000 || !diff.b_blob.nil? && diff.b_blob.data.length > 200000 + elsif !diff.a_blob.nil? && diff.a_blob.data.length > 2000000 || !diff.b_blob.nil? && diff.b_blob.data.length > 2000000 data.special_case = "File is too big to show diff." else if options[:use_syntax_highlighting] || options[:warm_the_cache] From c974b70b14df614e8ea20e00af51c34f3da78dcf Mon Sep 17 00:00:00 2001 From: Jan Bader Date: Wed, 8 Oct 2014 15:48:08 +0200 Subject: [PATCH 08/10] Also ignore PI_, DBSchema; Do not ignore Designer.cs --- lib/git_diff_utils.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/git_diff_utils.rb b/lib/git_diff_utils.rb index 99a8d9f2..4d41aaa9 100644 --- a/lib/git_diff_utils.rb +++ b/lib/git_diff_utils.rb @@ -34,7 +34,7 @@ def self.get_tagged_commit_diffs(repo_name, commit, options = {}) if GitHelper.blob_binary?(diff.a_blob) || GitHelper.blob_binary?(diff.b_blob) data.binary = true data.special_case = "This is a binary file." - elsif !(a_path =~ /\.(gen|properties|relations|functions|[dD]esigner)\.cs/).nil? + elsif !(a_path =~ /\.(gen|properties|relations|functions)\.cs/).nil? || !(a_path =~ /PI_/).nil? || !(a_path =~ /DBSchema.xml$/).nil? data.special_case = "Generated File - not showing diff." elsif data.submodule? data.special_case = data.submodule_special_case_message From 25b0fe6a80a35c9bafe14b81baf9f72820e9373e Mon Sep 17 00:00:00 2001 From: Jan Bader Date: Wed, 8 Oct 2014 15:50:56 +0200 Subject: [PATCH 09/10] Ignore spaces --- lib/git_diff_utils.rb | 9 ++++++--- lib/git_helper.rb | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/git_diff_utils.rb b/lib/git_diff_utils.rb index 4d41aaa9..1242ebc8 100644 --- a/lib/git_diff_utils.rb +++ b/lib/git_diff_utils.rb @@ -65,7 +65,10 @@ def self.get_tagged_commit_diffs(repo_name, commit, options = {}) before, after = [diff.a_blob, diff.b_blob].map { |blob| blob ? blob.data : "" } end total_size += [before.length, after.length].max - raw_diff = GitDiffUtils.diff(diff.a_blob, diff.b_blob) + + raw_diff = diff.diff + raw_diff.gsub! "\r", "" + raw_diff.sub %r{[^@]*}, "" unless options[:warm_the_cache] new_data = GitDiffUtils.tag_file(before, after, diff, raw_diff) @@ -197,10 +200,10 @@ def self.tag_diff(diff, raw_diff, before_highlighted, after_highlighted) def self.show(repo, commit) if commit.parents.size > 0 - diff = repo.git.native(:diff, { :full_index => true, :find_renames => true }, commit.parents[0].id, + diff = repo.git.native(:diff, { :ignore_all_space=> true, :full_index => true, :find_renames => true }, commit.parents[0].id, commit.id) else - raw_diff = repo.git.native(:show, { :full_index => true, :pretty => "raw" }, commit.id) + raw_diff = repo.git.native(:show, { :ignore_all_space=> true, :full_index => true, :pretty => "raw" }, commit.id) # git show has a lot of headers in the diff, we try to strip it out here cutpoint = raw_diff.index("diff --git a") diff = cutpoint ? raw_diff[cutpoint, raw_diff.length] : "" diff --git a/lib/git_helper.rb b/lib/git_helper.rb index 328cf320..5a220584 100644 --- a/lib/git_helper.rb +++ b/lib/git_helper.rb @@ -58,4 +58,4 @@ def self.rev_list(repo, command_options, mode = :commits) def self.blob_binary?(blob) blob && !blob.data.empty? && blob.data.index("\0") end -end \ No newline at end of file +end From 2381a149b95efc801bca4d12ed80e47037b7a15a Mon Sep 17 00:00:00 2001 From: Jan Bader Date: Wed, 8 Oct 2014 17:57:16 +0200 Subject: [PATCH 10/10] Only send actual diffs without context --- lib/git_diff_utils.rb | 27 ++------------------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/lib/git_diff_utils.rb b/lib/git_diff_utils.rb index 1242ebc8..19416430 100644 --- a/lib/git_diff_utils.rb +++ b/lib/git_diff_utils.rb @@ -23,7 +23,6 @@ def self.setup(redis) def self.get_tagged_commit_diffs(repo_name, commit, options = {}) repo = MetaRepo.instance.get_grit_repo(repo_name) begin - total_size = 0 GitDiffUtils.show(repo, commit).map do |diff| a_path = diff.a_path b_path = diff.b_path @@ -44,10 +43,6 @@ def self.get_tagged_commit_diffs(repo_name, commit, options = {}) data.special_case = "This is an empty file." elsif diff.renamed_file && diff.diff.empty? data.special_case = "File was renamed, but no other changes were made." - elsif total_size > 1000000 - data.special_case = "Maximum diff size reached." - elsif !diff.a_blob.nil? && diff.a_blob.data.length > 2000000 || !diff.b_blob.nil? && diff.b_blob.data.length > 2000000 - data.special_case = "File is too big to show diff." else if options[:use_syntax_highlighting] || options[:warm_the_cache] begin @@ -64,7 +59,6 @@ def self.get_tagged_commit_diffs(repo_name, commit, options = {}) # Diffs can be missing a_blob or b_blob if the change is an added or removed file. before, after = [diff.a_blob, diff.b_blob].map { |blob| blob ? blob.data : "" } end - total_size += [before.length, after.length].max raw_diff = diff.diff raw_diff.gsub! "\r", "" @@ -98,27 +92,10 @@ def self.tag_file(file_before, file_after, diff, raw_diff) chunks = tag_diff(diff, raw_diff, before_lines, after_lines) chunks.each_with_index do |chunk, i| - if chunk.original_line_start && chunk.original_line_start > orig_line - tagged_lines += before_lines[orig_line...chunk.original_line_start].map do |data| - diff_line += 1 - orig_line += 1 - LineDiff.new(:same, before_lines[orig_line - 1], orig_line, diff_line) - end - chunk_breaks << tagged_lines.size - end tagged_lines += chunk.tagged_lines orig_line += chunk.original_lines_changed diff_line += chunk.new_lines_changed end - - if !before_lines.empty? && orig_line <= before_lines.count - chunk_breaks << tagged_lines.size - tagged_lines += before_lines[orig_line..before_lines.count].map do |data| - diff_line += 1 - orig_line += 1 - LineDiff.new(:same, before_lines[orig_line - 1], orig_line, diff_line ) - end - end lines_added = tagged_lines.select { |line| line.tag == :added }.count lines_removed = tagged_lines.select { |line| line.tag == :removed }.count { :lines => tagged_lines, :breaks => chunk_breaks, @@ -200,10 +177,10 @@ def self.tag_diff(diff, raw_diff, before_highlighted, after_highlighted) def self.show(repo, commit) if commit.parents.size > 0 - diff = repo.git.native(:diff, { :ignore_all_space=> true, :full_index => true, :find_renames => true }, commit.parents[0].id, + diff = repo.git.native(:diff, { :inter_hunk_context => 33, :unified => 9, :ignore_all_space => true, :full_index => true, :find_renames => true }, commit.parents[0].id, commit.id) else - raw_diff = repo.git.native(:show, { :ignore_all_space=> true, :full_index => true, :pretty => "raw" }, commit.id) + raw_diff = repo.git.native(:show, { :inter_hunk_context => 33, :unified => 9, :ignore_all_space => true, :full_index => true, :pretty => "raw" }, commit.id) # git show has a lot of headers in the diff, we try to strip it out here cutpoint = raw_diff.index("diff --git a") diff = cutpoint ? raw_diff[cutpoint, raw_diff.length] : ""