Skip to content

Inline all text attachments #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ImTheKai merged 2 commits into from
Feb 26, 2026
Merged

Inline all text attachments #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a187044
Slightly clean up patch detection code
AndersAstrand Feb 13, 2026
fcaf1fc
Inline all plain text attachments
AndersAstrand Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions app/models/attachment.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 8 additions & 7 deletions app/views/topics/_message.html.slim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Copy link
Contributor

@moozzi moozzi Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about extracting this section to for example attachments/_preview.html.slim?
We could render attachments/_patch_preview.html.slim or attachments/_text_preview.html.slim (or attachments/_image_preview.html.slim if we decide to implement it) there, based on the attachment's type (method thats checks if its patch, text etc.).

I'm aware that it will create some code repetition, but in my opinion views will be much easier to understand.

Copy link
Collaborator Author

@AndersAstrand AndersAstrand Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good idea. I need to toy a little with what it would look like in practice.

Copy link
Collaborator

@dutow dutow Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndersAstrand do you still want to look into this?

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
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/topics_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down