Emacs: notmuch-mua-mail throws error if notmuch-fcc-dirs does not exist

Emacs: notmuch-mua-mail throws error if notmuch-fcc-dirs does not exist

[Felix Konstantin Maurer]

Hi,

I had a problem where creating a reply from a message would not include
the quoted original body in the emacs notmuch client. I now found out
what the problem was.

What would happen:
From a search in notmuch, I would open a thread.
In the read I would hit "r" or "R" to reply.
A new buffer would open, containing all headers and the "--text .." line.
The headers included an FCC header to a non existing sent folder.
I would be asked whether I want to create that folder but would decline.
Then I would be in the message buffer but without a quoted body.

What seemed to be the problem:
In notmuch-mua-mail the message-mail function is called, which asked me
about the folder. When I declined to create the folder, it returned an
error "Message not sent".
The rest of notmuch-mua-mail was not executed anymore, and also the rest
of notmuch-mua-reply.

I fixed it by setting notmuch-fcc-dirs to nil.
This is good for me, but might bite somebody else at some point.

I'm quite new to emacs and don't know much elisp. I hope my findings are
correct.
Maybe somebody with more experience can confirm this problem and propose
a good fix.

Regards
Felix

Re: Emacs: notmuch-mua-mail throws error if notmuch-fcc-dirs does not exist

[David Bremner]

Felix Konstantin Maurer <maufl@maufl.de> writes:

> I would be asked whether I want to create that folder but would decline.
> Then I would be in the message buffer but without a quoted body.
>

I agree that not aborting cleanly is a bug, since it's obviously
confusing for the user.  I wonder what the right thing to do here is?
Does it make sense to abort in the case of missing Fcc directory, or
just remove the Fcc header and print a warning?

d

Re: Emacs: notmuch-mua-mail throws error if notmuch-fcc-dirs does not exist

[Mark Walters]

This may have been obvious to others but wasn't to me until I started
digging into this: 

we add the function  notmuch-fcc-header-setup
to message-header-setup-hook

and it is this function that is calling the error and forcing the
exit. In other words the problem is entirely on the notmuch side, rather
than something caused by message-mode.

Best wishes

Mark



On Fri, 10 Jul 2015, Felix Konstantin Maurer <maufl@maufl.de> wrote:
> Hi,
>
> I had a problem where creating a reply from a message would not include
> the quoted original body in the emacs notmuch client. I now found out
> what the problem was.
>
> What would happen:
> From a search in notmuch, I would open a thread.
> In the read I would hit "r" or "R" to reply.
> A new buffer would open, containing all headers and the "--text .." line.
> The headers included an FCC header to a non existing sent folder.
> I would be asked whether I want to create that folder but would decline.
> Then I would be in the message buffer but without a quoted body.
>
> What seemed to be the problem:
> In notmuch-mua-mail the message-mail function is called, which asked me
> about the folder. When I declined to create the folder, it returned an
> error "Message not sent".
> The rest of notmuch-mua-mail was not executed anymore, and also the rest
> of notmuch-mua-reply.
>
> I fixed it by setting notmuch-fcc-dirs to nil.
> This is good for me, but might bite somebody else at some point.
>
> I'm quite new to emacs and don't know much elisp. I hope my findings are
> correct.
> Maybe somebody with more experience can confirm this problem and propose
> a good fix.
>
> Regards
> Felix
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Stop stomping on global fcc related variables

[David Bremner]

After several attempts [1] this is the best replacement for
notmuch-fcc-initialization I could come up with.

[1]: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=21174

[PATCH 1/2] emacs: define a notmuch-compose-mode based on message mode.

[David Bremner]

This is to provide a clean way of overriding e.g. keybindings when
sending mail from notmuch.
---
 emacs/notmuch-mua.el | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 33f1399..a7d3eaa 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -265,6 +265,9 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
   (message-goto-body)
   (set-buffer-modified-p nil))
 
