unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] emacs: add notmuch-address-post-completion-hook
@ 2016-11-03  0:36 David Bremner
  2016-11-03  9:27 ` Mark Walters
  0 siblings, 1 reply; 15+ messages in thread
From: David Bremner @ 2016-11-03  0:36 UTC (permalink / raw)
  To: notmuch

This hook can be used to update the message based on the results of
address completion. For example I use it to set the From address based
on the To address just completed.

The post-completion command is added to the notmuch-company backend to
ensure that the hook is also called company completion is started
without going through notmuch-address-expand-name. See the docstring of
`company-backends' for more information.
---
 emacs/notmuch-address.el | 10 ++++++++++
 emacs/notmuch-company.el |  1 +
 2 files changed, 11 insertions(+)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index b2e1fba..17960bf 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -98,6 +98,12 @@ to know how address selection is made by default."
   :group 'notmuch-send
   :group 'notmuch-external)
 
+(defcustom notmuch-address-completion-hook nil
+  "Functions called after completing address. The completed address is passed as an argument to each"
+  :type 'hook
+  :group 'notmuch-address
+  :group 'notmuch-hooks)
+
 (defun notmuch-address-selection-function (prompt collection initial-input)
   "Call (`completing-read'
       PROMPT COLLECTION nil nil INITIAL-INPUT 'notmuch-address-history)"
@@ -170,6 +176,10 @@ external commands."
     (process-lines notmuch-address-command original))))
 
 (defun notmuch-address-expand-name ()
+  (let ((return-value (notmuch-address-real-expand-name)))
+    (run-hook-with-args 'notmuch-address-completion-hook)))
+
+(defun notmuch-address-real-expand-name ()
   (cond
    ((and (eq notmuch-address-command 'internal)
 	 notmuch-address-use-company
diff --git a/emacs/notmuch-company.el b/emacs/notmuch-company.el
index 168315f..91c4804 100644
--- a/emacs/notmuch-company.el
+++ b/emacs/notmuch-company.el
@@ -86,6 +86,7 @@
       (match (if (string-match notmuch-company-last-prefix arg)
 		 (match-end 0)
 	       0))
+      (post-completion (run-hook-with-args 'notmuch-address-completion-hook arg))
       (no-cache t))))
 
 
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] emacs: add notmuch-address-post-completion-hook
  2016-11-03  0:36 [PATCH] emacs: add notmuch-address-post-completion-hook David Bremner
@ 2016-11-03  9:27 ` Mark Walters
  2016-11-03 11:38   ` David Bremner
  2016-11-03 11:56   ` [PATCH v3] " David Bremner
  0 siblings, 2 replies; 15+ messages in thread
From: Mark Walters @ 2016-11-03  9:27 UTC (permalink / raw)
  To: David Bremner, notmuch


On Thu, 03 Nov 2016, David Bremner <david@tethera.net> wrote:
> This hook can be used to update the message based on the results of
> address completion. For example I use it to set the From address based
> on the To address just completed.
>
> The post-completion command is added to the notmuch-company backend to
> ensure that the hook is also called company completion is started
> without going through notmuch-address-expand-name. See the docstring of
> `company-backends' for more information.
> ---
>  emacs/notmuch-address.el | 10 ++++++++++
>  emacs/notmuch-company.el |  1 +
>  2 files changed, 11 insertions(+)
>
> diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
> index b2e1fba..17960bf 100644
> --- a/emacs/notmuch-address.el
> +++ b/emacs/notmuch-address.el
> @@ -98,6 +98,12 @@ to know how address selection is made by default."
>    :group 'notmuch-send
>    :group 'notmuch-external)
>  
> +(defcustom notmuch-address-completion-hook nil
> +  "Functions called after completing address. The completed address is passed as an argument to each"

Hi when testing this with company I think the following happens: if you
trigger it manually with tab then I think the hook gets passed a list of
possible addresses, not just one.

> +  :type 'hook
> +  :group 'notmuch-address
> +  :group 'notmuch-hooks)
> +
>  (defun notmuch-address-selection-function (prompt collection initial-input)
>    "Call (`completing-read'
>        PROMPT COLLECTION nil nil INITIAL-INPUT 'notmuch-address-history)"
> @@ -170,6 +176,10 @@ external commands."
>      (process-lines notmuch-address-command original))))
>  
>  (defun notmuch-address-expand-name ()
> +  (let ((return-value (notmuch-address-real-expand-name)))
> +    (run-hook-with-args 'notmuch-address-completion-hook)))

I think you need to return return-value here? But you also need to
modify notmuch-address-real-expand-name to return chosen. Around line
217 of notmuch-address.el

      (if chosen
	  (progn
	    (push chosen notmuch-address-history)
	    (delete-region beg end)
	    (insert chosen))

should become 

      (if chosen
	  (progn
	    (push chosen notmuch-address-history)
	    (delete-region beg end)
	    (insert chosen)
            ;; Return completed address 
            chosen)

I guess an alternative would to just run the hook there, and then you
wouldn't need to the notmuch-address-real-expand-name function at
all. (Sorry for misleading you about this on irc yesterday.)


> +
> +(defun notmuch-address-real-expand-name ()
>    (cond
>     ((and (eq notmuch-address-command 'internal)
>  	 notmuch-address-use-company
> diff --git a/emacs/notmuch-company.el b/emacs/notmuch-company.el
> index 168315f..91c4804 100644
> --- a/emacs/notmuch-company.el
> +++ b/emacs/notmuch-company.el
> @@ -86,6 +86,7 @@
>        (match (if (string-match notmuch-company-last-prefix arg)
>  		 (match-end 0)
>  	       0))
> +      (post-completion (run-hook-with-args 'notmuch-address-completion-hook arg))
>        (no-cache t))))

To fix the list of addresses case you might be able to just put a
(unless (listp arg) or something round the run-hook-with-args

Best wishes

Mark

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] emacs: add notmuch-address-post-completion-hook
  2016-11-03  9:27 ` Mark Walters
@ 2016-11-03 11:38   ` David Bremner
  2016-11-03 11:56   ` [PATCH v3] " David Bremner
  1 sibling, 0 replies; 15+ messages in thread
From: David Bremner @ 2016-11-03 11:38 UTC (permalink / raw)
  To: Mark Walters, David Bremner, notmuch

This hook can be used to update the message based on the results of
address completion. For example using message-templ or gnus-alias to set
the From address based on the To address just completed.

The post-completion command is added to the notmuch-company backend to
ensure that the hook is also called company completion is started
without going through notmuch-address-expand-name. See the docstring of
`company-backends' for more information.
---
 emacs/notmuch-address.el | 7 +++++++
 emacs/notmuch-company.el | 1 +
 2 files changed, 8 insertions(+)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index b2e1fba..5324c92 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -98,6 +98,12 @@ to know how address selection is made by default."
   :group 'notmuch-send
   :group 'notmuch-external)
 
+(defcustom notmuch-address-completion-hook nil
+  "Functions called after completing address. The completed address is passed as an argument to each"
+  :type 'hook
+  :group 'notmuch-address
+  :group 'notmuch-hooks)
+
 (defun notmuch-address-selection-function (prompt collection initial-input)
   "Call (`completing-read'
       PROMPT COLLECTION nil nil INITIAL-INPUT 'notmuch-address-history)"
@@ -205,6 +211,7 @@ external commands."
       (if chosen
 	  (progn
 	    (push chosen notmuch-address-history)
+	    (run-hook-with-args 'notmuch-address-completion-hook return-value)
 	    (delete-region beg end)
 	    (insert chosen))
 	(message "No matches.")
diff --git a/emacs/notmuch-company.el b/emacs/notmuch-company.el
index 168315f..91c4804 100644
--- a/emacs/notmuch-company.el
+++ b/emacs/notmuch-company.el
@@ -86,6 +86,7 @@
       (match (if (string-match notmuch-company-last-prefix arg)
 		 (match-end 0)
 	       0))
+      (post-completion (run-hook-with-args 'notmuch-address-completion-hook arg))
       (no-cache t))))
 
 
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3] emacs: add notmuch-address-post-completion-hook
  2016-11-03  9:27 ` Mark Walters
  2016-11-03 11:38   ` David Bremner
@ 2016-11-03 11:56   ` David Bremner
  2016-11-03 17:08     ` Mark Walters
  1 sibling, 1 reply; 15+ messages in thread
From: David Bremner @ 2016-11-03 11:56 UTC (permalink / raw)
  To: Mark Walters, David Bremner, notmuch

This hook can be used to update the message based on the results of
address completion. For example using message-templ or gnus-alias to set
the From address based on the To address just completed.

The post-completion command is added to the notmuch-company backend to
ensure that the hook is also called company completion is started
without going through notmuch-address-expand-name. See the docstring of
`company-backends' for more information.
---

Sorry for the false send. My fingers somehow mix up Debian's
"reportbug" and git-send-email.

Here is a simplified version. As far as I could tell during testing
with company-mode the hook is only called via the post-completion
command, and always with a single argument. This might indicate a
separate bug, since I noticed in one fairly long running emacs session
(11 days), notmuch-address-history is nil.

I guess the docstring for notmuch-address-completion-hook should have
a '.' at the end, or probably ' function.'


 emacs/notmuch-address.el | 7 +++++++
 emacs/notmuch-company.el | 1 +
 2 files changed, 8 insertions(+)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index b2e1fba..d1abb21 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -98,6 +98,12 @@ to know how address selection is made by default."
   :group 'notmuch-send
   :group 'notmuch-external)
 
+(defcustom notmuch-address-completion-hook nil
+  "Functions called after completing address. The completed address is passed as an argument to each"
+  :type 'hook
+  :group 'notmuch-address
+  :group 'notmuch-hooks)
+
 (defun notmuch-address-selection-function (prompt collection initial-input)
   "Call (`completing-read'
       PROMPT COLLECTION nil nil INITIAL-INPUT 'notmuch-address-history)"
@@ -205,6 +211,7 @@ external commands."
       (if chosen
 	  (progn
 	    (push chosen notmuch-address-history)
+	    (run-hook-with-args 'notmuch-address-completion-hook chosen)
 	    (delete-region beg end)
 	    (insert chosen))
 	(message "No matches.")
diff --git a/emacs/notmuch-company.el b/emacs/notmuch-company.el
index 168315f..91c4804 100644
--- a/emacs/notmuch-company.el
+++ b/emacs/notmuch-company.el
@@ -86,6 +86,7 @@
       (match (if (string-match notmuch-company-last-prefix arg)
 		 (match-end 0)
 	       0))
+      (post-completion (run-hook-with-args 'notmuch-address-completion-hook arg))
       (no-cache t))))
 
 
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] emacs: add notmuch-address-post-completion-hook
  2016-11-03 11:56   ` [PATCH v3] " David Bremner
@ 2016-11-03 17:08     ` Mark Walters
  2016-11-03 18:00       ` David Bremner
  0 siblings, 1 reply; 15+ messages in thread
From: Mark Walters @ 2016-11-03 17:08 UTC (permalink / raw)
  To: David Bremner, David Bremner, notmuch


On Thu, 03 Nov 2016, David Bremner <david@tethera.net> wrote:
> This hook can be used to update the message based on the results of
> address completion. For example using message-templ or gnus-alias to set
> the From address based on the To address just completed.
>
> The post-completion command is added to the notmuch-company backend to
> ensure that the hook is also called company completion is started
> without going through notmuch-address-expand-name. See the docstring of
> `company-backends' for more information.
> ---
>
> Sorry for the false send. My fingers somehow mix up Debian's
> "reportbug" and git-send-email.
>
> Here is a simplified version. As far as I could tell during testing
> with company-mode the hook is only called via the post-completion
> command, and always with a single argument. This might indicate a
> separate bug, since I noticed in one fairly long running emacs session
> (11 days), notmuch-address-history is nil.
>
> I guess the docstring for notmuch-address-completion-hook should have
> a '.' at the end, or probably ' function.'
>
>
>  emacs/notmuch-address.el | 7 +++++++
>  emacs/notmuch-company.el | 1 +
>  2 files changed, 8 insertions(+)
>
> diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
> index b2e1fba..d1abb21 100644
> --- a/emacs/notmuch-address.el
> +++ b/emacs/notmuch-address.el
> @@ -98,6 +98,12 @@ to know how address selection is made by default."
>    :group 'notmuch-send
>    :group 'notmuch-external)
>  
> +(defcustom notmuch-address-completion-hook nil
> +  "Functions called after completing address. The completed address is passed as an argument to each"
> +  :type 'hook
> +  :group 'notmuch-address
> +  :group 'notmuch-hooks)
> +
>  (defun notmuch-address-selection-function (prompt collection initial-input)
>    "Call (`completing-read'
>        PROMPT COLLECTION nil nil INITIAL-INPUT 'notmuch-address-history)"
> @@ -205,6 +211,7 @@ external commands."
>        (if chosen
>  	  (progn
>  	    (push chosen notmuch-address-history)
> +	    (run-hook-with-args 'notmuch-address-completion-hook chosen)
>  	    (delete-region beg end)
>  	    (insert chosen))

Hi

Would it be worth putting the run-hook after the (insert chosen) rather
than before? That would mean that the hook had access to the full new
header. It would also mean that it wouldn't matter if the hook changed
the buffer -- as it is I think the replace might go wrong as we replace
beg to end and those seem to be integer-points not markers

One final query -- this function will be called when completing any of
To: Cc: Bcc: From: and some other less common headers. We could pass an
argument which says which header we are on but that is probably more
complexity than necessary. However, it is probably worth documenting
that it may be called from these headers in the defcustom for the hook.

But otherwise it looks good to me.

Best wishes

Mark



>  	(message "No matches.")
> diff --git a/emacs/notmuch-company.el b/emacs/notmuch-company.el
> index 168315f..91c4804 100644
> --- a/emacs/notmuch-company.el
> +++ b/emacs/notmuch-company.el
> @@ -86,6 +86,7 @@
>        (match (if (string-match notmuch-company-last-prefix arg)
>  		 (match-end 0)
>  	       0))
> +      (post-completion (run-hook-with-args 'notmuch-address-completion-hook arg))
>        (no-cache t))))
>  
>  
> -- 
> 2.10.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] emacs: add notmuch-address-post-completion-hook
  2016-11-03 17:08     ` Mark Walters
@ 2016-11-03 18:00       ` David Bremner
  2016-11-04  1:02         ` [PATCH v4] " David Bremner
  0 siblings, 1 reply; 15+ messages in thread
From: David Bremner @ 2016-11-03 18:00 UTC (permalink / raw)
  To: Mark Walters, notmuch

Mark Walters <markwalters1009@gmail.com> writes:


> Hi
>
> Would it be worth putting the run-hook after the (insert chosen) rather
> than before? That would mean that the hook had access to the full new
> header. It would also mean that it wouldn't matter if the hook changed
> the buffer -- as it is I think the replace might go wrong as we replace
> beg to end and those seem to be integer-points not markers

agreed.

>
> One final query -- this function will be called when completing any of
> To: Cc: Bcc: From: and some other less common headers. We could pass an
> argument which says which header we are on but that is probably more
> complexity than necessary. However, it is probably worth documenting
> that it may be called from these headers in the defcustom for the hook.
>

Just changing the doc sounds sensible to me. Adding extra arguments
sounds complicated since we are also calling the hook from within
the nothmuch-company backend.

d

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4] emacs: add notmuch-address-post-completion-hook
  2016-11-03 18:00       ` David Bremner
@ 2016-11-04  1:02         ` David Bremner
  2016-11-04  6:33           ` Mark Walters
                             ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: David Bremner @ 2016-11-04  1:02 UTC (permalink / raw)
  To: David Bremner, Mark Walters, notmuch

This hook can be used to update the message based on the results of
address completion. For example using message-templ or gnus-alias to set
the From address based on the To address just completed.

The post-completion command is added to the notmuch-company backend to
ensure that the hook is also called company completion is started
without going through notmuch-address-expand-name. See the docstring of
`company-backends' for more information.
---

Updates from Mark's second review
 emacs/notmuch-address.el | 14 +++++++++++++-
 emacs/notmuch-company.el |  1 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index b2e1fba..36c796f 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -98,6 +98,17 @@ to know how address selection is made by default."
   :group 'notmuch-send
   :group 'notmuch-external)
 
+(defcustom notmuch-address-completion-hook nil
+  "Functions called after completing address.
+
+The completed address is passed as an argument to each function.
+Note that this hook will be invoked for completion in headers
+matching `notmuch-address-completion-headers-regexp'.
+"
+  :type 'hook
+  :group 'notmuch-address
+  :group 'notmuch-hooks)
+
 (defun notmuch-address-selection-function (prompt collection initial-input)
   "Call (`completing-read'
       PROMPT COLLECTION nil nil INITIAL-INPUT 'notmuch-address-history)"
@@ -206,7 +217,8 @@ external commands."
 	  (progn
 	    (push chosen notmuch-address-history)
 	    (delete-region beg end)
-	    (insert chosen))
+	    (insert chosen)
+	    (run-hook-with-args 'notmuch-address-completion-hook chosen))
 	(message "No matches.")
 	(ding))))
    (t nil)))
diff --git a/emacs/notmuch-company.el b/emacs/notmuch-company.el
index 168315f..91c4804 100644
--- a/emacs/notmuch-company.el
+++ b/emacs/notmuch-company.el
@@ -86,6 +86,7 @@
       (match (if (string-match notmuch-company-last-prefix arg)
 		 (match-end 0)
 	       0))
+      (post-completion (run-hook-with-args 'notmuch-address-completion-hook arg))
       (no-cache t))))
 
 
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] emacs: add notmuch-address-post-completion-hook
  2016-11-04  1:02         ` [PATCH v4] " David Bremner
@ 2016-11-04  6:33           ` Mark Walters
  2016-11-04 15:53           ` David Bremner
  2016-11-12 19:07           ` Tomi Ollila
  2 siblings, 0 replies; 15+ messages in thread
From: Mark Walters @ 2016-11-04  6:33 UTC (permalink / raw)
  To: David Bremner, David Bremner, notmuch


This version looks good to me +1

Best wishes

Mark

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] emacs: add notmuch-address-post-completion-hook
  2016-11-04  1:02         ` [PATCH v4] " David Bremner
  2016-11-04  6:33           ` Mark Walters
@ 2016-11-04 15:53           ` David Bremner
  2016-11-12 19:07           ` Tomi Ollila
  2 siblings, 0 replies; 15+ messages in thread
From: David Bremner @ 2016-11-04 15:53 UTC (permalink / raw)
  To: Mark Walters, notmuch

David Bremner <david@tethera.net> writes:

> This hook can be used to update the message based on the results of
> address completion. For example using message-templ or gnus-alias to set
> the From address based on the To address just completed.
>
> The post-completion command is added to the notmuch-company backend to
> ensure that the hook is also called company completion is started
> without going through notmuch-address-expand-name. See the docstring of
> `company-backends' for more information.

pushed to master

d

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] emacs: add notmuch-address-post-completion-hook
  2016-11-04  1:02         ` [PATCH v4] " David Bremner
  2016-11-04  6:33           ` Mark Walters
  2016-11-04 15:53           ` David Bremner
@ 2016-11-12 19:07           ` Tomi Ollila
  2016-11-12 21:39             ` David Bremner
  2 siblings, 1 reply; 15+ messages in thread
From: Tomi Ollila @ 2016-11-12 19:07 UTC (permalink / raw)
  To: David Bremner, Mark Walters, notmuch

On Fri, Nov 04 2016, David Bremner <david@tethera.net> wrote:

> This hook can be used to update the message based on the results of
> address completion. For example using message-templ or gnus-alias to set
> the From address based on the To address just completed.
>
> The post-completion command is added to the notmuch-company backend to
> ensure that the hook is also called company completion is started
> without going through notmuch-address-expand-name. See the docstring of
> `company-backends' for more information.
> ---
>
> Updates from Mark's second review
>  emacs/notmuch-address.el | 14 +++++++++++++-
>  emacs/notmuch-company.el |  1 +
>  2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
> index b2e1fba..36c796f 100644
> --- a/emacs/notmuch-address.el
> +++ b/emacs/notmuch-address.el
> @@ -98,6 +98,17 @@ to know how address selection is made by default."
>    :group 'notmuch-send
>    :group 'notmuch-external)
>  
> +(defcustom notmuch-address-completion-hook nil
> +  "Functions called after completing address.
> +
> +The completed address is passed as an argument to each function.
> +Note that this hook will be invoked for completion in headers
> +matching `notmuch-address-completion-headers-regexp'.
> +"
> +  :type 'hook
> +  :group 'notmuch-address
> +  :group 'notmuch-hooks)
> +
>  (defun notmuch-address-selection-function (prompt collection initial-input)
>    "Call (`completing-read'
>        PROMPT COLLECTION nil nil INITIAL-INPUT 'notmuch-address-history)"
> @@ -206,7 +217,8 @@ external commands."
>  	  (progn
>  	    (push chosen notmuch-address-history)
>  	    (delete-region beg end)
> -	    (insert chosen))
> +	    (insert chosen)
> +	    (run-hook-with-args 'notmuch-address-completion-hook chosen))

Like someone (whose message I cannot find just now) mentioned in another
thread, just now it is right time to mention here too...

https://www.gnu.org/software/emacs/manual/html_node/elisp/Hooks.html

... that when hook name ends with `-hook` it is supposed to be "normal hook"
-- a function which does not take arguments nor return values.

So, I'd like to suggest that this variable is renamed to 
notmuch-address-completion-functions


Tomi


>  	(message "No matches.")
>  	(ding))))
>     (t nil)))

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] emacs: add notmuch-address-post-completion-hook
  2016-11-12 19:07           ` Tomi Ollila
@ 2016-11-12 21:39             ` David Bremner
  2016-11-13  7:34               ` Mark Walters
  0 siblings, 1 reply; 15+ messages in thread
From: David Bremner @ 2016-11-12 21:39 UTC (permalink / raw)
  To: Tomi Ollila, Mark Walters, notmuch

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

> Like someone (whose message I cannot find just now) mentioned in another
> thread, just now it is right time to mention here too...
>
> https://www.gnu.org/software/emacs/manual/html_node/elisp/Hooks.html
>
> ... that when hook name ends with `-hook` it is supposed to be "normal hook"
> -- a function which does not take arguments nor return values.
>
> So, I'd like to suggest that this variable is renamed to 
> notmuch-address-completion-functions
>

Well, I guess the -functions convention should be followed, but
notmuch-address-completion-functions seems a bit vague.

d

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] emacs: add notmuch-address-post-completion-hook
  2016-11-12 21:39             ` David Bremner
@ 2016-11-13  7:34               ` Mark Walters
  2016-11-13 12:39                 ` Tomi Ollila
  0 siblings, 1 reply; 15+ messages in thread
From: Mark Walters @ 2016-11-13  7:34 UTC (permalink / raw)
  To: David Bremner, Tomi Ollila, notmuch

On Sat, 12 Nov 2016, David Bremner <david@tethera.net> wrote:
> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>> Like someone (whose message I cannot find just now) mentioned in another
>> thread, just now it is right time to mention here too...
>>
>> https://www.gnu.org/software/emacs/manual/html_node/elisp/Hooks.html
>>
>> ... that when hook name ends with `-hook` it is supposed to be "normal hook"
>> -- a function which does not take arguments nor return values.
>>
>> So, I'd like to suggest that this variable is renamed to 
>> notmuch-address-completion-functions
>>
>
> Well, I guess the -functions convention should be followed, but
> notmuch-address-completion-functions seems a bit vague.

Maybe notmuch-address-post-completion-functions ?

Alternatively maybe we can end in -hook-functions to indicate it is a
function which is like a hook?

Best wishes

Mark

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4] emacs: add notmuch-address-post-completion-hook
  2016-11-13  7:34               ` Mark Walters
@ 2016-11-13 12:39                 ` Tomi Ollila
  2016-11-13 13:03                   ` [PATCH] emacs: rename notmuch-address-completion-hook to notmuch-address-post-completion-functions David Bremner
  0 siblings, 1 reply; 15+ messages in thread
From: Tomi Ollila @ 2016-11-13 12:39 UTC (permalink / raw)
  To: David Bremner, notmuch


(Trying to send this w/o Mark in To: -- in the hope also Mark receive this
email to his gmail account...)

On Sun, Nov 13 2016, Mark Walters <markwalters1009@gmail.com> wrote:

> On Sat, 12 Nov 2016, David Bremner <david@tethera.net> wrote:
>> Tomi Ollila <tomi.ollila@iki.fi> writes:
>>
>>> Like someone (whose message I cannot find just now) mentioned in another
>>> thread, just now it is right time to mention here too...
>>>
>>> https://www.gnu.org/software/emacs/manual/html_node/elisp/Hooks.html
>>>
>>> ... that when hook name ends with `-hook` it is supposed to be "normal hook"
>>> -- a function which does not take arguments nor return values.
>>>
>>> So, I'd like to suggest that this variable is renamed to 
>>> notmuch-address-completion-functions
>>>
>>
>> Well, I guess the -functions convention should be followed, but
>> notmuch-address-completion-functions seems a bit vague.
>
> Maybe notmuch-address-post-completion-functions ?
>
> Alternatively maybe we can end in -hook-functions to indicate it is a
> function which is like a hook?

I found *-hook-functions format mystically familiar but could not found any
references in emacs 24.5 (nor now latest git) elisp source; there has been
loadhist-hook-functions and vc-backend-hook-functions in the source but
those have been removed/replaced later (w/o explanation why). otoh there
are plenty of *-hook-function references (which value seem to be expected to
be symbol to function). Then I recognized I have had such definitions in
my *own* elisp configuration files something like 25 years... ;) (copied
from someone in my early emacs days).

So, after more grepping it looks to me that this 
    notmuch-address-post-completion-functions
is pretty good suggestion.

(Grepping through elisp source distributed with emacs there are quite
a few lines like (run-hook-with-args '...-hook args..). it is easy to
end up being incompliant with the emacs convention. But other code
not being compliant is not good reason to do the same (and thanks to
Matt (in id:qf537j3hu4p.fsf@marmstrong-linux.kir.corp.google.com) for
rest of us to recognize this early enough))

BTW: I just noticed that the message subject said: 
emacs: add notmuch-address-post-completion-hook
but the currently used variable name lacked this -post- part
-- caused a bit of confusion of what/why am I suggesting here ;)

Tomi

>
> Best wishes
>
> Mark

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] emacs: rename notmuch-address-completion-hook to notmuch-address-post-completion-functions
  2016-11-13 12:39                 ` Tomi Ollila
@ 2016-11-13 13:03                   ` David Bremner
  2016-11-13 17:28                     ` David Bremner
  0 siblings, 1 reply; 15+ messages in thread
From: David Bremner @ 2016-11-13 13:03 UTC (permalink / raw)
  To: Tomi Ollila, David Bremner, notmuch

Apparently it is a (not completely adhered to) emacs convention [1] that
only hooks that don't take arguments end in 'hook'

[1]: https://www.gnu.org/software/emacs/manual/html_node/elisp/Hooks.html
---
 emacs/notmuch-address.el | 4 ++--
 emacs/notmuch-company.el | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index 36c796f..5b2beef 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -98,7 +98,7 @@ to know how address selection is made by default."
   :group 'notmuch-send
   :group 'notmuch-external)
 
-(defcustom notmuch-address-completion-hook nil
+(defcustom notmuch-address-post-completion-functions nil
   "Functions called after completing address.
 
 The completed address is passed as an argument to each function.
@@ -218,7 +218,7 @@ external commands."
 	    (push chosen notmuch-address-history)
 	    (delete-region beg end)
 	    (insert chosen)
-	    (run-hook-with-args 'notmuch-address-completion-hook chosen))
+	    (run-hook-with-args 'notmuch-address-post-completion-functions chosen))
 	(message "No matches.")
 	(ding))))
    (t nil)))
diff --git a/emacs/notmuch-company.el b/emacs/notmuch-company.el
index 91c4804..b0f9782 100644
--- a/emacs/notmuch-company.el
+++ b/emacs/notmuch-company.el
@@ -86,7 +86,7 @@
       (match (if (string-match notmuch-company-last-prefix arg)
 		 (match-end 0)
 	       0))
-      (post-completion (run-hook-with-args 'notmuch-address-completion-hook arg))
+      (post-completion (run-hook-with-args 'notmuch-address-post-completion-functions arg))
       (no-cache t))))
 
 
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] emacs: rename notmuch-address-completion-hook to notmuch-address-post-completion-functions
  2016-11-13 13:03                   ` [PATCH] emacs: rename notmuch-address-completion-hook to notmuch-address-post-completion-functions David Bremner
@ 2016-11-13 17:28                     ` David Bremner
  0 siblings, 0 replies; 15+ messages in thread
From: David Bremner @ 2016-11-13 17:28 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

David Bremner <david@tethera.net> writes:

> Apparently it is a (not completely adhered to) emacs convention [1] that
> only hooks that don't take arguments end in 'hook'
>
> [1]: https://www.gnu.org/software/emacs/manual/html_node/elisp/Hooks.html
> ---

pushed to master

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-11-13 17:28 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-03  0:36 [PATCH] emacs: add notmuch-address-post-completion-hook David Bremner
2016-11-03  9:27 ` Mark Walters
2016-11-03 11:38   ` David Bremner
2016-11-03 11:56   ` [PATCH v3] " David Bremner
2016-11-03 17:08     ` Mark Walters
2016-11-03 18:00       ` David Bremner
2016-11-04  1:02         ` [PATCH v4] " David Bremner
2016-11-04  6:33           ` Mark Walters
2016-11-04 15:53           ` David Bremner
2016-11-12 19:07           ` Tomi Ollila
2016-11-12 21:39             ` David Bremner
2016-11-13  7:34               ` Mark Walters
2016-11-13 12:39                 ` Tomi Ollila
2016-11-13 13:03                   ` [PATCH] emacs: rename notmuch-address-completion-hook to notmuch-address-post-completion-functions David Bremner
2016-11-13 17:28                     ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).