Add patch diff highlighting and line stats #2 (#64)

Author: ImTheKai

app/assets/stylesheets/components/messages.css

@@ -252,6 +252,35 @@
     color: var(--color-text-secondary);
     font-weight: var(--font-weight-semibold);
   }
+
+  & code.diff-highlighted {
+    display: block;
+  }
+
+  & .diff-line {
+    display: block;
+    padding: 0 var(--spacing-2);
+    margin: 0 calc(-1 * var(--spacing-2));
+  }
+
+  & .diff-line-add {
+    background: var(--color-success-soft);
+    color: var(--color-success);
+  }
+
+  & .diff-line-del {
+    background: var(--color-danger-soft);
+    color: var(--color-danger);
+  }
+
+  & .diff-line-hunk {
+    background: var(--color-info-soft);
+    color: var(--color-info);
+  }
+
+  & .diff-line-header {
+    color: var(--color-text-muted);
+  }
 }
 
 .attachment {
@@ -286,6 +315,24 @@ summary.attachment-info {
   text-decoration: none;
 }
 
+.patchset-stats {
+  display: inline-flex;
+  align-items: center;
+  gap: var(--spacing-2);
+  margin-left: var(--spacing-2);
+  font-size: var(--font-size-xs);
+  font-weight: var(--font-weight-semibold);
+  white-space: nowrap;
+}
+
+.patchset-added {
+  color: var(--color-success);
+}
+
+.patchset-removed {
+  color: var(--color-danger);
+}
+
 .attachment-download:hover {
   color: var(--color-text-link-hover);
   text-decoration: underline;

app/assets/stylesheets/components/sidebar.css

@@ -176,6 +176,31 @@
   margin: var(--spacing-3) 0;
 }
 
+.sidebar .download-patchset {
+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+  gap: var(--spacing-3);
+  margin-bottom: var(--spacing-4);
+}
+
+.sidebar .patchset-stats {
+  display: inline-flex;
+  align-items: center;
+  gap: var(--spacing-2);
+  font-size: var(--font-size-sm);
+  font-weight: var(--font-weight-semibold);
+  white-space: nowrap;
+}
+
+.sidebar .patchset-added {
+  color: var(--color-success);
+}
+
+.sidebar .patchset-removed {
+  color: var(--color-danger);
+}
+
 .sidebar .attachments-list li + li {
   margin-top: var(--spacing-2);
 }

app/controllers/topics_controller.rb

@@ -65,6 +65,19 @@ def show
     end
 
     @has_patches = @messages.any? { |msg| msg.attachments.any?(&:patch_extension?) }
+    if @has_patches
+      latest_message = latest_patchset_message
+      if latest_message
+        patch_attachments = latest_message.attachments.select(&:patch?)
+        totals = patch_attachments.reduce({ added: 0, removed: 0 }) do |acc, attachment|
+          stats = attachment.diff_line_stats
+          acc[:added] += stats[:added]
+          acc[:removed] += stats[:removed]
+          acc
+        end
+        @latest_patchset_stats = totals
+      end
+    end
   end
 
   def aware
@@ -137,11 +150,7 @@ def unstar
   end
 
   def latest_patchset
-    latest_message = @topic.messages
-                           .where(id: Attachment.where(message_id: @topic.messages.select(:id))
-                                                .select(:message_id))
-                           .order(created_at: :desc)
-                           .find { |msg| msg.attachments.any?(&:patch_extension?) }
+    latest_message = latest_patchset_message
 
     return head :not_found unless latest_message
 
@@ -303,6 +312,14 @@ def user_state_frame
 
   private
 
+  def latest_patchset_message
+    @latest_patchset_message ||= @topic.messages
+      .where(id: Attachment.where(message_id: @topic.messages.select(:id))
+                           .select(:message_id))
+      .order(created_at: :desc)
+      .find { |msg| msg.attachments.any?(&:patch_extension?) }
+  end
+
   def set_topic
     @topic = Topic.includes(
       :creator,

app/javascript/controllers/diff_highlight_controller.js

@@ -0,0 +1,41 @@
+import { Controller } from "@hotwired/stimulus"
+
+export default class extends Controller {
+  connect() {
+    if (this.element.dataset.diffHighlighted === "true") return
+
+    const text = this.element.textContent
+    if (!text) return
+
+    const fragment = document.createDocumentFragment()
+    const lines = text.split("\n")
+
+    lines.forEach((line) => {
+      const span = document.createElement("span")
+      span.classList.add("diff-line")
+
+      if (line.startsWith("+") && !line.startsWith("+++")) {
+        span.classList.add("diff-line-add")
+      } else if (line.startsWith("-") && !line.startsWith("---")) {
+        span.classList.add("diff-line-del")
+      } else if (line.startsWith("@@")) {
+        span.classList.add("diff-line-hunk")
+      } else if (
+        line.startsWith("diff ") ||
+        line.startsWith("index ") ||
+        line.startsWith("---") ||
+        line.startsWith("+++")
+      ) {
+        span.classList.add("diff-line-header")
+      }
+
+      span.textContent = line.length ? line : " "
+      fragment.appendChild(span)
+    })
+
+    this.element.textContent = ""
+    this.element.appendChild(fragment)
+    this.element.dataset.diffHighlighted = "true"
+    this.element.classList.add("diff-highlighted")
+  }
+}

app/models/attachment.rb

@@ -28,6 +28,34 @@ def decoded_body_utf8
     raw.encode("UTF-8", invalid: :replace, undef: :replace, replace: "\uFFFD")
   end
 
+  def diff_line_stats
+    return { added: 0, removed: 0 } unless patch?
+    return @diff_line_stats if defined?(@diff_line_stats)
+
+    text = decoded_body_utf8
+    return @diff_line_stats = { added: 0, removed: 0 } unless text.present?
+
+    added = 0
+    removed = 0
+
+    text.each_line do |line|
+      if line.start_with?("+") && !line.start_with?("+++")
+        added += 1
+      elsif line.start_with?("-") && !line.start_with?("---")
+        removed += 1
+      elsif line.start_with?("> ")
+        added += 1
+      elsif line.start_with?("< ")
+        removed += 1
+      elsif line.start_with?("! ")
+        added += 1
+        removed += 1
+      end
+    end
+
+    @diff_line_stats = { added: added, removed: removed }
+  end
+
   private
 
   def patch_content?

app/views/topics/_message.html.slim

@@ -72,8 +72,13 @@
                   span.filename = attachment.file_name
                   span.content-type = attachment.content_type if attachment.content_type
                   = link_to "Download", attachment_path(attachment), class: "attachment-download", download: attachment.file_name, data: { turbo: false }
+                  - stats = attachment.diff_line_stats
+                  - if stats[:added].positive? || stats[:removed].positive?
+                    span.patchset-stats aria-label="Patch line changes" title="Lines added and removed by this patch"
+                      span.patchset-added +#{stats[:added]}
+                      span.patchset-removed -#{stats[:removed]}
               pre.attachment-content
-                code.language-diff
+                code.language-diff data-controller="diff-highlight"
                   = attachment.decoded_body_utf8
           - else
             .attachment

app/views/topics/show.html.slim

@@ -75,10 +75,14 @@
       summary.sidebar-heading Attachments
       .sidebar-section
         - if @has_patches
-          .download-patchset style="margin-bottom: 1rem;"
+          .download-patchset
             = link_to latest_patchset_topic_path(@topic), class: "button download-button", download: "topic-#{@topic.id}-patchset.tar.gz", data: { turbo: false } do
               i.fas.fa-download
               span Download Latest Patchset
+            - if @latest_patchset_stats
+              .patchset-stats aria-label="Patchset line changes" title="Lines added and removed by this patchset"
+                span.patchset-added +#{@latest_patchset_stats[:added]}
+                span.patchset-removed -#{@latest_patchset_stats[:removed]}
 
         - if attachment_messages.any?
           ul.attachments-list