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

everything below --citation follows this line (read-only)-- erased #73

Open
jonathanwilner opened this issue Sep 30, 2020 · 74 comments
Open

Comments

@jonathanwilner
Copy link

Every time I try to reply to a message using org-msg, any text that should be cited is instead removed from the message. This is true in the preview & the final results.

System is Notmuch on Emacs 27.1 or Emacs 28 on Mac OS X Catalina. I'm using Doom Emacs. I've tried the latest version from master as well as the experimental branch. Both produce the same results.

I'm running the latest notmuch from git.

Any ideas?

@TimQuelch
Copy link
Contributor

I believe this is the intended behaviour. If you want to quote something in your reply you should specifically include it above the --citation...-- line in a #+begin_quote ... #+end_quote block

@jonathanwilner
Copy link
Author

jonathanwilner commented Oct 1, 2020

So, it’s cited below the original email in order to allow the user to copy the text into the right block ? That makes sense.

Is there an example configuration that uses org-msg only for composition, not for replies,
In order to still use html mail, but make it less cumbersome to keep threads full of info for others in the email conversation?

@jeremy-compostella
Copy link
Owner

Is there an example configuration that uses org-msg only for composition, not for replies, In order to still use html mail, but make it less cumbersome to keep threads full of info for others in the email conversation?

Could you be more specific ? I am having a hard understanding your request

@whudwl
Copy link

whudwl commented May 4, 2021

@jeremy-compostella Sorry to intrude. But I was wondering the exact same question when trying to use org-msg with notmuch.

With mu4e, when replying to an email, the original email appears below my reply(below a grey horizontal line), making my reply totally Outlook style.

But with notmuch, my reply would only contain my reply, without the original email below it. (Hence as this issue said, things below --citation follows...--- are erased).

I could not figure out why there is such difference. Would you mind sharing your thoughts? Please let me know in case my description above isn't clear.

@whudwl
Copy link

whudwl commented May 4, 2021

@jeremy-compostella Sorry to intrude. But I was wondering the exact same question when trying to use org-msg with notmuch.

With mu4e, when replying to an email, the original email appears below my reply(below a grey horizontal line), making my reply totally Outlook style.

But with notmuch, my reply would only contain my reply, without the original email below it. (Hence as this issue said, things below --citation follows...--- are erased).

I could not figure out why there is such difference. Would you mind sharing your thoughts? Please let me know in case my description above isn't clear.

I have realized that the problem is the notmuch implementation did not privode a org-msg-save-article-for-reply-notmuch function.

@jonathanwilner
Copy link
Author

@whudwl - thank you for intruding! That's precisely the issue. I really appreciate your help documenting what happens and why !

@jeremy-compostella
Copy link
Owner

@whudwl and @jonathanwilner, feel to volunteer and write this function. I do not use notmuch and I cannot write it.

@whudwl
Copy link

whudwl commented May 5, 2021

just quickly whipped up this piece which seems to work for me. I'm no expert on lisp so this could be totally far from an optimal solution.

(defvar org-msg-notmuch-saved-article-path "")

