FEATURE: compact the diff, and truncate cleanly

FEATURE: compact the diff, and truncate cleanly

Previously we echoed the subject on every commit, it was very noisy. Additionally, we would truncate without indicating which was confusing.

For @saj :partying_face:

From 780e5419bc567f0656087ab5bd22b5f6a580fbf4 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Wed, 12 Dec 2018 17:46:25 +1100
Subject: [PATCH] FEATURE: compact the diff, and truncate cleanly

Previously we echoed the subject on every commit, it was very noisy. Additionally, we would truncate without indicating which was confusing.

For @saj :partying_face:

diff --git a/lib/discourse_code_review/github_repo.rb b/lib/discourse_code_review/github_repo.rb
index 2ad3f47..38119c8 100644
--- a/lib/discourse_code_review/github_repo.rb
+++ b/lib/discourse_code_review/github_repo.rb
@@ -32,7 +32,7 @@ module DiscourseCodeReview
     end
 
     def last_commit
-      PluginStore.get(DiscourseCodeReview::PluginName, LastCommit + @name) ||
+      PluginStore.get(DiscourseCodeReview::PluginName, LastCommit + @name).presence ||
         begin
           commits = [SiteSetting.code_review_catch_up_commits, 1].max - 1
           (self.last_commit = git("rev-parse HEAD~#{commits}", backup_command: 'rev-list --max-parents=0 HEAD'))
@@ -83,8 +83,10 @@ module DiscourseCodeReview
 
     end
 
-    def commits_since(hash = nil)
-      git("pull")
+    def commits_since(hash = nil, merge_github_info: true, pull: true)
+      if pull
+        git("pull")
+      end
 
       hash ||= last_commit
 
@@ -92,9 +94,11 @@ module DiscourseCodeReview
 
       commits = git("log #{hash}.. --pretty=%H").split("\n").map { |x| x.strip }
 
-      commits.each_slice(30).each do |x|
-        commits = octokit_client.commits(@name, sha: x.first)
-        github_info.concat(commits)
+      if merge_github_info
+        commits.each_slice(30).each do |x|
+          github_commits = octokit_client.commits(@name, sha: x.first)
+          github_info.concat(github_commits)
+        end
       end
 
       lookup = {}
@@ -117,11 +121,13 @@ module DiscourseCodeReview
 
         hash = fields[0].strip
 
-        diff = git("show --format=email #{hash}")
+        diff = git("show --format=%b #{hash}")
 
-        abbrev = diff.length > MAX_DIFF_LENGTH
-        if abbrev
+        truncated = diff.length > MAX_DIFF_LENGTH
+        if truncated
           diff = diff[0..MAX_DIFF_LENGTH]
+          diff.strip!
+          diff = diff.split("\n")[0..-2].join("\n")
         end
 
         github_data = lookup[hash] || {}
@@ -134,7 +140,7 @@ module DiscourseCodeReview
           body: fields[4],
           date: Time.at(fields[5].to_i).to_datetime,
           diff: diff,
-          diff_abbrev: abbrev
+          diff_truncated: truncated
         }.merge(github_data)
 
       end.reverse
diff --git a/lib/discourse_code_review/importer.rb b/lib/discourse_code_review/importer.rb
index 4d5e624..876a779 100644
--- a/lib/discourse_code_review/importer.rb
+++ b/lib/discourse_code_review/importer.rb
@@ -42,7 +42,12 @@ module DiscourseCodeReview
       # we add a unicode zero width joiner so code block is not corrupted
       diff = commit[:diff].gsub('`‍``', "`\u200d``")
 
-      raw = "<div class='excerpt'>\n#{commit[:body]}\n</div>\n\n`‍``diff\n#{diff}\n`‍``\n#{link}"
+      truncated_message =
+        if commit[:diff_truncated]
+          "\n[... diff too long, it was truncated ...]"
+        end
+
+      raw = "<div class='excerpt'>\n#{commit[:body]}\n</div>\n\n`‍``diff\n#{diff}\n#{truncated_message}`‍``\n#{link}"
 
       user = ensure_user(
         email: commit[:email],
diff --git a/spec/discourse_code_review/lib/github_repo_spec.rb b/spec/discourse_code_review/lib/github_repo_spec.rb
index 5dd1f63..969e0da 100644
--- a/spec/discourse_code_review/lib/github_repo_spec.rb
+++ b/spec/discourse_code_review/lib/github_repo_spec.rb
@@ -12,6 +12,31 @@ module DiscourseCodeReview
       FileUtils.rm_rf(@git_path)
     end
 
+    it "can cleanly truncate diffs" do
+      Dir.chdir(@git_path) do
+        `git init .`
+        File.write('a', "hello world\n" * 1000)
+        `git add a`
+        `git commit -am 'first commit'`
+        File.write('a', 'hello2')
+        `git commit -am 'second commit'`
+
+        repo = GithubRepo.new('fake_repo/fake_repo', nil)
+
+        repo.path = @git_path
+        repo.last_commit = nil
+
+        last_commit = repo.commits_since(nil, merge_github_info: false, pull: false).last
+        diff = last_commit[:diff]
+
+        expect(last_commit[:diff_truncated]).to eq(true)
+        expect(diff).to ending_with("hello world")
+
+        # no point repeating the message
+        expect(diff).not_to include("second commit")
+      end
+    end
+
     it "can respect catchup commits" do
       Dir.chdir(@git_path) do
         `git init .`

GitHub

2 Likes