+(define-derived-mode notmuch-compose-mode message-mode "notmuch-compose"
+  "Notmuch message composition mode. Mostly like `message-mode'")
+
 (defun notmuch-mua-mail (&optional to subject other-headers &rest other-args)
   "Invoke the notmuch mail composition window.
 
@@ -281,6 +284,7 @@ OTHER-ARGS are passed through to `message-mail'."
 		       (notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
 
   (apply #'message-mail to subject other-headers other-args)
+  (notmuch-compose-mode)
   (message-sort-headers)
   (message-hide-headers)
   (set-buffer-modified-p nil)
-- 
2.1.4

[PATCH 2/2] emacs: make modifications to message Fcc vars buffer-local

[David Bremner]

Previously we globally modified these variables, which tended to cause
problems for people using message-mode, but not notmuch-mua-mail, to
send mail.

User visible changes:

- calling notmuch-fcc-header-setup is no longer optional. OTOH, it
  seems to do the right thing if notmuch-fcc-dirs is set to nil.

- the Fcc header is visible during message composition

- the name in the mode line is changed, and (sadface) no longer
  matches the menu label.

- Previously notmuch-mua-send-and-exit was never called.  Either we
  misunderstood define-mail-user-agent, or it had a bug.  So there was
  no difference if the user called message-send-and-exit directly. Now
  there will be.
---
 emacs/notmuch-maildir-fcc.el | 23 +++++++++--------------
 emacs/notmuch-mua.el         |  6 +++++-
 test/test-lib.sh             |  4 ++--
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el
index 07eedba..c2f2f4c 100644
--- a/emacs/notmuch-maildir-fcc.el
+++ b/emacs/notmuch-maildir-fcc.el
@@ -59,23 +59,19 @@ yet when sending a mail."
  :require 'notmuch-fcc-initialization
  :group 'notmuch-send)
 
-(defun notmuch-fcc-initialization ()
-  "If notmuch-fcc-directories is set,
-   hook them into the message-fcc-handler-function"
-    ;; Set up the message-fcc-handler to move mails to the maildir in Fcc
-    ;; The parameter is set to mark messages as "seen"
-    (setq message-fcc-handler-function
-          (lambda (destdir)
-	    (notmuch-maildir-fcc-write-buffer-to-maildir destdir t)))
-    ;; add a hook to actually insert the Fcc header when sending
-    (add-hook 'message-header-setup-hook 'notmuch-fcc-header-setup))
+(defun notmuch-fcc-handler (destdir)
+  "Write buffer to `destdir', marking it as sent
+
+Intended to be dynamically bound to `message-fcc-handler-function'"
+    (notmuch-maildir-fcc-write-buffer-to-maildir destdir t))
 
 (defun notmuch-fcc-header-setup ()
   "Add an Fcc header to the current message buffer.
 
-Can be added to `message-send-hook' and will set the Fcc header
-based on the values of `notmuch-fcc-dirs'. An existing Fcc header
-will NOT be removed or replaced."
+Sets the Fcc header based on the values of `notmuch-fcc-dirs'.
+
+Originally intended to be use a hook function, but now called directly
+by notmuch-mua-mail"
 
   (let ((subdir
 	 (cond
@@ -213,6 +209,5 @@ return t if successful, and nil otherwise."
 	  (delete-file (concat destdir "/tmp/" msg-id))))
       t)))
 
-(notmuch-fcc-initialization)
 (provide 'notmuch-maildir-fcc)
 
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index a7d3eaa..4ae7d1e 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -268,6 +268,8 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
 (define-derived-mode notmuch-compose-mode message-mode "notmuch-compose"
   "Notmuch message composition mode. Mostly like `message-mode'")
 
+(define-key notmuch-compose-mode-map (kbd "C-c C-c") #'notmuch-mua-send-and-exit)
+
 (defun notmuch-mua-mail (&optional to subject other-headers &rest other-args)
   "Invoke the notmuch mail composition window.
 
@@ -285,6 +287,7 @@ OTHER-ARGS are passed through to `message-mail'."
 
   (apply #'message-mail to subject other-headers other-args)
   (notmuch-compose-mode)
+  (notmuch-fcc-header-setup)
   (message-sort-headers)
   (message-hide-headers)
   (set-buffer-modified-p nil)
@@ -398,7 +401,8 @@ will be addressed to all recipients of the source message."
 
 (defun notmuch-mua-send-and-exit (&optional arg)
   (interactive "P")
-  (message-send-and-exit arg))
+  (let ((message-fcc-handler-function #'notmuch-fcc-handler))
+    (message-send-and-exit arg)))
 
 (defun notmuch-mua-kill-buffer ()
   (interactive)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 3466e9c..cb8a6cf 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -487,7 +487,7 @@ emacs_deliver_message ()
 	   (message-goto-body)
 	   (insert \"${body}\")
 	   $@
-	   (message-send-and-exit))"
+	   (notmuch-mua-send-and-exit))"
 
     # In case message was sent properly, client waits for confirmation
     # before exiting and resuming control here; therefore making sure
@@ -522,7 +522,7 @@ emacs_fcc_message ()
 	   (message-goto-body)
 	   (insert \"${body}\")
 	   $@
-	   (message-send-and-exit))" || return 1
+	   (notmuch-mua-send-and-exit))" || return 1
     notmuch new >/dev/null
 }
 
-- 
2.1.4

Re: [PATCH 2/2] emacs: make modifications to message Fcc vars buffer-local

[Mark Walters]

On Sat, 01 Aug 2015, David Bremner <david@tethera.net> wrote:
> Previously we globally modified these variables, which tended to cause
> problems for people using message-mode, but not notmuch-mua-mail, to
> send mail.

I think I like this approach: I think we will like the freedom to tweak
message-mode.

> User visible changes:
>
> - calling notmuch-fcc-header-setup is no longer optional. OTOH, it
>   seems to do the right thing if notmuch-fcc-dirs is set to nil.
>
> - the Fcc header is visible during message composition

These both seem good. At the moment notmuch-fcc-header-setup checks
whether the fcc directory is a valid maildir. This used to be called on
sending but is now called when the message composition is started. I
think it would be better to move it into notmuch-mua-send-and-exit or
(perhaps better) in notmuch-fcc-handler itself (in case the user edits
it).

> - the name in the mode line is changed, and (sadface) no longer
>   matches the menu label.

Do you mean the buffer name is *mail*? We could just call rename-buffer
somewhere (maybe in our derived mode bit?)

> - Previously notmuch-mua-send-and-exit was never called.  Either we
>   misunderstood define-mail-user-agent, or it had a bug.  So there was
>   no difference if the user called message-send-and-exit directly. Now
>   there will be.

This is a little irritating in case people have customised key bindings
but I don't see a way round it. 

A related case, which we probably should fix, is that C-c C-s is bound to
message send (and don't exit), this probably needs a wrapper too.

Best wishes

Mark





> ---
>  emacs/notmuch-maildir-fcc.el | 23 +++++++++--------------
>  emacs/notmuch-mua.el         |  6 +++++-
>  test/test-lib.sh             |  4 ++--
>  3 files changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el
> index 07eedba..c2f2f4c 100644
> --- a/emacs/notmuch-maildir-fcc.el
> +++ b/emacs/notmuch-maildir-fcc.el
> @@ -59,23 +59,19 @@ yet when sending a mail."
>   :require 'notmuch-fcc-initialization
>   :group 'notmuch-send)
>  
> -(defun notmuch-fcc-initialization ()
> -  "If notmuch-fcc-directories is set,
> -   hook them into the message-fcc-handler-function"
> -    ;; Set up the message-fcc-handler to move mails to the maildir in Fcc
> -    ;; The parameter is set to mark messages as "seen"
> -    (setq message-fcc-handler-function
> -          (lambda (destdir)
> -	    (notmuch-maildir-fcc-write-buffer-to-maildir destdir t)))
> -    ;; add a hook to actually insert the Fcc header when sending
> -    (add-hook 'message-header-setup-hook 'notmuch-fcc-header-setup))
> +(defun notmuch-fcc-handler (destdir)
> +  "Write buffer to `destdir', marking it as sent
> +
> +Intended to be dynamically bound to `message-fcc-handler-function'"
> +    (notmuch-maildir-fcc-write-buffer-to-maildir destdir t))
>  
>  (defun notmuch-fcc-header-setup ()
>    "Add an Fcc header to the current message buffer.
>  
> -Can be added to `message-send-hook' and will set the Fcc header
> -based on the values of `notmuch-fcc-dirs'. An existing Fcc header
> -will NOT be removed or replaced."
> +Sets the Fcc header based on the values of `notmuch-fcc-dirs'.
> +
> +Originally intended to be use a hook function, but now called directly
> +by notmuch-mua-mail"
>  
>    (let ((subdir
>  	 (cond
> @@ -213,6 +209,5 @@ return t if successful, and nil otherwise."
>  	  (delete-file (concat destdir "/tmp/" msg-id))))
>        t)))
>  
> -(notmuch-fcc-initialization)
>  (provide 'notmuch-maildir-fcc)
>  
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index a7d3eaa..4ae7d1e 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -268,6 +268,8 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
>  (define-derived-mode notmuch-compose-mode message-mode "notmuch-compose"
>    "Notmuch message composition mode. Mostly like `message-mode'")
>  
> +(define-key notmuch-compose-mode-map (kbd "C-c C-c") #'notmuch-mua-send-and-exit)
> +
>  (defun notmuch-mua-mail (&optional to subject other-headers &rest other-args)
>    "Invoke the notmuch mail composition window.
>  
> @@ -285,6 +287,7 @@ OTHER-ARGS are passed through to `message-mail'."
>  
>    (apply #'message-mail to subject other-headers other-args)
>    (notmuch-compose-mode)
> +  (notmuch-fcc-header-setup)
>    (message-sort-headers)
>    (message-hide-headers)
>    (set-buffer-modified-p nil)
> @@ -398,7 +401,8 @@ will be addressed to all recipients of the source message."
>  
>  (defun notmuch-mua-send-and-exit (&optional arg)
>    (interactive "P")
> -  (message-send-and-exit arg))
> +  (let ((message-fcc-handler-function #'notmuch-fcc-handler))
> +    (message-send-and-exit arg)))
>  
>  (defun notmuch-mua-kill-buffer ()
>    (interactive)
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 3466e9c..cb8a6cf 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -487,7 +487,7 @@ emacs_deliver_message ()
>  	   (message-goto-body)
>  	   (insert \"${body}\")
>  	   $@
> -	   (message-send-and-exit))"
> +	   (notmuch-mua-send-and-exit))"
>  
>      # In case message was sent properly, client waits for confirmation
>      # before exiting and resuming control here; therefore making sure
> @@ -522,7 +522,7 @@ emacs_fcc_message ()
>  	   (message-goto-body)
>  	   (insert \"${body}\")
>  	   $@
> -	   (message-send-and-exit))" || return 1
> +	   (notmuch-mua-send-and-exit))" || return 1
>      notmuch new >/dev/null
>  }
>  
> -- 
> 2.1.4

Re: [PATCH 2/2] emacs: make modifications to message Fcc vars buffer-local

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

>
> Do you mean the buffer name is *mail*? We could just call rename-buffer
> somewhere (maybe in our derived mode bit?)

The mode-line says "notmuch-compose", but the menu is called
"Message". Maybe we should call the mode "notmuch-message" and write
"Notmuch Message" in the mode line. You know what Tomi always says about
naming things ;). 

>
> A related case, which we probably should fix, is that C-c C-s is bound to
> message send (and don't exit), this probably needs a wrapper too

That's a good point, and should probably even be done in the initial
merged patch series.

Re: [PATCH 2/2] emacs: make modifications to message Fcc vars buffer-local

[Tomi Ollila]

On Mon, Aug 03 2015, David Bremner <david@tethera.net> wrote:

> Mark Walters <markwalters1009@gmail.com> writes:
>
>>
>> Do you mean the buffer name is *mail*? We could just call rename-buffer
>> somewhere (maybe in our derived mode bit?)
>
> The mode-line says "notmuch-compose", but the menu is called
> "Message". Maybe we should call the mode "notmuch-message" and write
> "Notmuch Message" in the mode line. You know what Tomi always says about
> naming things ;). 

Looks like a good naming!

Also note that we do have the BOTH hard things currently effective during
developent and testing: cache invalidation, off-by-one errors and naming
things :D

Tomi

[PATCH 1/2] emacs: define a notmuch-compose-mode based on message mode.

[David Bremner]

This is to provide a clean way of overriding e.g. keybindings when
sending mail from notmuch.
---
 emacs/notmuch-mua.el | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 3e52d5e..1018321 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -268,6 +268,9 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
   (message-goto-body)
   (set-buffer-modified-p nil))
 
+(define-derived-mode notmuch-message-mode message-mode "Notmuch Message"
+  "Notmuch message composition mode. Mostly like `message-mode'")
+
 (defun notmuch-mua-mail (&optional to subject other-headers &rest other-args)
   "Invoke the notmuch mail composition window.
 
@@ -284,6 +287,7 @@ OTHER-ARGS are passed through to `message-mail'."
 		       (notmuch-user-name) " <" (notmuch-user-primary-email) ">")) other-headers))
 
   (apply #'message-mail to subject other-headers other-args)
+  (notmuch-message-mode)
   (message-sort-headers)
   (message-hide-headers)
   (set-buffer-modified-p nil)
-- 
2.1.4

[PATCH 2/2] emacs: make modifications to message Fcc vars buffer-local

[David Bremner]

Previously we globally modified these variables, which tended to cause
problems for people using message-mode, but not notmuch-mua-mail, to
send mail.

User visible changes:

- calling notmuch-fcc-header-setup is no longer optional. OTOH, it
  seems to do the right thing if notmuch-fcc-dirs is set to nil.

- the Fcc header is visible during message composition

- the name in the mode line is changed, and (sadface) no longer
  matches the menu label.

- Previously notmuch-mua-send-and-exit was never called.  Either we
  misunderstood define-mail-user-agent, or it had a bug.  So there was
  no difference if the user called message-send-and-exit directly. Now
  there will be.

- user bindings to C-c C-s and C-c C-s in message-mode-map are
  overridden. The user can override them in notmuch-message-mode-map,
  but then they're on their own for Fcc handling.
---
 emacs/notmuch-maildir-fcc.el | 23 +++++++++--------------
 emacs/notmuch-mua.el         | 15 +++++++++++++--
 test/test-lib.sh             |  4 ++--
 3 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el
index 07eedba..c2f2f4c 100644
--- a/emacs/notmuch-maildir-fcc.el
+++ b/emacs/notmuch-maildir-fcc.el
@@ -59,23 +59,19 @@ yet when sending a mail."
  :require 'notmuch-fcc-initialization
  :group 'notmuch-send)
 
-(defun notmuch-fcc-initialization ()
-  "If notmuch-fcc-directories is set,
-   hook them into the message-fcc-handler-function"
-    ;; Set up the message-fcc-handler to move mails to the maildir in Fcc
-    ;; The parameter is set to mark messages as "seen"
-    (setq message-fcc-handler-function
-          (lambda (destdir)
-	    (notmuch-maildir-fcc-write-buffer-to-maildir destdir t)))
-    ;; add a hook to actually insert the Fcc header when sending
-    (add-hook 'message-header-setup-hook 'notmuch-fcc-header-setup))
+(defun notmuch-fcc-handler (destdir)
+  "Write buffer to `destdir', marking it as sent
+
+Intended to be dynamically bound to `message-fcc-handler-function'"
+    (notmuch-maildir-fcc-write-buffer-to-maildir destdir t))
 
 (defun notmuch-fcc-header-setup ()
   "Add an Fcc header to the current message buffer.
 
-Can be added to `message-send-hook' and will set the Fcc header
-based on the values of `notmuch-fcc-dirs'. An existing Fcc header
-will NOT be removed or replaced."
+Sets the Fcc header based on the values of `notmuch-fcc-dirs'.
+
+Originally intended to be use a hook function, but now called directly
+by notmuch-mua-mail"
 
   (let ((subdir
 	 (cond
@@ -213,6 +209,5 @@ return t if successful, and nil otherwise."
 	  (delete-file (concat destdir "/tmp/" msg-id))))
       t)))
 
-(notmuch-fcc-initialization)
 (provide 'notmuch-maildir-fcc)
 
diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 1018321..803459a 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -268,9 +268,13 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
   (message-goto-body)
   (set-buffer-modified-p nil))
 
-(define-derived-mode notmuch-message-mode message-mode "Notmuch Message"
+(define-derived-mode notmuch-message-mode message-mode "Message[Notmuch]"
   "Notmuch message composition mode. Mostly like `message-mode'")
 
+(define-key notmuch-message-mode-map (kbd "C-c C-c") #'notmuch-mua-send-and-exit)
+(define-key notmuch-message-mode-map (kbd "C-c C-s") #'notmuch-mua-send)
+
+
 (defun notmuch-mua-mail (&optional to subject other-headers &rest other-args)
   "Invoke the notmuch mail composition window.
 
@@ -288,6 +292,7 @@ OTHER-ARGS are passed through to `message-mail'."
 
   (apply #'message-mail to subject other-headers other-args)
   (notmuch-message-mode)
+  (notmuch-fcc-header-setup)
   (message-sort-headers)
   (message-hide-headers)
   (set-buffer-modified-p nil)
@@ -401,7 +406,13 @@ will be addressed to all recipients of the source message."
 
 (defun notmuch-mua-send-and-exit (&optional arg)
   (interactive "P")
-  (message-send-and-exit arg))
+  (let ((message-fcc-handler-function #'notmuch-fcc-handler))
+    (message-send-and-exit arg)))
+
+(defun notmuch-mua-send (&optional arg)
+  (interactive "P")
+  (let ((message-fcc-handler-function #'notmuch-fcc-handler))
+    (message-send arg)))
 
 (defun notmuch-mua-kill-buffer ()
   (interactive)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index db3b6aa..eeb5487 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -487,7 +487,7 @@ emacs_deliver_message ()
 	   (message-goto-body)
 	   (insert \"${body}\")
 	   $@
-	   (message-send-and-exit))"
+	   (notmuch-mua-send-and-exit))"
 
     # In case message was sent properly, client waits for confirmation
     # before exiting and resuming control here; therefore making sure
@@ -522,7 +522,7 @@ emacs_fcc_message ()
 	   (message-goto-body)
 	   (insert \"${body}\")
 	   $@
-	   (message-send-and-exit))" || return 1
+	   (notmuch-mua-send-and-exit))" || return 1
     notmuch new >/dev/null
 }
 
-- 
2.1.4

Re: [PATCH 2/2] emacs: make modifications to message Fcc vars buffer-local

[Tomi Ollila]

On Tue, Aug 04 2015, David Bremner <david@tethera.net> wrote:

> Previously we globally modified these variables, which tended to cause
> problems for people using message-mode, but not notmuch-mua-mail, to
> send mail.
>
> User visible changes:
>
> - calling notmuch-fcc-header-setup is no longer optional. OTOH, it
>   seems to do the right thing if notmuch-fcc-dirs is set to nil.
>
> - the Fcc header is visible during message composition
>
> - the name in the mode line is changed, and (sadface) no longer
>   matches the menu label.
>
> - Previously notmuch-mua-send-and-exit was never called.  Either we
>   misunderstood define-mail-user-agent, or it had a bug.  So there was
>   no difference if the user called message-send-and-exit directly. Now
>   there will be.
>
> - user bindings to C-c C-s and C-c C-s in message-mode-map are
>   overridden. The user can override them in notmuch-message-mode-map,
>   but then they're on their own for Fcc handling.

This looks pretty good to me -- one of the above is `C-c C-c` and
capitalization could be consistent in the commit message.

One thing come to my mind: would it be possible to use buffer-local
variable message-fcc-handler-function in notmuch-message-mode buffers so
that such wrapper functions defined below would not be needed. i.e.

  (make-local-variable 'message-fcc-handler-function)
  (setq message-fcc-handler-function #'notmuch-fcc-handler)

(or not? :D)

one more thing (extra newline) inline below
 
Tomi

> ---
>  emacs/notmuch-maildir-fcc.el | 23 +++++++++--------------
>  emacs/notmuch-mua.el         | 15 +++++++++++++--
>  test/test-lib.sh             |  4 ++--
>  3 files changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el
> index 07eedba..c2f2f4c 100644
> --- a/emacs/notmuch-maildir-fcc.el
> +++ b/emacs/notmuch-maildir-fcc.el
> @@ -59,23 +59,19 @@ yet when sending a mail."
>   :require 'notmuch-fcc-initialization
>   :group 'notmuch-send)
>  
> -(defun notmuch-fcc-initialization ()
> -  "If notmuch-fcc-directories is set,
> -   hook them into the message-fcc-handler-function"
> -    ;; Set up the message-fcc-handler to move mails to the maildir in Fcc
> -    ;; The parameter is set to mark messages as "seen"
> -    (setq message-fcc-handler-function
> -          (lambda (destdir)
> -	    (notmuch-maildir-fcc-write-buffer-to-maildir destdir t)))
> -    ;; add a hook to actually insert the Fcc header when sending
> -    (add-hook 'message-header-setup-hook 'notmuch-fcc-header-setup))
> +(defun notmuch-fcc-handler (destdir)
> +  "Write buffer to `destdir', marking it as sent
> +
> +Intended to be dynamically bound to `message-fcc-handler-function'"
> +    (notmuch-maildir-fcc-write-buffer-to-maildir destdir t))
>  
>  (defun notmuch-fcc-header-setup ()
>    "Add an Fcc header to the current message buffer.
>  
> -Can be added to `message-send-hook' and will set the Fcc header
> -based on the values of `notmuch-fcc-dirs'. An existing Fcc header
> -will NOT be removed or replaced."
> +Sets the Fcc header based on the values of `notmuch-fcc-dirs'.
> +
> +Originally intended to be use a hook function, but now called directly
> +by notmuch-mua-mail"
>  
>    (let ((subdir
>  	 (cond
> @@ -213,6 +209,5 @@ return t if successful, and nil otherwise."
>  	  (delete-file (concat destdir "/tmp/" msg-id))))
>        t)))
>  
> -(notmuch-fcc-initialization)
>  (provide 'notmuch-maildir-fcc)
>  
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 1018321..803459a 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -268,9 +268,13 @@ Note that these functions use `mail-citation-hook' if that is non-nil."
>    (message-goto-body)
>    (set-buffer-modified-p nil))
>  
> -(define-derived-mode notmuch-message-mode message-mode "Notmuch Message"
> +(define-derived-mode notmuch-message-mode message-mode "Message[Notmuch]"
>    "Notmuch message composition mode. Mostly like `message-mode'")
>  
> +(define-key notmuch-message-mode-map (kbd "C-c C-c") #'notmuch-mua-send-and-exit)
> +(define-key notmuch-message-mode-map (kbd "C-c C-s") #'notmuch-mua-send)
> +
> +

2 empty lines above -- nowhere else in notmuch-mua.el is such a thing.

>  (defun notmuch-mua-mail (&optional to subject other-headers &rest other-args)
>    "Invoke the notmuch mail composition window.
>  
> @@ -288,6 +292,7 @@ OTHER-ARGS are passed through to `message-mail'."
>  
>    (apply #'message-mail to subject other-headers other-args)
>    (notmuch-message-mode)
> +  (notmuch-fcc-header-setup)
>    (message-sort-headers)
>    (message-hide-headers)
>    (set-buffer-modified-p nil)
> @@ -401,7 +406,13 @@ will be addressed to all recipients of the source message."
>  
>  (defun notmuch-mua-send-and-exit (&optional arg)
>    (interactive "P")
> -  (message-send-and-exit arg))
> +  (let ((message-fcc-handler-function #'notmuch-fcc-handler))
> +    (message-send-and-exit arg)))
> +
> +(defun notmuch-mua-send (&optional arg)
> +  (interactive "P")
> +  (let ((message-fcc-handler-function #'notmuch-fcc-handler))
> +    (message-send arg)))
>  
>  (defun notmuch-mua-kill-buffer ()
>    (interactive)
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index db3b6aa..eeb5487 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -487,7 +487,7 @@ emacs_deliver_message ()
>  	   (message-goto-body)
>  	   (insert \"${body}\")
>  	   $@
> -	   (message-send-and-exit))"
> +	   (notmuch-mua-send-and-exit))"
>  
>      # In case message was sent properly, client waits for confirmation
>      # before exiting and resuming control here; therefore making sure
> @@ -522,7 +522,7 @@ emacs_fcc_message ()
>  	   (message-goto-body)
>  	   (insert \"${body}\")
>  	   $@
> -	   (message-send-and-exit))" || return 1
> +	   (notmuch-mua-send-and-exit))" || return 1
>      notmuch new >/dev/null
>  }
>  
> -- 
> 2.1.4

Re: [PATCH 2/2] emacs: make modifications to message Fcc vars buffer-local

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

>
> One thing come to my mind: would it be possible to use buffer-local
> variable message-fcc-handler-function in notmuch-message-mode buffers so
> that such wrapper functions defined below would not be needed. i.e.
>
>   (make-local-variable 'message-fcc-handler-function)
>   (setq message-fcc-handler-function #'notmuch-fcc-handler)
>

This was my first approach, and I actually filed

     http://debbugs.gnu.org/cgi/bugreport.cgi?bug=21174

about why it doesn't work.

Great minds think alike / Fools seldom differ, as they say ;)

d

Re: [PATCH 2/2] emacs: make modifications to message Fcc vars buffer-local

[Tomi Ollila]

On Thu, Aug 06 2015, David Bremner <david@tethera.net> wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>>
>> One thing come to my mind: would it be possible to use buffer-local
>> variable message-fcc-handler-function in notmuch-message-mode buffers so
>> that such wrapper functions defined below would not be needed. i.e.
>>
>>   (make-local-variable 'message-fcc-handler-function)
>>   (setq message-fcc-handler-function #'notmuch-fcc-handler)
>>
>
> This was my first approach, and I actually filed
>
>      http://debbugs.gnu.org/cgi/bugreport.cgi?bug=21174
>
> about why it doesn't work.
>
> Great minds think alike / Fools seldom differ, as they say ;)

Uh ;/ You could add some note about buffer-local variables not
applicable in this context for future archeologists to understand ;)
-- either in commit message or in comment..

>
> d

Tomi

Re: [PATCH 2/2] emacs: make modifications to message Fcc vars buffer-local

[David Bremner]

David Bremner <david@tethera.net> writes:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>>
>> One thing come to my mind: would it be possible to use buffer-local
>> variable message-fcc-handler-function in notmuch-message-mode buffers so
>> that such wrapper functions defined below would not be needed. i.e.
>>
>>   (make-local-variable 'message-fcc-handler-function)
>>   (setq message-fcc-handler-function #'notmuch-fcc-handler)
>>
>
> This was my first approach, and I actually filed
>
>      http://debbugs.gnu.org/cgi/bugreport.cgi?bug=21174
>
> about why it doesn't work.
>
> Great minds think alike / Fools seldom differ, as they say ;)
>
> d

For the record, this bug is fixed in emacs git.

d

Re: [PATCH 2/2] emacs: make modifications to message Fcc vars buffer-local

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> capitalization could be consistent in the commit message.
>
> One thing come to my mind: would it be possible to use buffer-local
> variable message-fcc-handler-function in notmuch-message-mode buffers so
> that such wrapper functions defined below would not be needed. i.e.
>
>   (make-local-variable 'message-fcc-handler-function)
>   (setq message-fcc-handler-function #'notmuch-fcc-handler)
>
> (or not? :D)
>
> one more thing (extra newline) inline below
>  

Oops, missed that part. Pushed both with edited commit messages.

Re: [PATCH 2/2] emacs: make modifications to message Fcc vars buffer-local

[David Bremner]

David Bremner <david@tethera.net> writes:

>
> Oops, missed that part. Pushed both with edited commit messages.

And a whitespace cleanup patch.

d