Inline all text attachments (#65)

* Slightly clean up patch detection code

Makes the code for detecting if an attachment is a patch or have a file
extension indicating that it's a patch slightly nicer.

* Inline all plain text attachments

Not only patches.

Author: AndersAstrand

app/models/attachment.rb

@@ -5,12 +5,31 @@ class Attachment < ApplicationRecord
   after_create :mark_topic_has_attachments
   after_destroy :update_topic_has_attachments
 
+  # This list is in no way comprehensive. It can be extended as needed.
+  TEXT_APPLICATION_TYPES = %w[
+    application/json
+    application/sql
+    application/x-perl
+    application/x-python
+    application/x-ruby
+    application/x-sh
+    application/x-shellscript
+    application/xml
+    application/yaml
+  ].freeze
+
+  def text?
+    return true if content_type&.start_with?("text/")
+    return true if content_type&.end_with?("+xml", "+json")
+    TEXT_APPLICATION_TYPES.include?(content_type)
+  end
+
   def patch?
-    file_name&.ends_with?(".patch") || file_name&.ends_with?(".diff") || patch_content?
+    patch_extension? || patch_content?
   end
 
   def patch_extension?
-    file_name&.ends_with?(".patch") || file_name&.ends_with?(".diff")
+    file_name&.ends_with?(".patch", ".diff")
   end
 
   def decoded_body

app/views/topics/_message.html.slim

@@ -65,20 +65,21 @@
       .message-attachments id="message-#{message.id}-attachments"
         h4 Attachments:
         - message.attachments.each do |attachment|
-          - if attachment.patch?
+          - if attachment.patch? || attachment.text?
             details.attachment
               summary.attachment-info
                 span.attachment-summary-row
                   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]}
+                  - if attachment.patch?
+                    - 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 data-controller="diff-highlight"
+                code data-controller=("diff-highlight" if attachment.patch?)
                   = attachment.decoded_body_utf8
           - else
             .attachment

spec/requests/topics_spec.rb

@@ -133,7 +133,7 @@ def attach_verified_alias(user, email:, primary: true)
 
         expect(response).to have_http_status(:success)
         expect(response.body).to include("Attachments:")
-        expect(response.body).to include('class="language-diff"')
+        expect(response.body).to include('data-controller="diff-highlight"')
         expect(response.body).to include("diff --git")
       end
     end