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

[mu4e bug] mu4e-compose-pre-hook is broken in v1.12+ #2715

Closed
3 tasks done
milouse opened this issue Jun 17, 2024 · 4 comments · May be fixed by #2716
Closed
3 tasks done

[mu4e bug] mu4e-compose-pre-hook is broken in v1.12+ #2715

milouse opened this issue Jun 17, 2024 · 4 comments · May be fixed by #2716
Labels
bug mu4e specific for mu4e

Comments

@milouse
Copy link
Contributor

milouse commented Jun 17, 2024

Describe the bug

Due to an old known bug in date time formatting, my setup relies on preparatory work done for every draft. I expose it in this old issue: #2113 (comment)

To make it short, I temporarily store the mu4e-compose-parent-message in a variable, to be able to get it again to generate the citation line (historically the parent-message variable was lost while generating the citation line. Maybe this is not the case anymore, but this is another subject).

Please note the documentation for the mu4e-compose-pre-hook still say the following:

Hook run just before message composition starts.

If the compose-type is a symbol, either ‘reply’ or ‘forward’, the
variable ‘mu4e-compose-parent-message’ is the message replied to
/ being forwarded / edited, and ‘mu4e-compose-type’ contains the
type of message to be composed.

However, in version 1.12.0 through 1.12.5 the mu4e-compose-pre-hook call has disapeared and any functions added to the hook is never called. From 3ff2f9f the hook is called again… but too soon in the chain: mu4e-compose-parent-message and mu4e-compose-type are only set after the hook is run in the function mu4e--draft

How to Reproduce

  • Add a function to mu4e-compose-pre-hook
  • Start a draft

The function is never called, or after 3ff2f9f, they are called but mu4e-compose-parent-message and mu4e-compose-type are nil.

Possible solution

If I may, I’d have moved the following block:

(cl-assert (member compose-type '(reply forward edit new)))
  (cl-assert (eq (if parent t nil)
                 (if (member compose-type '(reply forward)) t nil)))
  ;; remember some variables, e.g for user hooks. These are permanent-local
  ;; hence survive the mode-switch below (we do this so these useful vars are
  ;; available in mode-hooks.
  (setq-local
   mu4e-compose-parent-message parent
   mu4e-compose-type compose-type)

from the function mu4e--prepare-draft-buffer to the function mu4e--prepare-draft, before calling the pre-hook.

However this implies to call mu4e--prepare-draft with the compose-type as parameter, what is not possible today, and I’m not sure about the implication of such a move.

Environment

Emacs 29.3 with mu 1.12.5

Checklist

  • you are running either an 1.10.x/1.12.x release or master (otherwise please upgrade)
  • you can reproduce the problem without 3rd party extensions (including Doom/Evil, various extensions etc.)
  • you have read all of the above
@djcb
Copy link
Owner

djcb commented Jun 17, 2024

That's indeed broken, I fixed it a some point.. but apparently it broke again :-|

Also thank you for the PR... I'm pushing a change that is slightly different, running the hook a bit earlier so that (as the docstring suggests) one could influence the later draft-creation steps.

@milouse
Copy link
Contributor Author

milouse commented Jun 18, 2024

Good to know. If you need I’m willing to test it :)

@djcb djcb removed the new label Jun 19, 2024
djcb added a commit that referenced this issue Jun 19, 2024
Run the hook earlier, and ensure mu4e-compose-type &
mu4e-compose-parent-message are set.

As noted in #2715.
@milouse
Copy link
Contributor Author

milouse commented Jun 19, 2024

I just checked your implementation and it’s far more simple and better than mine.

It leverages just a little thing: maybe you should also run the cl-assert from the mu4e--prepare-draft-buffer before even running the hooks? The idea being to fail sooner if the message-type or parent variables are not valid?

I’m speaking about these lines:

  (cl-assert (member compose-type '(reply forward edit new)))
  (cl-assert (eq (if parent t nil)
                 (if (member compose-type '(reply forward)) t nil)))

@djcb
Copy link
Owner

djcb commented Jun 27, 2024

Maybe we could copy them to the being of mu4e--draft, but tbh, these assert are more-or-less leftovers from development, so don't care too much about them.

Anyway, thanks for the heads up! Closing this ticket.

@djcb djcb closed this as completed Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mu4e specific for mu4e
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants