Enhancements to composition buffers

[alphapapa]

See #31 (comment) and #31 (comment):

@phil-s

I believe this will be useful for me:

;; Open Ement Compose buffers below the Room buffer.
(add-to-list 'display-buffer-alist
             (cons "^\\*Ement compose: "
                   (cons 'display-buffer-below-selected
                         '((window-height . 6)
			   (inhibit-same-window . t)
                           (reusable-frames . nil)))))

Yeah, that looks handy. Might add that to the docs.

I also notice that these buffers are in fundamental-mode. At minimum, I'd suggest:

(define-derived-mode ement-compose-mode text-mode "Ement Compose"
  "Major mode for composing messages for Matrix rooms.")

I think I have a TODO around here somewhere for that...

A key binding to abort the message + delete the window would be useful.

You can simply kill the buffer, like any other buffer. But if you really want, we could add something like C-c C-k, I guess, similar to Magit commit buffers.

And if there were an option to send the message directly without going via the minibuffer, I'd probably have pretty much everything I wanted at that point.

I'm open to that idea, sure. What keybinding would you suggest? Maybe C-c C-RET, or C-c RET?

[phil-s]

C-c C-c is a pretty standard "I've finished doing the thing" binding; and both that and C-c C-k for aborting are familiar to Magit users editing commit messages (which feels fairly analogous); so offhand I'd suggest those.

[alphapapa]

C-c C-c would be fine if it weren't for supporting Org mode in composition buffers, in which C-c C-c has an existing meaning. That's why I went with C-x C-s for the binding to return to composing in the minibuffer.

[phil-s]

Ah, I see. Your C-c RET suggestion sounds pretty good, then.

[phil-s]

My IRC muscle-memory continues to try to start composing messages by simply typing (and I have usually typed the first SPC before I've realised I'm doing it wrong, which then takes me to a different room), so I've just done the following. I know that various letters are bound to other commands, which makes the resulting behaviour inconsistent, but I think this will save me more headaches than it creates (or maybe I'll just be more likely to perform unintended room commands... we'll see how it goes...)

I'm putting the code here just in case this is also what someone else wants.

