Should the wrap prefix %W take newlines into account?

[phil-s]

I'd been using the "IRC-style with right margin, with wrap-prefix" option for ement-room-message-format-spec but (on account of a user who had taken to putting rather lengthy status info in their 'friendly' username) wanted to stop the wrap prefix from following the username.

I decided I liked the idea of mimicing the Element formatting I was used to, where I had the username on top, the timestamp underneath on the left, and the message wrapping at the edge of the timestamp. I figured that would be this:

(setq ement-room-message-format-spec "%S:\n%t %W%B%r")

However the %W takes the entirety of the preceding string into account, and so the subsequent lines of the message are wrapped as if they had both the username and the timestamp on the left.

I don't imagine that would ever be useful when there are newlines involved, so should this be changed so that %W only considers the line it's in rather than the entire string prefix?

I tried to be sneaky by putting the wrap prefix at the start followed by a carriage return and then the 'real' value, but that just got me a ^M in the output :)

[alphapapa]

Would you show me a screenshot of what you mean, please?

[phil-s]

So for %S:\n%t %W the %W indent ends up incorporating the width of both %S and %t when I want it to only use %t. (And for my case with the user with the very long username, the indent is huge.)

[alphapapa]

Thanks. Well, as long as the code isn't too complex and doesn't perform poorly, I've no objection to implementing this.

[phil-s]

As a naive implementation, I can confirm this change has the desired effect for me as far as rendering goes.

@@ -3353,7 +3353,11 @@
       (when ement-room--format-message-wrap-prefix
         (when-let ((wrap-prefix-end (next-single-property-change (point-min) 'wrap-prefix-end)))
           (let* ((prefix-width (string-width
-                                (buffer-substring-no-properties (point-min) wrap-prefix-end)))
+                                (buffer-substring-no-properties
+                                 (save-excursion
+                                   (goto-char wrap-prefix-end)
+                                   (re-search-backward "\n\\|^"))
+                                 wrap-prefix-end)))
                  (prefix (propertize " " 'display `((space :width ,prefix-width)))))
             (goto-char wrap-prefix-end)
             (delete-char 1)

[phil-s]

I presume skip-chars-backward is going to be better:

modified   ement-room.el
@@ -3352,8 +3352,12 @@ Format defaults to `ement-room-message-format-spec', which see."
       ;; Propertize margin text.
       (when ement-room--format-message-wrap-prefix
         (when-let ((wrap-prefix-end (next-single-property-change (point-min) 'wrap-prefix-end)))
-          (let* ((prefix-width (string-width
-                                (buffer-substring-no-properties (point-min) wrap-prefix-end)))
+          (let* ((wrap-prefix-start (save-excursion
+                                      (goto-char wrap-prefix-end)
+                                      (skip-chars-backward "^\n")
+                                      (point)))
+                 (prefix-width (string-width (buffer-substring-no-properties
+                                              wrap-prefix-start wrap-prefix-end)))
                  (prefix (propertize " " 'display `((space :width ,prefix-width)))))
             (goto-char wrap-prefix-end)
             (delete-char 1)

I guess that the save-excursion can just be progn.

[phil-s]

I'm not seeing any noticeable performance differences with this vs the original code.

[phil-s]

It took me a moment to realise that it was just line-beginning-position I wanted :)

With a little rearrangement, I think we can simply use:

@@ -3351,11 +3352,11 @@ Format defaults to `ement-room-message-format-spec', which see."
       ;; Propertize margin text.
       (when ement-room--format-message-wrap-prefix
         (when-let ((wrap-prefix-end (next-single-property-change (point-min) 'wrap-prefix-end)))
-          (let* ((prefix-width (string-width
-                                (buffer-substring-no-properties (point-min) wrap-prefix-end)))
+          (goto-char wrap-prefix-end)
+          (delete-char 1)
+          (let* ((prefix-width (string-width (buffer-substring-no-properties
+                                              (line-beginning-position) (point))))
                  (prefix (propertize " " 'display `((space :width ,prefix-width)))))
-            (goto-char wrap-prefix-end)
-            (delete-char 1)
             ;; We apply the prefix to the entire event as `wrap-prefix', and to just the
             ;; body as `line-prefix'.
             (put-text-property (point-min) (point-max) 'wrap-prefix prefix)

[alphapapa]

@phil-s Thanks. Would you confirm that with that change the rendering is unchanged using the set of default ement-room-message-format-spec choices?

[phil-s]

Testing still pending, but just a little note for anyone else interested:

I tried setting the ement-room-timestamp face to use a smaller font, but because of what I'm doing in this issue that had the side-effect of "outdenting" the first line of the message slightly with respect to the wrapped lines which followed. However, serendipitously there was a nice way around that!

With the timestamp face set to height 0.8 and the timestamp format set to HH:MM the 5 monospace timestamp characters occupy exactly 4 monospace characters at the normal size, and so the following makes everything line up perfectly:

(ement-room-message-format-spec-setter
   'ement-room-message-format-spec "\n%S:\n%t%W %B%r")

I.e. putting the space after %W instead of before it, so that every subsequent line is indented 5 spaces rather than the 6 characters of the initial line, because that 6 is composed of (5 + 1) where the 5 (timestamp) is the equivalent of 4 regular characters and the 1 is a space with the regular face. So the first line is effectively indented (4 + 1) normal-sized characters and subsequent lines are equivalently indented by 5 normal-sized spaces.

YMMV.

[phil-s]

@phil-s Thanks. Would you confirm that with that change the rendering is unchanged using the set of default ement-room-message-format-spec choices?

I've just done this. I used two instances of Emacs, one with this change and one without, and did a side-by-side comparison of all of the default formats, and none of them were affected. I only observed the difference when I switched them to use my custom format with the newline.

I'll raise a PR now.