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] Outdated mu4e-mark-at-point docstring #2767

Closed
Uthar opened this issue Oct 1, 2024 · 1 comment
Closed

[mu4e bug] Outdated mu4e-mark-at-point docstring #2767

Uthar opened this issue Oct 1, 2024 · 1 comment
Labels

Comments

@Uthar
Copy link

Uthar commented Oct 1, 2024

Outdated mu4e-mark-at-point docstring

Hello!

I wanted to make trash marking act as a move to the trash folder, without setting the trashed flag.

This is a the scenario when the server handles auto-trashing, mentioned in the isync manual. It's also briefly mentioned in section 8.4 of the mu4e info manual.

The previous similar discussions are: #1018, #1136.

I implemented an advice function for this:

(defun mu4e-trash-by-moving-advice (args)
  "Makes `mu4e-mark-at-point' handle trash marks as moves to the trash folder."
  (cl-destructuring-bind (mark &optional target) args
    (if (eql mark 'trash)
      (list 'move (mu4e-get-trash-folder (mu4e-message-at-point)))
      args)))
  
(advice-add 'mu4e-mark-at-point :filter-args 'mu4e-trash-by-moving-advice)

It seems a better solution than modifying mu4e-marks (which was mentioned by previous posters: #1136), because it doesn't reference any internals or modify a defconst.

But I found the docstring of mu4e-mark-at-point confusing:

It says that for trash marks the target argument is non-nil. However, the current implementation ignores any provided target argument and uses a dynamically computed value via mu4e--mark-get-dyn-target. This, in turn, calls what seems to be an internal function under mu4e-marks -> 'trash -> :dyn-target. It then calls the user-provided mu4e-trash-folder function with the result of mu4e-message-at-point as argument. But if it did receive a meaningful target argument, I would have to consider handling it in the advice.

  • Is that docstring outdated or am I misunderstanding the code?
  • Is that advice reasonable from maintainer perspective, or is it likely to cause problems with future versions of mu4e?

How to Reproduce

It's only a documentation issue.

Environment

mu/mu4e 1.12.5

@Uthar Uthar added bug mu4e specific for mu4e new labels Oct 1, 2024
@djcb djcb added documentation and removed new labels Oct 5, 2024
@djcb
Copy link
Owner

djcb commented Oct 16, 2024

Wow, it's been years since I've seen that code... You are right, the comment was wrong, I've updated it now; thank you!

@djcb djcb closed this as completed in 7776e2b Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants