Skip to content
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.

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

fixes #200 #201

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fixes #200 #201

wants to merge 1 commit into from

Conversation

jsilve24
Copy link

@jsilve24 jsilve24 commented Dec 4, 2024

remove undo from message-sent-hook as it was causing an error and for html drafts to be sent after successful message sending (unintended behavior).

Also discussed in #182 where the hook was suspected to be causing problems #182

Note, in #182 multiple users report no issues after removing hook.

@jeremy-compostella
Copy link
Owner

Could you only remove it for mu4e? It works just fine for gnus.

fixes jeremy-compostella#200

remove undo from message-sent-hook as it was causing an error and for html drafts to be sent after successful message sending (unintended behavior).

Also discussed in jeremy-compostella#182 where the hook was suspected to be causing problems jeremy-compostella#182

Note, in jeremy-compostella#182 multiple users report no issues after removing hook.

As requested, only turn remove hook from mu4e
@jsilve24
Copy link
Author

jsilve24 commented Dec 6, 2024

updated with requested change, reverted my master branch to keep git history clean.

@jsilve24 jsilve24 reopened this Dec 6, 2024
@zauster
Copy link

zauster commented Dec 9, 2024

Hey @jsilve24 ,

I am using your repo with my doom config now. It works as it does not complain about "undo outside of visible portion" or so. But drafts are still saved (in html format I think) and not deleted after a successful send. Do you see the same behaviour?

@jsilve24
Copy link
Author

jsilve24 commented Dec 9, 2024 via email

@zauster
Copy link

zauster commented Dec 13, 2024

Could this commit in mu be the cause of this? Seems it was part of mu 1.12.7 that came out in end of October.

djcb/mu@413c03e

@jsilve24
Copy link
Author

jsilve24 commented Dec 13, 2024

Good find. It could be. I just tried out the following function and it does seem to have avoided the "wrote draft" message. Will use this in my config for a day or two to see if any issues pop up.

Not sure what the best fix would be. Probably a bug report to mu4e... Whats interesting is that the undesired behavior is only happening with org-msg. Without org-msg loaded its not a problem. Not sure why though...

@zauster
Copy link

zauster commented Dec 13, 2024

What do you mean with "the following function"? The mu4e--compose-before-send function without the change in the commit?

@jsilve24
Copy link
Author

Oops, my bad.

