jeremy-compostella / org-msg

OrgMsg is a GNU/Emacs global minor mode mixing up Org mode and Message mode to compose and reply to emails in a Outlook HTML friendly style.
GNU General Public License v3.0
275 stars 58 forks source link

Sent messages are not saved when using org-msg #58

Closed ngleb closed 3 years ago

ngleb commented 4 years ago

I have the following code that adds GCC header, and it works without org-msg. All sent messages are saved to "Sent Items" folder.

(setq gnus-message-archive-group "nnimap+name:Sent Items")

But when I use org-msg, I see GCC header with the correct folder specified, but sent emails are not saved anywhere.

I tried setting mail-user-agent to gnus-user-agent, but the result is the same - sent messages are not saved anywhere.

Is this is a known issue?

tminor commented 4 years ago

The same seems to be true for notmuch (with some differences in implementation, of course). I haven't looked closely yet but plan to do so when I have some free time.

tminor commented 4 years ago

I did some (very) brief digging a moment ago and found that (at least in the case of notmuch) the magic happens when notmuch-mua-send-and-exit is invoked (which calls notmuch-maildir-fcc-file-fcc). That function is normally bound to C-c C-c in notmuch-message-mode. In org-msg-mode the same key combination invokes org-ctrl-c-ctrl-c (which I guess is no surprise). I'm not sure if this is at all similar to your issue with Gnus :/

I'll report back once I've hacked together a workaround.

tminor commented 4 years ago

@ngleb (and @happymcplaksin since you brought this issue to my attention): I finally got around to taking a closer look and found a hack that seems to work (for notmuch, at least):

diff --git a/org-msg.el b/org-msg.el
index 2224084..f8f21ee 100644
--- a/org-msg.el
+++ b/org-msg.el
@@ -917,7 +917,8 @@ area."
 If the current buffer is OrgMsg buffer and OrgMsg is enabled (see
 `org-msg-toggle'), it calls `message-send-and-exit'."
   (when (eq major-mode 'org-msg-edit-mode)
-    (message-send-and-exit)))
+    (letf (((symbol-function 'message-do-fcc) #'notmuch-maildir-message-do-fcc))
+      (message-send-and-exit))))

 (defun org-msg-tab ()
   "Complete names or Org mode visibility cycle.

The stuff I added was taken from the defun for notmuch-mua-send-common but I didn't find a similarly named Gnus function after a quick browse through counsel-describe-function.

EDIT: There are a few functions with gcc in their name. Maybe gnus-inews-add-send-actions is a good place to start? That function does this:

(add-hook 'message-sent-hook (if gnus-agent
                                 'gnus-agent-possibly-do-gcc
                               'gnus-inews-do-gcc) nil t)
happymcplaksin commented 4 years ago

FYI GCC is working with Gnus. When I first tested a while back I swear it didn't work! But maybe I was wrong. Anyhow, today I went to find what was preventing GCC from working with org-msg and discovered that it's working!

morganwillcock commented 4 years ago

I think whether it works may be dependent on which mail backend you are using. I'm using nnimap and the gcc setting seems to be ignored. I was testing a couple of weeks ago (I've only just found this package - it looks great!) so it would have been with recent versions of everything.

happymcplaksin commented 4 years ago

Hmm, I'm using nnimap with Gnus too. In case it helps, today I wiped my previous org-mode try and started over using org-msg-20200722.2238 from MELPA

morganwillcock commented 4 years ago

I've re-tested using the master branch and it still didn't work for me. So I tried various combinations of sending and I've found that under one condition it works:

if I initiate sending mail from the group buffer in Gnus (compose-mail [C-x m]) the Gcc field is honoured and the mail is archived to the sent mail folder using IMAP - it doesn't seem to matter what is under point, I tried with my nnimap inbox group and an nnrss group and it worked in both cases.

If I enter a group and reply to message it doesn't seem to work. I do use posting styles to switch between two sending addresses, but the Gcc field always remains the same. Disabling the use of posting styles doesn't fix it either, so that doesn't seem to be the direct cause of the problem.

happymcplaksin commented 4 years ago

@morganwillcock AHA! Thanks for finding that. It's the same for me: replies do not get put in the GCC folder. GCC does work when forwarding a message.

happymcplaksin commented 4 years ago

They problem may be that the local value message-sent-hook in the unsent mail buffer ends up nil when replying. When composing a new message, the value is (gnus-agent-possibly-do-gcc t)

I'm out of time to debug this further at the moment but will try again later. Unless somebody beats me to it :)

jeremy-compostella commented 4 years ago

Sorry to join so late the investigation but I was so busy at work that out took over my personal time too.

I looked at this issue an hour last week but I had to stop before I can come up with a patch. The problem is actually very simple. Some gnus function mentioned in this discussion set up the hook variable. This message-sent-hook variable is a buffer local variable. Buffer local variable are destroyed when a major mode is applied (when org-msg-edit-mode is called). This is where I ran out time. Next step is to study options to prevent this to happen or to save and restore certain variables in a clean fashion.

I may get to it later this afternoon.

On Wed, Jul 29, 2020, 5:29 AM Mark Plaksin notifications@github.com wrote:

They problem may be that the local value message-sent-hook in the unsent mail buffer ends up nil when replying. When composing a new message, the value is (gnus-agent-possibly-do-gcc t)

I'm out of time to debug this further at the moment but will try again later. Unless somebody beats me to it :)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jeremy-compostella/org-msg/issues/58#issuecomment-665634938, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMACBFKBKZ256NHOEUVTVTR6AI3TANCNFSM4N3HYMFA .

jeremy-compostella commented 4 years ago

I finally got a few minutes to look at this issue again. For the gnus backend the explanation is included in the experimental commit da41528ffdc00b8790750839b8369fad8ff5b0fa (see below). Note that the behavior between a new message (working) and replying (not working) is due to the path taken and when is the message-sent-hook variable changed. Is there are some gnus backend user on this thread I would appreciate if you could give a change to this patch. I recommend that you either disable Org-Msg mode with M-x org-msg-mode before loading the new module or use a fresh Emacs. This is important as some global/local variable are impacted.

This patch resolves issue #58 for the gnus backend.  When a reply
buffer is created by gnus the `gnus-agent-possibly-do-gcc' function is
added to the `message-sent-hook' hook list variable. The
`gnus-agent-possibly-do-gcc' function role is to copy the sent message into
the sent folder it has been specified in a Gcc' field.

Unfortunately, the gnus module makes the `message-sent-hook' a local
variable and local variables are killed when switching to a new major
mode which is what is done `org-msg-post-setup' when
`org-msg-edit-mode' is called.

By marking the `message-sent-hook' variable `permanent-local'
switching to a new major mode preserves this variable and
`gnus-agent-possibly-do-gcc' stays in the `message-sent-hook' hook
list variable.

This patch also makes the `message-send-hook' a local variable for
consistency and its value is now modified from `org-msg-edit-mode'
too.

Signed-off-by: Jeremy Compostella <jeremy.compostella@gmail.com>

@TimQuelch, regarding the notmuch backend I looked at your patches and they are interesting. However, I am under the impression that there could be a simpler and less intrusive solution: calling notmuch-mua-send-and-exit instead of message-send-and-exit. Could you give a try to M-x message-send-and-exit instead of C-c C-C ? If this works then we could just create a mua backend callback for send-and-exit in Org-Msg.

TimQuelch commented 4 years ago

I'll have a play around with it in a bit. I did not dive too deeply into the source code before submitting that fix, so it is definitely likely that there is a more optimal method to implement it

happymcplaksin commented 4 years ago

@jeremy-compostella The experimental commit works great for me with Gnus; thank you!

tminor commented 4 years ago

@jeremy-compostella M-x notmuch-mua-send-and-exit worked for me but M-x message-send-and-exit didn't.

morganwillcock commented 4 years ago

The experimental branch fixes the issue for me, and the Gcc field was honoured in every case where Gnus was already running (I think this is expected behaviour?).

There does seem to be a new side-effect introduced, that once the message is sent and the buffer is closed I am now switched to the default *GNU Emacs* buffer instead instead of returning to where I initiated the reply. I have double checked that the master branch doesn't do this.

TimQuelch commented 4 years ago

@tminor Annoyingly I am getting (somewhat) the opposite results.

@jeremy-compostella

<#part type=“text/html” disposition=inline> <html xmlns=“http://
www.w3.org/1999/xhtml” lang=“en” xml:lang=“en”><head><!– 2020-08-13
Thu 09:47 –><meta http-equiv=“Content-Type” content=“text/html;
charset=utf-8”/><meta name=“viewport” content=“width=device-width,
initial-scale=1”/><meta name=“generator” content=“Org mode”/></head>
<body> <div style=“font-family:&quot;Arial&quot;;font-size:10pt;
line-height:11pt;” id=“content”> <p style=“text-decoration:none;
margin-bottom:0px;margin-top:10px;line-height:11pt;font-size:10pt;
font-family:&quot;Arial&quot;;max-width:100ch;”> Thanks, </p>

<div style=“font-family:&quot;Arial&quot;;font-size:10pt;
margin-bottom:20px;font-family:&quot;Arial&quot;;font-size:10pt;
line-height:11pt;”> <p style=“text-decoration:none;margin-bottom:0px;
margin-top:10px;line-height:11pt;font-size:10pt;font-family:&quot;
Arial&quot;;max-width:100ch;”> Tim Quelch </p>

</div> </div> </body></html><#/part>

Author: Tim Quelch

Created: 2020-08-13 Thu 09:47
TimQuelch commented 4 years ago

I will have a play around in https://github.com/jeremy-compostella/org-msg/pull/64 to see if I can get it working with a proper callback

jeremy-compostella commented 4 years ago

Your issue looks like what you would see if you switch to the new experimental branch without unloading the org-msg module first or start with a fresh emacs.

jeremy-compostella commented 4 years ago

I have just pushed 75f8440067a995f0f70ef58c488b668acfd5ae44 on the experimental branch which should address the sent folder issue for notmuch. Could you give it a try ?

TimQuelch commented 4 years ago

https://github.com/jeremy-compostella/org-msg/commit/1a56754cd0ab2c2a13adca3695ab4b7cf7a30923 is still not working for me :(

I'm getting the same results as before with the odd raw html output

TimQuelch commented 4 years ago

Tim Quelch

Table of Contents


<#part type="text/html" disposition=inline> <html xmlns="http://www.w3.org/1999/xhtml" lang="en" xml:lang="en"><meta http-equiv="Content-Type" content="text/html;charset=utf-8"/><meta name="viewport" content="width=device-width, initial-scale=1"/><meta name="generator" content="Org mode"/> <div style="font-family:"Arial";font-size:10pt;line-height:11pt;" id="content"> <p style="text-decoration:none;margin-bottom:0px;margin-top:10px;line-height:11pt;font-size:10pt;font-family:"Arial";"> This is an email reply with the current release

<p style="text-decoration:none;margin-bottom:0px;margin-top:10px;line-height:11pt;font-size:10pt;font-family:"Arial";"> Thanks,

<div style="font-family:"Arial";font-size:10pt;margin-bottom:20px;font-family:"Arial";font-size:10pt;line-height:11pt;"> <p style="text-decoration:none;margin-bottom:0px;margin-top:10px;line-height:11pt;font-size:10pt;font-family:"Arial";"> Tim Quelch

<#/part>

TimQuelch commented 4 years ago

The above reply is what an email looks like in the current build :/

I have a strong suspicion it has something to do with the way the hooks are set and called by both org-msg and notmuch

tminor commented 4 years ago

I've had the same experience as @TimQuelch with 1a56754cd0ab2c2a13adca3695ab4b7cf7a30923 :(

jeremy-compostella commented 4 years ago

To my understanding the only you could get such a situation is if the org-msg-prepare-to-send is called twice. The first time is turn your org buffer into a mml/html, the second turns the html into html. As you know I don't use the notmuch so I am limited in the experiment I can perform and have to rely on you to instrument and report debug information.

First, could you identify the commit causing this issue ? You could run git bisect between 2db6725 and 1a56754. Make sure you run a fresh emacs for each try. If I had to guess I would think that da41528 is the one causing this issue.

Second, we need to identify where are these two calls coming from. You can add some instrumentation in org-msg-prepare-to-send like (message "org-msg-prepare-to-send called"), then go the *Messages* buffer and verify that you indeed see org-msg-prepare-to-send called twice. If so then you need to capture the stacks. Instead of message use error and capture the stack from the *Backtrace* buffer. Then you need to capture the second stack. You can use a variable that you change on the first org-msg-prepare-to-send to make the second call detect that this is the second call and call the error function. Then *Backtrace* you will have the stack of the second call.

I am looking forward to receive this debug information.

TimQuelch commented 4 years ago

Good news, this is not a regression! It simply has never worked!

I tested all the way back to 3c19b67 (when notmuch support was first added). Emails sent with notmuch-mua-send-and-exit all had the incorrect html output.

Do those other debugging steps and get back with the results

TimQuelch commented 4 years ago

It looks like notmuch-send-common calls the message-send-hooks (through some indirection by running notmuch-mua-send-hooks, which by default has notmuch-mua-message-send-hook set as the only function, which runs message-send-hooks)

message-send-hooks is then called again in message-send (where you would expect it to be called...)

This seems to be a bug on notmuch's end. I've sent an email in their mailing list to see if it was a specific design decision or a bug that should be fixed.

Backtrace 1

Debugger entered--Lisp error: (error "Calling org-msg-prepare-to-send")
  signal(error ("Calling org-msg-prepare-to-send"))
  error("Calling org-msg-prepare-to-send")
  org-msg-prepare-to-send()
  run-hooks(message-send-hook)
  notmuch-mua-message-send-hook()
  run-hooks(notmuch-mua-send-hook)
  notmuch-mua-send-common(nil t)
  notmuch-mua-send-and-exit(nil)
  funcall-interactively(notmuch-mua-send-and-exit nil)
  call-interactively(notmuch-mua-send-and-exit record nil)
  command-execute(notmuch-mua-send-and-exit record)
  helm-M-x-execute-command(notmuch-mua-send-and-exit)

Backtrace 2

Debugger entered--Lisp error: (error "Calling org-msg-prepare-to-send")
  signal(error ("Calling org-msg-prepare-to-send"))
  error("Calling org-msg-prepare-to-send")
  (progn (error "Calling org-msg-prepare-to-send"))
  (if called-prepare (progn (error "Calling org-msg-prepare-to-send")))
  org-msg-prepare-to-send()
  run-hooks(message-send-hook)
  message-send(nil)
  message-send-and-exit(nil)
  notmuch-mua-send-common(nil t)
  notmuch-mua-send-and-exit(nil)
  funcall-interactively(notmuch-mua-send-and-exit nil)
  call-interactively(notmuch-mua-send-and-exit record nil)
  command-execute(notmuch-mua-send-and-exit record)
  helm-M-x-execute-command(notmuch-mua-send-and-exit)
TimQuelch commented 4 years ago

A short term user fix (and my suggested fix to the notmuch devs) is to remove notmuch-mua-message-send-hook from notmuch-mua-send-hook, as all this does is run message-send-hook.

(remove-hook 'notmuch-mua-send-hook 'notmuch-mua-message-send-hook)

jeremy-compostella commented 4 years ago

Nice catch. However, I believe that there have been at least several org-msg notmuch user for a while now and I find it a bit surprising that nobody complained about it earlier. But this is not the first time I open the notmuch emacs source code and find the implementation a bit off the Emacs standard.

It looked at the notmuch code and I have been able to match your stacks. I think that we have two options:

  1. the one you suggested
  2. create a function like
    (defun org-msg-edit-mode-notmuch ()
    (remove-hook 'message-send-hook 'org-msg-prepare-to-send t))

The benefit of #2 is that he does not impact the notmuch execution flow.

TimQuelch commented 4 years ago

The reason it didn't pop up sooner is because previously org-msg was just calling message-send-and-exit, rather than notmuch-mua-send-and-exit which calls the hooks the extra time. This manifested in the much easier to miss issue of not handling saving sent messages correctly.

I believe with #2 like that, org-msg-prepare-to-send would not be called at all

I think the correct way to handle #2 would be to

This would ensure it is still being called only once. That solution would also persist if this is fixed on notmuch's end

jeremy-compostella commented 4 years ago

Hi @TimQuelch,

I have spent some time thinking about the best way to fix while following good software practices for this module (stay agnostic of the MUA implementation, stick to Emacs idioms, improve the solution instead of creating workarounds ...) and I ended up with the following solution that I pushed on the experimental branch (I re-wrote the history).

Make `org-msg-prepare-to-send' more robust

Tim Quelch reported in #58 that the notmuch MUA directly and
indirectly runs the `message-send-hook'. This makes the message
converted to HTML twice, causing raw HTML to be included in the sent
message.

The best way to fix this is certainly in motmuch. Tim reported this
issue to the notmuch developers through their mailing list (see
"emacs: `message-send-hook` being called twice").

There are many ways to work-around this issue as removing
`org-msg-prepare-to-send' from the `message-send-hook' ...  But it
appears to me that instead of creating a workaround we could make the
`org-msg-prepare-to-send' more robust.  The goal of this function is
to convert the OrgMode content of the org-msg-edit-mode buffer to
MML/HTML content.  Instead of assuming that the content is OrgMode,
this function will now use a text property to know if the content is
OrgMode or MML.

When the `org-msg-prepare-to-send' is called more than once, it prints
a warning message. It gives a chance to the user to detect and report
a potential issue with a backend implementation.

Reported-by: Tim Quelch <tim@tquelch.com>
Signed-off-by: Jeremy Compostella <jeremy.compostella@gmail.com>

Regards, Jeremy

jeremy-compostella commented 4 years ago

@TimQuelch does this work for you ?

TimQuelch commented 4 years ago

@jeremy-compostella I just did a very quick test and it seems like it is working! Thanks very much

TimQuelch commented 3 years ago

This seems to be a bug on notmuch's end. I've sent an email in their mailing list to see if it was a specific design decision or a bug that should be fixed.

This should be fixed in notmuch upstream now as well

jeremy-compostella commented 3 years ago

There does seem to be a new side-effect introduced, that once the message is sent and the buffer is closed I am now switched to the default *GNU Emacs* buffer instead instead of returning to where I initiated the reply. I have double checked that the master branch doesn't do this.

I have pushed a patch on the experimental branch to take care of this issue. Let me know if it works for you.

morganwillcock commented 3 years ago

That does fix it. Thanks!

jeremy-compostella commented 3 years ago

Fantastic ! Now I can finally close this ticket :)