(defun org-msg-notmuch-save-article (handle)
  "recursively search through the result of (mm-dissect-buffer) and save the text/html part.
path of the saved file is saved in valiable org-msg-notmuch-saved-article-path
"
  (cond
   ((stringp (car handle)) (mapcar #'org-msg-notmuch-save-article (cdr handle)))
   ((bufferp (car handle))
    (if (string= (car (car (cdr handle))) "text/html")
        (progn
          (let ((file (make-temp-file "org-msg" nil ".html")))
            (mm-save-part-to-file handle file)
            (setq org-msg-notmuch-saved-article-path file)))))
   (t
    (mapcar #'org-msg-notmuch-save-article handle))))

(defun org-msg-save-article-for-reply-notmuch ()
  (let ((message-id  (save-excursion
                       (goto-char (point-min))
                       (re-search-forward "In-Reply-To: <\\(.*\\)>")
                       (match-string-no-properties 1))))
    (save-window-excursion
      (let ((notmuch-show-only-matching-messages t))
        (notmuch-show (format "id:%s" message-id)))
      (with-current-notmuch-show-message
       (org-msg-notmuch-save-article (mm-dissect-buffer)))
      (list org-msg-notmuch-saved-article-path))))

@jeremy-compostella
Copy link
Owner

jeremy-compostella commented May 6, 2021

This is interesting. While I was re-working you code I realized a few things:

  1. it does not save the images or re-bind them in the <img> tags
  2. it does create the header

Then I figured we could re-use the gnus engine.

Does the following work for you ? Note that I reworked some stuff too, so you may have to debug it a little bit.

(defun org-msg-save-article-for-reply-notmuch ()
  (let ((id (trim (org-msg-message-fetch-field "in-reply-to")))
	header parts)
    (cl-flet ((get-field (field)
	       (when-let ((value (org-msg-message-fetch-field field)))
		 (concat (capitalize field) ": " value))))
      (save-window-excursion
	(let ((notmuch-show-only-matching-messages t))
          (notmuch-show (format "id:%s" (substring id 1 -1))))
	(with-current-notmuch-show-message
	 (let ((fields (mapcar #'get-field
			       '("from" "subject" "to" "cc" "date"))))
	   (setf header (mapconcat 'identity (delq nil fields) "\n")))
	 (setf parts (mm-dissect-buffer)))
      (let* ((browse-url-browser-function #'ignore)
	     (save (cl-copy-list gnus-article-browse-html-temp-list)))
	(cl-letf (((symbol-function 'gnus-summary-show-article) #'ignore))
	  (save-window-excursion
	    (gnus-article-browse-html-parts parts header)))
	(let ((temp-files (cl-set-difference gnus-article-browse-html-temp-list save
					     :test 'string=)))
	  (setq gnus-article-browse-html-temp-list save)
	  temp-files))))))

It creates a code duplication in OrgMsg that if it that code work will have to be taken care of.

@whudwl
Copy link

whudwl commented May 6, 2021

This is interesting. While I was re-working you code I realized a few things:

  1. it does not save the images or re-bind them in the <img> tags
  2. it does create the header

Then I figured we could re-use the gnus engine.

Does the following work for you ? Note that I reworked some stuff too, so you may have to debug it a little bit.

(defun org-msg-save-article-for-reply-notmuch ()
  (let ((id (trim (org-msg-message-fetch-field "in-reply-to")))
	header parts)
    (cl-flet ((get-field (field)
	       (when-let ((value (org-msg-message-fetch-field field)))
		 (concat (capitalize field) ": " value))))
      (save-window-excursion
	(let ((notmuch-show-only-matching-messages t))
          (notmuch-show (format "id:%s" (substring id 1 -1))))
	(with-current-notmuch-show-message
	 (let ((fields (mapcar #'get-field
			       '("from" "subject" "to" "cc" "date"))))
	   (setf header (mapconcat 'identity (delq nil fields) "\n")))
	 (setf parts (mm-dissect-buffer)))
      (let* ((browse-url-browser-function #'ignore)
	     (save (cl-copy-list gnus-article-browse-html-temp-list)))
	(cl-letf (((symbol-function 'gnus-summary-show-article) #'ignore))
	  (save-window-excursion
	    (gnus-article-browse-html-parts parts header)))
	(let ((temp-files (cl-set-difference gnus-article-browse-html-temp-list save
					     :test 'string=)))
	  (setq gnus-article-browse-html-temp-list save)
	  temp-files))))))

It creates a code duplication in OrgMsg that if it that code work will have to be taken care of.

where does the (trim ...) function come from? My emacs couldn't find it.

@jeremy-compostella
Copy link
Owner

jeremy-compostella commented May 6, 2021 via email

@whudwl
Copy link

whudwl commented May 6, 2021

I don't know. I added it for safety but you can probably remove the call to this function.

On Wed, May 5, 2021, 5:57 PM David @.**> wrote: This is interesting. While I was re-working you code I realized a few things: 1. it does not save the images or re-bind them in the tags 2. it does create the header Then I figured we could re-use the gnus engine. Does the following work for you ? Note that I reworked some stuff too, so you may have to debug it a little bit. (defun org-msg-save-article-for-reply-notmuch () (let ((id (trim (org-msg-message-fetch-field "in-reply-to"))) header parts) (cl-flet ((get-field (field) (when-let ((value (org-msg-message-fetch-field field))) (concat (capitalize field) ": " value)))) (save-window-excursion (let ((notmuch-show-only-matching-messages t)) (notmuch-show (format "id:%s" (substring id 1 -1)))) (with-current-notmuch-show-message (let ((fields (mapcar #'get-field '("from" "subject" "to" "cc" "date")))) (setf header (mapconcat 'identity (delq nil fields) "\n"))) (setf parts (mm-dissect-buffer))) (let ((browse-url-browser-function #'ignore) (save (cl-copy-list gnus-article-browse-html-temp-list))) (cl-letf (((symbol-function 'gnus-summary-show-article) #'ignore)) (save-window-excursion (gnus-article-browse-html-parts parts header))) (let ((temp-files (cl-set-difference gnus-article-browse-html-temp-list save :test 'string=))) (setq gnus-article-browse-html-temp-list save) temp-files)))))) It creates a code duplication in OrgMsg that if it that code work will have to be taken care of. where does the (trim ...) function come from? My emacs couldn't find it. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#73 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMACBHUAJRAFIR5F3AO7XTTMHSP7ANCNFSM4R7WSCZA .

I'm getting gnus-article-browse-html-parts: No buffer named *Article* error for most emails. any ideas what could be the issue?

@jeremy-compostella
Copy link
Owner

jeremy-compostella commented May 6, 2021 via email

@whudwl
Copy link

whudwl commented May 6, 2021

What do you mean by most emails? Does it work with some email? What is different in this emails!

Did a bit of more testing. Seems the error happens when there are inline images.

  1. Sent myself an empty email with an inline image. When replying to it, got the "No buffer named Article" error.
  2. Sent myself an empty email with an image attachment. Replying to it worked fine.

@whudwl
Copy link

whudwl commented May 6, 2021

What do you mean by most emails? Does it work with some email? What is different in this emails!

seems the error is emitted from line 2845 of gnus-art.el.

				(with-current-buffer gnus-article-buffer
				  gnus-article-mime-handles)

the value of the gnus-article-buffer variable is *Article* . Since I don't use gnus, I guess that's the default value.

@whudwl
Copy link

whudwl commented May 6, 2021

It seems gnus relies on the local variable gnus-article-mime-handles in the gnus-article-buffer to save cid contents. so how about this:

(defun org-msg-save-article-for-reply-notmuch ()
  (let ((id (string-trim (org-msg-message-fetch-field "in-reply-to")))
        header parts)
    (cl-flet ((get-field (field)
                         (when-let ((value (org-msg-message-fetch-field field)))
                           (concat (capitalize field) ": " value))))
      (save-window-excursion
        (let ((notmuch-show-only-matching-messages t))
          (notmuch-show (format "id:%s" (substring id 1 -1))))
        (with-current-notmuch-show-message
         (let ((fields (mapcar #'get-field
                               '("from" "subject" "to" "cc" "date"))))
           (setf header (mapconcat 'identity (delq nil fields) "\n")))
         (setf parts (mm-dissect-buffer)))
        (let* ((browse-url-browser-function #'ignore)
               (save (cl-copy-list gnus-article-browse-html-temp-list)))
          (cl-letf (((symbol-function 'gnus-summary-show-article) #'ignore))
            (save-window-excursion
              (setq-local gnus-article-mime-handles parts)
              (let ((gnus-article-buffer (current-buffer)))
                (gnus-article-browse-html-parts parts header))))
          (let ((temp-files (cl-set-difference gnus-article-browse-html-temp-list save
                                               :test 'string=)))
            (setq gnus-article-browse-html-temp-list save)
            temp-files))))))

@jeremy-compostella
Copy link
Owner

Even simpler:

(defun org-msg-save-article-for-reply-notmuch ()
  (let ((id (trim-string (org-msg-message-fetch-field "in-reply-to")))
	header parts)
    (cl-flet ((get-field (field)
	       (when-let ((value (org-msg-message-fetch-field field)))
		 (concat (capitalize field) ": " value))))
      (save-window-excursion
	(let ((notmuch-show-only-matching-messages t))
          (notmuch-show (format "id:%s" (substring id 1 -1))))
	(with-current-notmuch-show-message
	 (let ((fields (mapcar #'get-field
			       '("from" "subject" "to" "cc" "date"))))
	   (setf header (mapconcat 'identity (delq nil fields) "\n")))
	 (setf parts (mm-dissect-buffer))))
      (with-temp-buffer
	(let ((gnus-article-buffer (current-buffer))
	      (gnus-article-mime-handles parts))
	  (let* ((browse-url-browser-function #'ignore)
		 (save (cl-copy-list gnus-article-browse-html-temp-list)))
	    (cl-letf (((symbol-function 'gnus-summary-show-article) #'ignore))
	      (save-window-excursion
		(gnus-article-browse-html-parts parts header)))
	    (let ((temp-files (cl-set-difference gnus-article-browse-html-temp-list save
						 :test 'string=)))
	      (setq gnus-article-browse-html-temp-list save)
	      temp-files)))))))

jeremy-compostella added a commit that referenced this issue May 6, 2021
This patch addresses #73.

Signed-off-by: Jeremy Compostella <jeremy.compostella@gmail.com>
jeremy-compostella added a commit that referenced this issue May 6, 2021
This patch addresses #73.

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

I pushed the improved changes to the experimental branch. On this branch I also implemented org-msg-article-htmlp-notmuch.
I don't use notmuch so I can't really test. Could you test and debug the experimental branch ? I tried to unitary test what I can but notmuch specific function are out of reach.

@whudwl
Copy link

whudwl commented May 6, 2021

@jeremy-compostella Thank you so much for that. I did a bit of testing of the experimental branch, made some minor fixes. Now it pretty much works perfectly for me. Have created pull request.

@jonathanwilner
Copy link
Author

I've also tested the experimental branch and it's working for me as well using notmuch! Thanks @whudwl + @jeremy-compostella !

@whudwl
Copy link

whudwl commented May 6, 2021

I've also tested the experimental branch and it's working for me as well using notmuch! Thanks @whudwl + @jeremy-compostella !

Emm. Strange. It worked for you as it is? I had to modify it a bit for it to work.

@jonathanwilner
Copy link
Author

Yes - each one of these has worked for me out of the box - except for "trim" - which I couldn't find either.

The current experimental branch has a weird "select all" effect where all of the previous message is selected for yank that is going on when the reply is first composed, but it's not holding me back too much.

@whudwl
Copy link

whudwl commented May 6, 2021 via email

jeremy-compostella added a commit that referenced this issue May 6, 2021
This patch addresses #73.

Signed-off-by: Dave Dai <whudwl@gmail.com>
Signed-off-by: Jeremy Compostella <jeremy.compostella@gmail.com>
@jeremy-compostella
Copy link
Owner

jeremy-compostella commented May 6, 2021

@whudwl I integrated all your changes. Let me know if it works better.
BTW, I removed the call to string-trim because the message-fetch-field function already does it.

jeremy-compostella added a commit that referenced this issue May 7, 2021
This patch addresses #73.

Signed-off-by: Dave Dai <whudwl@gmail.com>
Signed-off-by: Jeremy Compostella <jeremy.compostella@gmail.com>
@HyunggyuJang
Copy link

HyunggyuJang commented May 12, 2021

@jeremy-compostella
Sorry about littering this space. I examined the source code closely, and it is indeed intended behavior? The whole org-msg-post-setup command wrapped by when so that when it comes to reply-to-text type, do not trigger org-msg-edit-mode. If that so, what I conceived as malfunction is from the fact that, so far, I accustomed to always on org-msg-edit-mode since its type always has been reply-to-html.

It makes sense to not activate org-msg-edit-mode for non html type messages, since it doesn’t do anything for those plain text exports.

Saying this, I think your new commit in experimental branch works seamlessly to me, also.

@whudwl
Copy link

whudwl commented May 12, 2021

org-msg is supposed to determine if it's replying to a html message or text message. So what @HyunggyuJang ponted out is probably indeed a problem.
I have tried replying to some text emails, I also got various issues. But different issues though. Will do more testing later and report back.

@whudwl
Copy link

whudwl commented May 12, 2021

Now I'm getting Invalid function: with-current-notmuch-show-message error...

@jonathanwilner
Copy link
Author

Hi all,

This current version in the experimental branch appears to erase the forwarded email again & leave just the HTML signature. Previous versions appeared to work beautifully to forward HTML email.

This could be just me & my config. Please ignore if it's working well for @whudwl or others.

Thanks!

@jeremy-compostella
Copy link
Owner

@HyunggyuJang, indeed if you do not have specified the list of alternative for reply to text, OrgMsg does not enable itself (as described in the documentation). Disabling itself for text message has always been the default behavior of OrgMsg since the very beginning and except for the notmuch backend which was very poorly written.

Patch b26aa6c put the notmuch backend to the same level. I suggest that you define org-msg-default-alternatives appropriately for your use-case. See my configuration example in the README.org.

@jeremy-compostella
Copy link
Owner

jeremy-compostella commented May 12, 2021

@jonathanwilner and @whudwl I would appreciate if a notmuch user could try to debug the experimental branch.

@jeremy-compostella
Copy link
Owner

@whudwl any update on your testing with regard to respond to various email type (text, html, mixed) ?

@HyunggyuJang
Copy link

HyunggyuJang commented May 12, 2021

@jeremy-compostella Seems like my configuration for Org Msg is somewhat obsolete, haven’t tracked the changes breaking backward compatibility so far.
Specifically mine follows the version before the commit, d670f91, and I glimpsed that the clause before treating ’reply-to-text specially, there was general one, but haven’t reach to the point that I have to update, or inspect change logs. Think setting it properly as you did in README.org would perfectly fit to my use-case.

@jeremy-compostella
Copy link
Owner

jeremy-compostella commented May 12, 2021

@HyunggyuJang

Seems like my configuration for Org Msg is somewhat obsolete, haven’t tracked the changes breaking backward compatibility so far.

There should not be any backward compatibility issue except as I said with the notmuch backend as it was poorly implemented originally and was not behaving as it should have.

Think setting it properly as you did in README.org would perfectly fit to my use-case.

Does it work ? Do you still faces a crash on reply to text only emails ?

@HyunggyuJang
Copy link

HyunggyuJang commented May 13, 2021 via email

@HyunggyuJang
Copy link

HyunggyuJang commented May 13, 2021

@jeremy-compostella

Can confirm now it works also for text only messages.

However, the signature part comes after the text citation, is it intended? To me it betrays what the top-posting style describes. On top of that, when it comes to reply to html mail, it only crawls the target mail, to which replying command issued, not the full thread, as a read only citation. Since I have no expertise about mail things, don’t know whether it is along the line or not.

Furthermore, what I’ve delineated at this comment, was from html only message, not the text only format, still exists.

I’ll send the problematic mail to your email address. Note that if [ text/html ] part nested within [ multipart/mixed ] then, it doesn’t arouse any issue.

Thanks for your hard work.

@whudwl
Copy link

whudwl commented May 13, 2021

@jeremy-compostella

Got an (wrong-type-argument stringp nil) error when trying to reply to an email. Not sure if it's related to this thread though.
It's an auto generated, notification type of email, very simple.
Trace here:
trace.txt

And for another email I replied to, the subject in the quoted section got doubled up a dozen times. For example, the subject of the mail I replied to is "urgent issue", though the subject of my reply is OK, but below my reply, in the citation section, the subject became "urgent issue urgent issue urgent issue.....". Just FYI, could be off topic.

For most emails, it works pretty well though. I thought all the heavy lifting is done by gnus engine which is supposed to be mature, so a bit surprised.

@whudwl
Copy link

whudwl commented May 13, 2021

@jeremy-compostella It seems, for this super simple email I was trying to reply to, it had only 1 part (not sure this is how to describe it?). mm-dissect-buffer returns:
( *mm*-581355 (text/html (charset . UTF-8)) 7bit nil nil nil nil nil)

which needs to be enclosed in a list. so inside org-msg-save-article-for-reply-gnus, it should probably be like this?

(gnus-article-browse-html-parts (list parts) header)

@jeremy-compostella
Copy link
Owner

@whudwl, good catch! Indeed, the gnus-article-browse-html-article takes care of this so we should also take care of this. I have pushed a patch on the experiment branch for you and @HyunggyuJang to test.

I removed 7c235d5 from the experimental branch but would appreciate if someone could look into it. It would be better to get rid of the use of a macro.

@jonathanwilner
Copy link
Author

Hi @jeremy-compostella - this continues to generally work well for me, except for forwarding HTML email. I've got to turn off org-msg before a forward now, or I just get the signature, but not the forwarded message in the multi-part message.

@jeremy-compostella
Copy link
Owner

@HyunggyuJang

However, the signature part comes after the text citation, is it intended? To me it betrays what the top-posting style describes.

It does not betray anything. The posting-style only applied to HTML emails.

(defcustom org-msg-posting-style 'top-posting
  "Define the posting style for HTML replies.
Can be either `top-posting' or nil."
  :type '(symbol))

Or from the README.org

OrgMsg composes reply to HTML emails in top-posting style. This behavior can be disabled by setting org-msg-posting-style to any value other than top-posting.

On top of that, when it comes to reply to html mail, it only crawls the target mail, to which replying command issued, not the full thread, as a read only citation. Since I have no expertise about mail things, don’t know whether it is along the line or not.

Could you provide more details? (I feel like you are expecting to much from what OrgMsg can do).

@jeremy-compostella
Copy link
Owner

@jonathanwilner

Hi @jeremy-compostella - this continues to generally work well for me, except for forwarding HTML email. I've got to turn off org-msg before a forward now, or I just get the signature, but not the forwarded message in the multi-part message.

Since which commit does forwarding does not work for you anymore ?

@jeremy-compostella
Copy link
Owner

@whudwl, could you validate 18a4620 which basically implement your suggestion? I would like to push it to master ASAP as the issue it fixes an issue which should be highly visible to notmuch users.

@jeremy-compostella
Copy link
Owner

@whudwl I found a way to validate and pushed to master. I also found a bug when forwarding an html email which is not a multipart. I pushed the fix to master too.

@jeremy-compostella
Copy link
Owner

@HyunggyuJang I tested 0418fa1 which remove the use of `with-current-notmuch-show-message' and it works like a charm on a clean Emacs.

@jonathanwilner
Copy link
Author

Hi @jeremy-compostella - it seems (at least for me) that I'm getting the "Select All" effect again from the most recent master.

Re: forwarding HTML - I can forward cleanly a message with no plaintext. If it has HTML and Plaintext, the forward now includes the From, To, Subject and Date, but then "---End of Forwarded Message---"

@HyunggyuJang
Copy link

@jeremy-compostella
Thanks for your detailed advice! I’ll look into relevant source code, or protocol related to top posting style. I’d better study these concepts before leaving comments.

Glad to know that you found the way to test notmuch related stuff. At a glimpse, your new commits works smoothly in my setting.

Thanks for your enthusiasm and endeavor!

@jeremy-compostella
Copy link
Owner

@HyunggyuJang, including the experimental branch ?

@HyunggyuJang
Copy link

HyunggyuJang commented May 17, 2021 via email

@joshuaoco
Copy link

Aha, I will be the annoying person who was relying on a bug :)

I use notmuch and I've only just updated and noticed that my HTML replies now include the citation.

This is actually not what I want, and I preferred not including the citation in my reply.

Is there any configuration to stop this?
Thanks

@jeremy-compostella
Copy link
Owner

I believe so, have a look at the documentation (README.org). If you set org-msg-posting-style to nil, then the citation is not going to be included in your HTML replies.

@joshuaoco
Copy link

joshuaoco commented Aug 31, 2021 via email

@jeremy-compostella
Copy link
Owner

This second part of the issue has nothing to do with org-msg. This is the responsibility of the Mail User Agent to pass (or not) the citation the the mail composer. For instance, with gnus, I call call either gnus-summary-reply or gnus-summary-reply-with-original to decide if I want the original message in my reply buffer. I don't which MUA you are using but you should look into the options it offers.

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

No branches or pull requests

6 participants