This:

  (defun mu4e--compose-before-send ()
    "Function called just before sending a message."
    ;; Remove References: if In-Reply-To: is missing.
    ;; This allows the user to effectively start a new message-thread by
    ;; removing the In-Reply-To header.
    (when (eq mu4e-compose-type 'reply)
      (unless (message-field-value "In-Reply-To")
	(message-remove-header "References")))
    (when use-hard-newlines
      (mu4e--send-harden-newlines))
    ;; in any case, make sure to save the message; this will also trigger
    ;; before/after save hooks, which fixes up various fields.
    (set-buffer-modified-p t)
    ;; (save-buffer) ;; removed due to Forked due to this issue https://github.com/jeremy-compostella/org-msg/issues/200
    ;; now handle what happens _after_ sending
    (add-hook 'message-sent-hook #'mu4e--compose-message-sent nil t))

Same function you highlighted. I just commented out hte save-buffer command. I have it added to my config after mu4e loads and it seems to be fixing the problem. No side-effects noted yet. Obviously this is not a great solution but more of a test.

@zauster
Copy link

zauster commented Dec 15, 2024

I have another find: The function mu4e--compose-before-send (the function that now saves the buffer) in mu is executed in the message-send-hook. org-msg adds the function org-msg-prepare-to-send to the same hook. The org-msg-prepare-to-send creates an mml-version of the drafted mail.
So my guess is that the change in mu now saves the mml-version of the mail, but that file is not deleted when successfully sent. That's why the issue only happens when org-msg is enabled.
Option 1: make sure that mu4e--compose-before-send is executed before org-msg-prepare-to-send when the hook is executed, so that the mml-version is surely created after saving the buffer. Could that help?
Option 2: add a function to message-sent-hook (mind that it is the message-sent-hook) that additionally deletes the mml file.

@spencerjackson
Copy link

I've taken a look at this, and have some observations.

First, the output in the Messages buffer is pretty different depending on whether mu4e--compose-before-send is allowed to save a draft or not.

With (save-buffer):

Mark set
Saving file /home/sajack/.mail/sajack/Drafts/cur/1736719104.49db6e550b771a6e.localhost:2,DS...
Wrote /home/sajack/.mail/sajack/Drafts/cur/1736719104.49db6e550b771a6e.localhost:2,DS
Sending...
Sending via mail...
Sending email  
Sending email done
message-send: Text is read-only
Quit
Mark set

Without (save-buffer) in mu4e--compose-before-send :

Sending...
Sending via mail...
Sending email  
Sending email done
Saving file /home/sajack/.mail/sajack/sent/cur/1736719342.144a65ad12a967bd.localhost:2,S...
Wrote /home/sajack/.mail/sajack/sent/cur/1736719342.144a65ad12a967bd.localhost:2,S
Undo
Sending...done
error in process filter: mu4e-error: [mu4e] Error 117: failed to add path: /home/sajack/.mail/sajack/Drafts/cur/1736719342.33a07f855dbb8d89.localhost:2,DS: file @ '/home/sajack/.mail/sajack/Drafts/cur/1736719342.33a07f855dbb8d89.localhost:2,DS' is not readable
error in process filter: [mu4e] Error 117: failed to add path: /home/sajack/.mail/sajack/Drafts/cur/1736719342.33a07f855dbb8d89.localhost:2,DS: file @ '/home/sajack/.mail/sajack/Drafts/cur/1736719342.33a07f855dbb8d89.localhost:2,DS' is not readable

We see three differences.

  1. We made mu4e sad, because the file it tried to save wasn't there.
  2. With (save-buffer) we got an extra error: "message-send: Text is read-only"
  3. Without (save-buffer) we... got further into message-send? We see "Sending...done", which we didn't see with (save-buffer).

Altogether, this suggests that saving the buffer causes something to break during message-send, which somehow interrupts parts of the sending process.

Debugging with edebug suggests that the warning is getting emitted during erase-buffer in message-do-fcc during message-send.

Setting (setq inhibit-read-only t) before sending prevents the HTMLized draft from being shown, reinforcing the idea that this has something to do with read-only text.

Poking at the draft buffer, (erase-buffer) doesn't work. Also, the string "--text follows this line--" can't be edited.

Starting a fresh org-msg compose buffer, saving it, and then running (erase-buffer) also results in an error. After saving "--text follows this line--" is read only. Opening a regular mu4e compose window and saving does not make "--text follows this line--" read only. This suggests that mu4e's before and after save hooks are causing some kind of problem.

Looking at mu4e's save hooks, mu4e--delimit-headers seems like a worthy candidate for further investigation: it removes "--text follows this line--" prior to saving and restores it after. The text which is restored after saving comes from "mu4e--header-separator", which is defined (on my system) as:

(defconst mu4e--header-separator ;; XX properties down work... why not?
  (propertize "--text follows this line--" 'read-only t 'intangible t)
  "Line used to separate headers from text in messages being composed.")

Which is... interesting. So, not only does mu4e set it as read-only, but they believe that for some reason the property isn't taking affect. But org-msg somehow... works around mu4e's text property bug, causing an issue deeper down in message.el?

Anyway, this suggests an easy test: First, we create an org-msg compose buffer, run mu4e--delimit-headers, then test if "--text follows this line--" is read only. And, yes, it is! Creating a regular mu4e compose buffer, and running mu4e--delimit-headers... does not create read only text.

So, something about org-msg fixes a bug in mu4e, exposing read-only buffer text to untested logic down in message-send. Perhaps it's something about the major modes? In org-mode, it's possible to run (insert mu4e--header-separator) and get read-only text but in mu4e's native compose buffer it's not. Maybe message-mode has some logic to prevent read only text? Checking the mode's definition:

  ;; Mmmm... Forbidden properties...
  (add-hook 'after-change-functions #'message-strip-forbidden-properties
	    nil 'local)

There is a hook that strips forbidden properties!

Adding the hook to org-msg-edit-mode, plus the patch in this PR makes mu4e send the message, and then hide the buffer without showing me the HTMLized unsaved buffer. I'll push that change up as a PR on top of this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants