Improving support for variable-pitch text

[phil-s]

I'd like to be able to configure a variable-pitch font for all regular text in a chat room, while seeing a fixed-width font for any pre-formatted text.

At present I can see a user option ement-room-shr-use-fonts which results in formatted (org.matrix.custom.html) messages being variable-pitch, but (as the docstring points out), "some messages won’t display in the same font as others", and seeing fixed-width messages interspersed with the formatted ones ends up being fairly ugly and distracting.

It would be nice if there was an option for un-formatted text to be rendered with the same variable-pitch face.

I can customize the ement-room-message-text face, but experimentally this causes pre-formatted code blocks (sent via the org-mode support) to render variable-pitch (although in-line verbatim and code text does end up fixed-width).

Without that face change I think 'shr-use-fonts' was doing all the right things for formatted messages, so a way to target only "unformatted" messages could work, but there might be some cleaner approach to unifying the behaviour.

[alphapapa]

It seems like a nice idea, yes. Maybe it could be done by adjusting the inheritance hierarchy of the Ement faces.

[phil-s]

I found myself looking at this again today, and believe I have a decent solution. I'll make a PR sometime soon.

modified   ement-room.el
@@ -1197,7 +1197,7 @@ ement-room--event-body-face
   ;; This used to be a macro in --format-message, which is probably better for
   ;; performance, but using a function is clearer, and avoids premature optimization.
   (pcase-let* (((cl-struct ement-event sender
-                           (content (map msgtype))
+                           (content (map msgtype format))
                            (unsigned (map ('redacted_by unsigned-redacted-by)))
                            (local (map ('redacted-by local-redacted-by))))
                 event)
@@ -1224,7 +1224,16 @@ ement-room--event-body-face
                                               (color-darken-name message-color ement-room-prism-message-lightening))))))))
                (redacted-face (when (or local-redacted-by unsigned-redacted-by)
                                 'ement-room-redacted))
-               (body-face (list :inherit (delq nil (list redacted-face context-face type-face)))))
+               ;; When `ement-room-shr-use-fonts', also use the variable-pitch `shr-text'
+               ;; face for non-HTML messages, for visual consistency.  Note that messages
+               ;; with format "org.matrix.custom.html" are already formatted by shr, so we
+               ;; do not apply this to those messages.
+               (shr-text-face (and ement-room-shr-use-fonts
+                                   (equal msgtype "m.text")
+                                   (not (equal format "org.matrix.custom.html"))
+                                   (or (and (facep 'shr-text) 'shr-text)
+                                       'variable-pitch)))
+               (body-face (list :inherit (delq nil (list redacted-face context-face type-face shr-text-face)))))
     (if prism-color
         (plist-put body-face :foreground prism-color)
       body-face)))

Note that you should not make ement-room-message-text variable-pitch, as that will still override what shr does with HTML text. If you were using that approach before, revert it.

Instead, this change applies to plain text messages the same face as shr applies to the plain text parts of HTML messages.

[phil-s]

Not quite right. When I post a HTML message I see this face text property on the body:

  (shr-text (:inherit (ement-room-self-message ement-room-message-text)))

If I then edit that message, I see this:

  (shr-text (:inherit (ement-room-self-message ement-room-message-text shr-text)))

And the effect of that was that the font size increased.

This is because shr-text inherits from variable-pitch-text which sets Height: 1.1 so I'm getting that multiple twice.

We could inherit from variable-pitch instead of shr-text, but then we'd see a font size which was smaller than intended in the plain text case.

[phil-s]

So that's a bug in my code, as the edit event meets my criteria for adding the shr-text face, because the (format . "org.matrix.custom.html") is within a m.new_content section rather than directly in content.

 (:content
  (body . "bar'")
  (m.new_content
   (body . "bar'")
   (format . "org.matrix.custom.html")
   (formatted_body . "<p>\nbar'</p>")
   (msgtype . "m.text"))
  (m.relates_to
   (event_id . "$uKSPbwa7uNru3BocehZZWyQ-8PonzVh1VXGOQmk6W-M")
   (rel_type . "m.replace"))
  (msgtype . "m.text"))

I'll fix this later.

[phil-s]

...or right away :)

Are there other special cases to consider besides this one?

modified   ement-room.el
@@ -1197,10 +1197,11 @@ ement-room--event-body-face
   ;; This used to be a macro in --format-message, which is probably better for
   ;; performance, but using a function is clearer, and avoids premature optimization.
   (pcase-let* (((cl-struct ement-event sender
-                           (content (map msgtype))
+                           (content (map msgtype format ('m.new_content new-content)))
                            (unsigned (map ('redacted_by unsigned-redacted-by)))
                            (local (map ('redacted-by local-redacted-by))))
                 event)
+               (format (or format (cdr (assq 'format new-content))))
                ((cl-struct ement-user (id sender-id)) sender)
                ((cl-struct ement-session user) session)
                ((cl-struct ement-user (id user-id)) user)
@@ -1224,7 +1225,16 @@ ement-room--event-body-face
                                               (color-darken-name message-color ement-room-prism-message-lightening))))))))
                (redacted-face (when (or local-redacted-by unsigned-redacted-by)
                                 'ement-room-redacted))
-               (body-face (list :inherit (delq nil (list redacted-face context-face type-face)))))
+               ;; When `ement-room-shr-use-fonts', also use the variable-pitch `shr-text'
+               ;; face for non-HTML messages, for visual consistency.  Note that messages
+               ;; with format "org.matrix.custom.html" are already formatted by shr, so we
+               ;; do not apply this to those messages.
+               (shr-text-face (and ement-room-shr-use-fonts
+                                   (equal msgtype "m.text")
+                                   (not (equal format "org.matrix.custom.html"))
+                                   (or (and (facep 'shr-text) 'shr-text)
+                                       'variable-pitch)))
+               (body-face (list :inherit (delq nil (list redacted-face context-face type-face shr-text-face)))))
     (if prism-color
         (plist-put body-face :foreground prism-color)
       body-face)))

[alphapapa]

Are there other special cases to consider besides this one?

I don't know.

A couple of minor requests:

• Please use alist-get instead of (cdr (assq ....
• Please use (when (and ...) value) instead of (and ... value). It helps separate the conditions from the value.

[phil-s]

No problem. I've relocated the alist-get so that it doesn't happen unless needed. I'll run with this for a bit and see whether I spot any other cases.

modified   ement-room.el
@@ -1197,7 +1197,7 @@ ement-room--event-body-face
   ;; This used to be a macro in --format-message, which is probably better for
   ;; performance, but using a function is clearer, and avoids premature optimization.
   (pcase-let* (((cl-struct ement-event sender
-                           (content (map msgtype))
+                           (content (map msgtype format ('m.new_content new-content)))
                            (unsigned (map ('redacted_by unsigned-redacted-by)))
                            (local (map ('redacted-by local-redacted-by))))
                 event)
@@ -1224,7 +1224,17 @@ ement-room--event-body-face
                                               (color-darken-name message-color ement-room-prism-message-lightening))))))))
                (redacted-face (when (or local-redacted-by unsigned-redacted-by)
                                 'ement-room-redacted))
-               (body-face (list :inherit (delq nil (list redacted-face context-face type-face)))))
+               ;; For visual consistency, apply the variable-pitch `shr-text' face to
+               ;; non-HTML messages when `ement-room-shr-use-fonts' is non-nil (HTML
+               ;; messages are fontified by shr itself).
+               (shr-text-face (when (and ement-room-shr-use-fonts
+                                         (equal msgtype "m.text")
+                                         (not (equal (or format (alist-get 'format new-content))
+                                                     "org.matrix.custom.html")))
+                                ;; The `shr-text' face was added in Emacs 29.1.
+                                (or (and (facep 'shr-text) 'shr-text)
+                                    'variable-pitch)))
+               (body-face (list :inherit (delq nil (list redacted-face context-face type-face shr-text-face)))))
     (if prism-color
         (plist-put body-face :foreground prism-color)
       body-face)))