(require 'cl-lib)
(eval-when-compile (require 'ement-macros))

(with-eval-after-load "ement-room"
  (define-key ement-room-mode-map [remap self-insert-command]
    #'my-ement-room-insert-compose-message))

(defun my-ement-room-insert-compose-message ()
  "Begin composing a message."
  (interactive)
  (apply #'ement-room-compose-message (ement-with-room-and-session
				       (list ement-room ement-session)))
  (push last-command-event unread-command-events)
  ;; Set key bindings in the compose buffer.
  (local-set-key [remap save-buffer] #'my-ement-room-compose-send-direct)
  (local-set-key (kbd "C-c C-k") #'my-ement-room-compose-abort)
  (local-set-key [remap delete-backward-char]
                 `(menu-item "" my-ement-room-compose-abort
                             :filter (lambda (cmd)
                                       (and (<= (buffer-size) 1)
                                            (save-restriction (widen) (eobp))
                                            cmd)))))

(defun my-ement-room-compose-abort ()
  "Kill the compose buffer and window."
  (interactive)
  (let ((room ement-room))
    (kill-buffer)
    (delete-window)
    ;; Make sure we end up with the associated room buffer selected.
    (when-let ((win (catch 'room-win
                      (walk-windows
                       (lambda (win)
                         (with-selected-window win
                           (and (derived-mode-p 'ement-room-mode)
                                (bound-and-true-p ement-room)
                                (eq ement-room room)
                                (throw 'room-win win))))))))
      (select-window win))))

Or we can move the conflicting default bindings to a new prefix to eliminate the inconsistencies. I've bound the prefix map to DEL which seemed like a convenient key to hijack in this situation.

(defun my-ement-room-config ()
  "Called via `with-eval-after-load' for ement-room.el."
  ;; Start a new message just by typing.
  (define-key ement-room-mode-map [remap self-insert-command]
    #'my-ement-room-insert-compose-message)
  ;; Remap the default bindings which might conflict with that.
  (defvar my-ement-room-mode-prefix-map (make-sparse-keymap))
  (fset 'my-ement-room-mode-prefix-map my-ement-room-mode-prefix-map)
  (define-key ement-room-mode-map (kbd "DEL") 'my-ement-room-mode-prefix-map)
  ;; Assume that I'll only start typing a message with a letter or number.
  (dolist (char (nconc (number-sequence ?a ?z)
                       (number-sequence ?A ?Z)
                       (number-sequence ?0 ?9)))
    (when-let* ((key (char-to-string char))
                (cmd (lookup-key ement-room-mode-map key)))
      (define-key my-ement-room-mode-prefix-map key cmd)
      (define-key ement-room-mode-map key nil))))

(with-eval-after-load "ement-room"
  (my-ement-room-config))

[phil-s]

Does this adaptation of ement-room-compose-send look right for the "send message directly from the compose buffer" command?

(defun ement-room-compose-buffer-string-trimmed ()
  "Like `buffer-string' trimmed with `string-trim'."
  (buffer-substring-no-properties (progn (goto-char (point-min))
                                         (skip-chars-forward " \t\r\n")
                                         (point))
                                  (progn (goto-char (point-max))
                                         (skip-chars-backward " \t\r\n")
                                         (point))))

(defun ement-room-compose-send-direct ()
  "Directly send the current compose buffer's contents.
To be called from an `ement-room-compose' buffer."
  (interactive)
  (cl-assert ement-room-compose-buffer)
  (cl-assert ement-room)
  (cl-assert ement-session)
  ;; Prepare message.
  (let ((body (ement-room-compose-buffer-string-trimmed))
        (send-message-filter ement-room-send-message-filter)
        (replying-to-event ement-room-replying-to-event)
        (editing-event ement-room-editing-event)
        (room ement-room)
        (session ement-session))
    (add-to-history 'ement-room-message-history body)
    (quit-restore-window nil 'kill)
    (ement-view-room room session)
    ;; Send message.
    (let ((ement-room-send-message-filter send-message-filter))
      (if editing-event
          (ement-room-edit-message (ement--original-event-for editing-event session)
                                   room session body)
        (ement-room-send-message room session
                                 :body body
                                 :replying-to-event (and replying-to-event
                                                         (ement--original-event-for
                                                          replying-to-event session)))))))

I opted for trimming surrounding whitespace in the buffer in order to avoid generating additional strings.

Edit: 2023-09-25: updated to match #189

Edit: 2023-10-04: updated for changes in ement version 0.13.

[alphapapa]

Does this adaptation of ement-room-compose-send look right for the "send message directly from the compose buffer" command?

Probably, but if possible I'd like to avoid adding another command that does all that stuff. It would be nice to reuse the existing command that puts the message to be sent back into the minibuffer; we could probably find a way to cause the message to be sent immediately from the minibuffer.

[phil-s]

I don't actually understand the workflow of moving back to the minibuffer from the compose buffer.

I understand that composing entire messages in the minibuffer can be desirable; and I understand that being able to "upgrade" from the minibuffer to a compose buffer is desirable in cases where the message is more elaborate; but having ended up in a compose buffer, I don't understand why anyone would ever want to go back to the minibuffer as an intermediate step before sending a message. That seems clunky to me.

(I may be missing the reason for this on account of my not wanting to use the minibuffer for editing messages at all, though.)

[alphapapa]

I guess it could be seen as clunky, yes. My interpretation is that the minibuffer is the way to send a message, so the existing functionality just allows you to move the message back and forth between a full buffer and the minibuffer; or it could be that putting it back in the minibuffer serves as a confirmation step after composing a more "serious" message.

But I've no objection to adding a command to send it directly from the full buffer. I'd just like to, if possible, make the code as simple as possible, probably by just moving the message back to the minibuffer and then calling the command to send the message from the minibuffer--the message's moving back into the minibuffer should happen without being visible to the user, because redraw shouldn't happen until after it's sent.

[phil-s]

My intention is to create a PR at some point to add an option to use the compose-buffer-centric behaviour I'm working on. I think some of the other users will like it, so I feel it would be a nice option to include.

It more or less gives the feel of the "shell-like prompt" approach, but as it's actually based on the pre-existing compose buffer support I don't think it will require too much in the way of new code (although at this point I've only dealt with writing new messages). The most disruptive thing I'm doing so far is messing with ement-room-mode-map, but I expect I can come up with something which will be clean enough for the purposes of toggling the option on and off.

[progfolio]

related: #200 would allow removing the default compose-send behavior and replacing it to the user's content.

[phil-s]

I've updated #140 (comment) for ement v0.13.

(And as a general note, locally I'm running a bunch of WIP changes beyond what I have in this thread, and so it's possible that I'll break something in an update on account of it not actually being what I'm using. If you're using my code and any such breakage transpires, please let me know.)

[phil-s]

There is now a PR for this work: #236

Feel free to test and provide feedback.

[phil-s]

#236 has been merged (yay!), so I think we can close this.

(edit: ack, typo; not 246. although that is also merged :).