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

Compatibilty with mu4e 1.12 #190

Merged
merged 1 commit into from
Sep 2, 2024

Conversation

danielfleischer
Copy link
Contributor

  • Collection of various patches to make org-msg compatible with mu 1.12+.
  • It will break compatibility with older versions as there are major changes in the mu library.

@milanglacier
Copy link

Hi I did some test on this branch, there is a small problem I noticed:

  1. there is an error message when trying to send the mail:
primitive-undo: Changes to be undone are outside visible portion of buffer

However, the mail can be sent succesfully though. I have no idea what does this message mean.

@danielfleischer
Copy link
Contributor Author

danielfleischer commented Apr 14, 2024

Try to comment the line:

(add-hook 'message-sent-hook 'undo t t)

I thought that isn't needed but some people came up with that fix. Also, are you following the latest code, not just the official 1.12 release?

@martin5233
Copy link

I'm also experiencing a similar problem: After sending a mail I see the buffer with mail converted to HTML and the buffer marked as modified. The minibuffer contains a message 'Text is read-only'. The mail is successfully sent.
I have tried commenting out the line
(add-hook 'message-sent-hook 'undo t t)
but this didn't help. It wasn't clear to me, what your suggestion to follow the latest code meant. Should I try to use mu4e master in conjunction with your PR?
I have tried to debug into this, but was soon lost in unfamiliar code.

@danielfleischer
Copy link
Contributor Author

Yes, you should use mu4e from master as there were many updates to the composing code after the official release. Perhaps I should have been more explicit, as the the 1.12 code changes fast.

@milanglacier
Copy link

Yes, you should use mu4e from master as there were many updates to the composing code after the official release. Perhaps I should have been more explicit, as the the 1.12 code changes fast.

if this is the case, perhaps we can’t merge this PR now? If this PR relies on the master branch code and has unresolved problems on official release.

Since I believe most of people will just follow the official release (probably install from their package manager) instead of building from the source?

@danielfleischer
Copy link
Contributor Author

Well, there was a 1.12.4 release a couple of days ago, containing the fixes I'm talking about. So no need to compile from master, just make sure you are updating, and not use the original 1.12.0 from February.

@milanglacier
Copy link

Just updated my mu to 1.12.4 and it seems that it just works. No strange errors, and the mail was sent successfully.

@abougouffa
Copy link
Contributor

@jeremy-compostella
This is working fine as of today (mu 0.12.5), can you merge it please!

@djs42012
Copy link

This works but I am getting Symbol’s function definition is void: mu4e~proc-sent after sending messages. The message sends successfully but the window does not close and I am left with a temp buffer.

@danielfleischer
Copy link
Contributor Author

This works but I am getting Symbol’s function definition is void: mu4e~proc-sent after sending messages. The message sends successfully but the window does not close and I am left with a temp buffer.

You are not running the PR correctly.

@djs42012
Copy link

You’re right.I got mixed up and used:

(package! org-msg
  :recipe (:host github :repo "jeremy-compostella/org-msg" :branch "jcompost/mu-1.12-support-with-backward-compatibility")
  :pin "cc25647")

Instead of your PR:

(package! org-msg
  :recipe (:host github :repo "danielfleischer/org-msg" :branch "1.12")
  :pin "4dcd70f")

@danielfleischer
Copy link
Contributor Author

Still works with v1.12.6.

@podiki
Copy link
Contributor

podiki commented Aug 19, 2024

@jeremy-compostella will this be merged? I have been using it locally for a while now as otherwise org-msg doesn't work

Edit: And would be great to have a new release (version) for this as well, for downstream packaging.

@milanglacier
Copy link

An off topic mumble as an old mu4e user here:

I have switched to notmuch for about 1-2 months and I keep using org-msg to compose HTML emails using notmuch frontend. It just works as it should be. In fact, searching the issue list in this repo, the occurrences of notmuch is far less than mu4e. The most frustrating thing for mu4e, i.e the breaking change here and there just made me tired. I used some homemade shell script (in notmuch pre and post hook) to do the mail movement stuffs and I don't miss anything from mu4e.

@danielfleischer
Copy link
Contributor Author

@milanglacier please don't off topic here or in any pull-request discussion, it's not reddit or social media and the linear format makes any off topic an on topic discussion. This is strictly a technical discussion about suggested code changes to a library. As such you're not contributing anything.

@danielfleischer
Copy link
Contributor Author

Ready to ship; the overall change after the recent master update is removing some hooks, originally from mu4e-compose which aren't needed anymore.

@jeremy-compostella
Copy link
Owner

Do you mind making a clean (or multiple) commit(s) with a nice headline and a descriptive commit. I would like to avoid nasty merge commits. Especially when one dl something that the other undo 🤔
Also, it seems that your last patch has nothing to do with my, could you move it out ?

@danielfleischer
Copy link
Contributor Author

@jeremy-compostella done; after your recent "backward compatibility" commit, all that is left is cleaning the hooks a bit.

org-msg.el Outdated
(setq mu4e-sent-func 'mu4e-sent-handler)
(mu4e~proc-sent (buffer-file-name))))
nil t))
(add-hook 'message-sent-hook #'mu4e--compose-before-send nil t))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds weird to tie before-send to the sent-hook. Could you confirm this is correct ?

Also, do you think you could use a construction like the following (untested btw) ? I would to keep it compatible if mu4e change the prefix of this function.

(when-let ((sent-hook (org-msg--mu4e-fun "compose-before-send")
    (add-hook 'message-sent-hook sent-hook)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the when-let block and tested it.

Regarding the mu4e--compose-before-send hook, it actually deals with emails before and after sending so that's fine.

Composing and drafting were largely delegated to Gnus facilities. Code
copied from `mu4e-compose.el` is not needed.
@benthamite
Copy link

@jeremy-compostella I am getting the error reported by @milanglacier above. Is there a reason for keeping (add-hook 'message-sent-hook 'undo t t) in the definition of org-msg-edit-mode? This error prevents the killing of the message buffer when message-kill-buffer-on-exit is set to t.

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.

8 participants