[PATCH] RFC: eldoc-documentation-functions hook

[PATCH] RFC: eldoc-documentation-functions hook

[Mark Oteiza]

This is a draft patch changing eldoc-documentation-function into a hook
variable, so instead of using add-function, one can instead use add-hook
to control the behavior of eldoc.  It is backwards compatible.

Perhaps the main motivation for doing this is that I don't like the
unreadable mess I see when looking at the value of
eldoc-documentation-function.  For example, the value in
*scratch* from emacs -Q.  This also exists in other places, like lambdas
as defcustom values in eshell, but that's another matter.

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 6c2f869..bfbfd41 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -43,7 +43,7 @@
 
 ;; Major modes for other languages may use ElDoc by defining an
 ;; appropriate function as the buffer-local value of
-;; `eldoc-documentation-function'.
+;; `eldoc-documentation-functions'.
 
 ;;; Code:
 
@@ -80,7 +80,7 @@ eldoc-argument-case
 returns another string is acceptable.
 
 Note that this variable has no effect, unless
-`eldoc-documentation-function' handles it explicitly."
+`eldoc-documentation-functions' handles it explicitly."
   :type '(radio (function-item upcase)
 		(function-item downcase)
                 function)
@@ -103,7 +103,7 @@ eldoc-echo-area-use-multiline-p
 truncated to make more of the arglist or documentation string visible.
 
 Note that this variable has no effect, unless
-`eldoc-documentation-function' handles it explicitly."
+`eldoc-documentation-functions' handles it explicitly."
   :type '(radio (const :tag "Always" t)
                 (const :tag "Never" nil)
                 (const :tag "Yes, but truncate symbol names if it will\
@@ -113,7 +113,7 @@ eldoc-echo-area-use-multiline-p
 (defface eldoc-highlight-function-argument
   '((t (:inherit bold)))
   "Face used for the argument at point in a function's argument list.
-Note that this face has no effect unless the `eldoc-documentation-function'
+Note that this face has no effect unless the `eldoc-documentation-functions'
 handles it explicitly."
   :group 'eldoc)
 
@@ -186,7 +186,7 @@ eldoc-mode
   :group 'eldoc :lighter eldoc-minor-mode-string
   (setq eldoc-last-message nil)
   (cond
-   ((memq eldoc-documentation-function '(nil ignore))
+   ((not (eldoc-supported-p))
     (message "There is no ElDoc support in this buffer")
     (setq eldoc-mode nil))
    (eldoc-mode
@@ -211,7 +211,7 @@ global-eldoc-mode
 
 If Global Eldoc mode is on, `eldoc-mode' will be enabled in all
 buffers where it's applicable.  These are buffers that have modes
-that have enabled eldoc support.  See `eldoc-documentation-function'."
+that have enabled eldoc support.  See `eldoc-documentation-functions'."
   :group 'eldoc
   :global t
   :initialize 'custom-initialize-delay
@@ -236,9 +236,7 @@ eldoc-schedule-timer
 	     eldoc-idle-delay nil
 	     (lambda ()
                (when (or eldoc-mode
-                         (and global-eldoc-mode
-                              (not (memq eldoc-documentation-function
-                                         '(nil ignore)))))
+                         (and global-eldoc-mode (eldoc-supported-p)))
                  (eldoc-print-current-symbol-info))))))
 
   ;; If user has changed the idle delay, update the timer.
@@ -334,26 +332,29 @@ eldoc-display-message-no-interference-p
 
 \f
 ;;;###autoload
-(defvar eldoc-documentation-function #'ignore
-  "Function to call to return doc string.
-The function of no args should return a one-line string for displaying
-doc about a function etc. appropriate to the context around point.
-It should return nil if there's no doc appropriate for the context.
-Typically doc is returned if point is on a function-like name or in its
-arg list.
+(defvar eldoc-documentation-functions #'ignore
+  "Hook to run to obtain doc string.
+Each element of this variable should be a function of no args
+that should return a one-line string for displaying doc about a
+function etc. appropriate to the context around point.  It should
+return nil if there is no doc appropriate for the context.
+Typically, doc is returned if point is on a function-like name or
+in its arg list.
 
 The result is used as is, so the function must explicitly handle
 the variables `eldoc-argument-case' and `eldoc-echo-area-use-multiline-p',
 and the face `eldoc-highlight-function-argument', if they are to have any
 effect.
 
-Major modes should modify this variable using `add-function', for example:
-  (add-function :before-until (local \\='eldoc-documentation-function)
-                #\\='foo-mode-eldoc-function)
+Major modes should modify this variable using `add-hook', for example:
+  (add-hook \\='eldoc-documentation-functions #\\='foo-eldoc nil t)
 so that the global documentation function (i.e. the default value of the
 variable) is taken into account if the major mode specific function does not
 return any documentation.")
 
+(define-obsolete-variable-alias 'eldoc-documentation-function
+  'eldoc-documentation-functions "25.2")
+
 (defun eldoc-print-current-symbol-info ()
   ;; This is run from post-command-hook or some idle timer thing,
   ;; so we need to be careful that errors aren't ignored.
@@ -363,7 +364,19 @@ eldoc-print-current-symbol-info
              (when eldoc-last-message
                (eldoc-message nil)
                nil))
-	 (eldoc-message (funcall eldoc-documentation-function)))))
+	 (eldoc-message
+          (run-hook-with-args-until-success 'eldoc-documentation-functions)))))
+
+(defun eldoc-supported-p ()
+  "Return t if `eldoc-documentation-functions' has non-null elements."
+  (when eldoc-documentation-functions 
+    (catch :eldoc-supported
+      (mapc
+       (lambda (fun)
+         (when (not (memq fun '(nil ignore)))
+           (throw :eldoc-supported t)))
+       eldoc-documentation-functions)
+      nil)))
 
 ;; If the entire line cannot fit in the echo area, the symbol name may be
 ;; truncated or eliminated entirely from the output to make room for the

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Eli Zaretskii]

> Date: Sun, 12 Jun 2016 02:12:29 -0400
> From: Mark Oteiza <mvoteiza@udel.edu>
> 
> This is a draft patch changing eldoc-documentation-function into a hook
> variable, so instead of using add-function, one can instead use add-hook
> to control the behavior of eldoc.  It is backwards compatible.

Thanks.  Please be sure to update the ELisp manual accordingly.

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Leo Liu]

On 2016-06-12 10:09 +0300, Eli Zaretskii wrote:
> Thanks.  Please be sure to update the ELisp manual accordingly.

I think the merit of the change needs to be debated. Add-function gives
us a lot of flexibility and is used in quite a few places. Also this is
incompatible change.

Leo

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Eli Zaretskii]

> From: Leo Liu <sdl.web@gmail.com>
> Date: Sun, 12 Jun 2016 15:46:16 +0800
> 
> On 2016-06-12 10:09 +0300, Eli Zaretskii wrote:
> > Thanks.  Please be sure to update the ELisp manual accordingly.
> 
> I think the merit of the change needs to be debated.

Please go ahead and debate.  Al I'm saying is that the patch is
incomplete without the corresponding change to the manual.

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Noam Postavsky]

On Sun, Jun 12, 2016 at 2:12 AM, Mark Oteiza <mvoteiza@udel.edu> wrote:
> Perhaps the main motivation for doing this is that I don't like the
> unreadable mess I see when looking at the value of
> eldoc-documentation-function.  For example, the value in
> *scratch* from emacs -Q.  This also exists in other places, like lambdas
> as defcustom values in eshell, but that's another matter.

Could we improve the display of function values instead?

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Mark Oteiza]

---

On 12/06/16 at 10:09am, Eli Zaretskii wrote:
> > Date: Sun, 12 Jun 2016 02:12:29 -0400
> > From: Mark Oteiza <mvoteiza@udel.edu>
> > 
> > This is a draft patch changing eldoc-documentation-function into a hook
> > variable, so instead of using add-function, one can instead use add-hook
> > to control the behavior of eldoc.  It is backwards compatible.
> 
> Thanks.  Please be sure to update the ELisp manual accordingly.

Thanks, looks like there's just one place. Also added a mention in NEWS
and changed the default value back to nil, since this isn't being
unconditionally funcall'd anymore.

In the manual, it appears that the -functions suffix is to be used for
abnormal hooks; however this is a normal hook, so it would seem the name
should use the -hook suffix.

diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi
index 7b76e6a..b5d07da 100644
--- a/doc/lispref/modes.texi
+++ b/doc/lispref/modes.texi
@@ -417,7 +417,7 @@ Major Mode Conventions
 
 @item
 The mode can specify a local value for
-@code{eldoc-documentation-function} to tell ElDoc mode how to handle
+@code{eldoc-documentation-functions} to tell ElDoc mode how to handle
 this mode.
 
 @item
diff --git a/etc/NEWS b/etc/NEWS
index e2c99a1..92bf8e4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -212,6 +212,12 @@ viewing HTML files and the like.
 breakpoint (e.g. with "f" and "o") by customizing the new option
 'edebug-sit-on-break'.
 
+** ElDoc
+
++++
+*** 'eldoc-documentation-functions' replaces 'eldoc-documentation-function'.
+This is now a hook variable; its default value is nil.
+
 ** eww
 
 +++
diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 6c2f869..812c398 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -43,7 +43,7 @@
 
 ;; Major modes for other languages may use ElDoc by defining an
 ;; appropriate function as the buffer-local value of
-;; `eldoc-documentation-function'.
+;; `eldoc-documentation-functions'.
 
 ;;; Code:
 
@@ -80,7 +80,7 @@ eldoc-argument-case
 returns another string is acceptable.
 
 Note that this variable has no effect, unless
-`eldoc-documentation-function' handles it explicitly."
+`eldoc-documentation-functions' handle it explicitly."
   :type '(radio (function-item upcase)
 		(function-item downcase)
                 function)
@@ -103,7 +103,7 @@ eldoc-echo-area-use-multiline-p
 truncated to make more of the arglist or documentation string visible.
 
 Note that this variable has no effect, unless
-`eldoc-documentation-function' handles it explicitly."
+`eldoc-documentation-functions' handle it explicitly."
   :type '(radio (const :tag "Always" t)
                 (const :tag "Never" nil)
                 (const :tag "Yes, but truncate symbol names if it will\
@@ -113,8 +113,8 @@ eldoc-echo-area-use-multiline-p
 (defface eldoc-highlight-function-argument
   '((t (:inherit bold)))
   "Face used for the argument at point in a function's argument list.
-Note that this face has no effect unless the `eldoc-documentation-function'
-handles it explicitly."
+Note that this face has no effect unless the `eldoc-documentation-functions'
+handle it explicitly."
   :group 'eldoc)
 
 ;;; No user options below here.
@@ -186,7 +186,7 @@ eldoc-mode
   :group 'eldoc :lighter eldoc-minor-mode-string
   (setq eldoc-last-message nil)
   (cond
-   ((memq eldoc-documentation-function '(nil ignore))
+   ((not (eldoc-supported-p))
     (message "There is no ElDoc support in this buffer")
     (setq eldoc-mode nil))
    (eldoc-mode
@@ -211,7 +211,7 @@ global-eldoc-mode
 
 If Global Eldoc mode is on, `eldoc-mode' will be enabled in all
 buffers where it's applicable.  These are buffers that have modes
-that have enabled eldoc support.  See `eldoc-documentation-function'."
+that have enabled eldoc support.  See `eldoc-documentation-functions'."
   :group 'eldoc
   :global t
   :initialize 'custom-initialize-delay
@@ -236,9 +236,7 @@ eldoc-schedule-timer
 	     eldoc-idle-delay nil
 	     (lambda ()
                (when (or eldoc-mode
-                         (and global-eldoc-mode
-                              (not (memq eldoc-documentation-function
-                                         '(nil ignore)))))
+                         (and global-eldoc-mode (eldoc-supported-p)))
                  (eldoc-print-current-symbol-info))))))
 
   ;; If user has changed the idle delay, update the timer.
@@ -334,26 +332,29 @@ eldoc-display-message-no-interference-p
 
 \f
 ;;;###autoload
-(defvar eldoc-documentation-function #'ignore
-  "Function to call to return doc string.
-The function of no args should return a one-line string for displaying
-doc about a function etc. appropriate to the context around point.
-It should return nil if there's no doc appropriate for the context.
-Typically doc is returned if point is on a function-like name or in its
-arg list.
+(defvar eldoc-documentation-functions nil
+  "Hook to run to obtain doc string.
+Each element of this variable should be a function of no args
+that returns a one-line string for displaying doc about a
+function etc. appropriate to the context around point.  It should
+return nil if there is no doc appropriate for the context.
+Typically, doc is returned if point is on a function-like name or
+in its arg list.
 
 The result is used as is, so the function must explicitly handle
 the variables `eldoc-argument-case' and `eldoc-echo-area-use-multiline-p',
 and the face `eldoc-highlight-function-argument', if they are to have any
 effect.
 
-Major modes should modify this variable using `add-function', for example:
-  (add-function :before-until (local \\='eldoc-documentation-function)
-                #\\='foo-mode-eldoc-function)
+Major modes should modify this variable using `add-hook', for example:
+  (add-hook \\='eldoc-documentation-functions #\\='foo-eldoc nil t)
 so that the global documentation function (i.e. the default value of the
 variable) is taken into account if the major mode specific function does not
 return any documentation.")
 
+(define-obsolete-variable-alias 'eldoc-documentation-function
+  'eldoc-documentation-functions "25.2")
+
 (defun eldoc-print-current-symbol-info ()
   ;; This is run from post-command-hook or some idle timer thing,
   ;; so we need to be careful that errors aren't ignored.
@@ -363,7 +364,19 @@ eldoc-print-current-symbol-info
              (when eldoc-last-message
                (eldoc-message nil)
                nil))
-	 (eldoc-message (funcall eldoc-documentation-function)))))
+	 (eldoc-message
+          (run-hook-with-args-until-success 'eldoc-documentation-functions)))))
+
+(defun eldoc-supported-p ()
+  "Return t if `eldoc-documentation-functions' has non-null elements."
+  (when eldoc-documentation-functions 
+    (catch :eldoc-supported
+      (mapc
+       (lambda (fun)
+         (when (not (memq fun '(nil ignore)))
+           (throw :eldoc-supported t)))
+       eldoc-documentation-functions)
+      nil)))
 
 ;; If the entire line cannot fit in the echo area, the symbol name may be
 ;; truncated or eliminated entirely from the output to make room for the

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Mark Oteiza]

On 12/06/16 at 09:23am, Noam Postavsky wrote:
> On Sun, Jun 12, 2016 at 2:12 AM, Mark Oteiza <mvoteiza@udel.edu> wrote:
> > Perhaps the main motivation for doing this is that I don't like the
> > unreadable mess I see when looking at the value of
> > eldoc-documentation-function.  For example, the value in
> > *scratch* from emacs -Q.  This also exists in other places, like lambdas
> > as defcustom values in eshell, but that's another matter.
> 
> Could we improve the display of function values instead?

Why not both?  AFAICT the current behavior in eldoc is an imitation of
what hooks do already.

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Dmitry Gutov]

On 06/12/2016 09:52 PM, Mark Oteiza wrote:

>> Could we improve the display of function values instead?
>
> Why not both?  AFAICT the current behavior in eldoc is an imitation of
> what hooks do already.

add-function is more powerful than hooks, see the different ways you can 
combine the functions.

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Mark Oteiza]

On 12/06/16 at 09:57pm, Dmitry Gutov wrote:
> On 06/12/2016 09:52 PM, Mark Oteiza wrote:
> 
> > > Could we improve the display of function values instead?
> > 
> > Why not both?  AFAICT the current behavior in eldoc is an imitation of
> > what hooks do already.
> 
> add-function is more powerful than hooks, see the different ways you can
> combine the functions.

That does not mean we should be using advice where hooks are a natural
solution.

Nothing prevents you from using add-function on a hook or a function
that is part of a hook.

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Dmitry Gutov]

On 06/12/2016 10:44 PM, Mark Oteiza wrote:

>> add-function is more powerful than hooks, see the different ways you can
>> combine the functions.
>
> That does not mean we should be using advice where hooks are a natural
> solution.

Maybe, maybe not. After introducing nadvice, it was Stefan's position 
that most hooks should be replaced with using advice, and 
eldoc-documentation-function in particular.

I don't know how I feel about "most hooks", but there doesn't seem to be 
a strong reason to reverse his choice on this one variable.

> Nothing prevents you from using add-function on a hook

I don't think that's possible.

> or a function
> that is part of a hook.

You'd have to know the names of the individual functions in advance.

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Leo Liu]

On 2016-06-12 14:52 -0400, Mark Oteiza wrote:
> AFAICT the current behavior in eldoc is an imitation of what hooks do
> already.

No. It is far more than a hook otherwise people would have designed it
as a hook in the first place.

Leo

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

It is asking for trouble to have anything in the Emacs sources
put advice onto any function, because it interferes with debugging.
If you see a call to function foo, which has advice on it, you will be
perplexed why its behavior does not match its source.

(If you wrote the advice, you won't have this problem if you remember
that you put the advice on.)

A hook is cleaner, because the call to run-hooks in the source code of
foo informs you that you should check what hook functions there are.

Thus, I ask people to take care not to install anything in the Emacs
sources which creates advice -- and to replace any existing advice
with hooks.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Mark Oteiza]

On 12/06/16 at 02:24pm, Mark Oteiza wrote:
> ---
> 
> On 12/06/16 at 10:09am, Eli Zaretskii wrote:
> > > Date: Sun, 12 Jun 2016 02:12:29 -0400
> > > From: Mark Oteiza <mvoteiza@udel.edu>
> > > 
> > > This is a draft patch changing eldoc-documentation-function into a hook
> > > variable, so instead of using add-function, one can instead use add-hook
> > > to control the behavior of eldoc.  It is backwards compatible.
> > 
> > Thanks.  Please be sure to update the ELisp manual accordingly.
> 
> <snip> changed the default value back to nil, since this isn't being
> unconditionally funcall'd anymore.

Ah, this was a mistake wrt backwards compatibility.  The default can be
changed sometime in the future, I suppose.

Also had to fix eldoc-supported-p...

> In the manual, it appears that the -functions suffix is to be used for
> abnormal hooks; however this is a normal hook, so it would seem the name
> should use the -hook suffix.

... but I'll wait for some judgement on this before sending another
patch.

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Mark Oteiza]

---
On 13/06/16 at 05:17pm, Mark Oteiza wrote:
> On 12/06/16 at 02:24pm, Mark Oteiza wrote:
> > On 12/06/16 at 10:09am, Eli Zaretskii wrote:
> > > > Date: Sun, 12 Jun 2016 02:12:29 -0400
> > > > From: Mark Oteiza <mvoteiza@udel.edu>
> > > > 
> > > > This is a draft patch changing eldoc-documentation-function into a hook
> > > > variable, so instead of using add-function, one can instead use add-hook
> > > > to control the behavior of eldoc.  It is backwards compatible.
> > > 
> > > Thanks.  Please be sure to update the ELisp manual accordingly.
> > 
> > <snip> changed the default value back to nil, since this isn't being
> > unconditionally funcall'd anymore.
> 
> Ah, this was a mistake wrt backwards compatibility.  The default can be
> changed sometime in the future, I suppose.
> 
> Also had to fix eldoc-supported-p...
> 
> > In the manual, it appears that the -functions suffix is to be used for
> > abnormal hooks; however this is a normal hook, so it would seem the name
> > should use the -hook suffix.
> 
> ... but I'll wait for some judgement on this before sending another
> patch.

Nevermind, I missed the part of the manual that says -functions is used
when a hooks result is used.

Here is the patch that I have been using.  No problems encountered.

diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi
index 7b76e6a..b5d07da 100644
--- a/doc/lispref/modes.texi
+++ b/doc/lispref/modes.texi
@@ -417,7 +417,7 @@ Major Mode Conventions
 
 @item
 The mode can specify a local value for
-@code{eldoc-documentation-function} to tell ElDoc mode how to handle
+@code{eldoc-documentation-functions} to tell ElDoc mode how to handle
 this mode.
 
 @item
diff --git a/etc/NEWS b/etc/NEWS
index e2c99a1..92bf8e4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -212,6 +212,11 @@ viewing HTML files and the like.
 breakpoint (e.g. with "f" and "o") by customizing the new option
 'edebug-sit-on-break'.
 
+** ElDoc
+
++++
+*** 'eldoc-documentation-functions' replaces 'eldoc-documentation-function'.
+
 ** eww
 
 +++
diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 6c2f869..67d0a9e 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -43,7 +43,7 @@
 
 ;; Major modes for other languages may use ElDoc by defining an
 ;; appropriate function as the buffer-local value of
-;; `eldoc-documentation-function'.
+;; `eldoc-documentation-functions'.
 
 ;;; Code:
 
@@ -80,7 +80,7 @@ eldoc-argument-case
 returns another string is acceptable.
 
 Note that this variable has no effect, unless
-`eldoc-documentation-function' handles it explicitly."
+`eldoc-documentation-functions' handle it explicitly."
   :type '(radio (function-item upcase)
 		(function-item downcase)
                 function)
@@ -103,7 +103,7 @@ eldoc-echo-area-use-multiline-p
 truncated to make more of the arglist or documentation string visible.
 
 Note that this variable has no effect, unless
-`eldoc-documentation-function' handles it explicitly."
+`eldoc-documentation-functions' handle it explicitly."
   :type '(radio (const :tag "Always" t)
                 (const :tag "Never" nil)
                 (const :tag "Yes, but truncate symbol names if it will\
@@ -113,8 +113,8 @@ eldoc-echo-area-use-multiline-p
 (defface eldoc-highlight-function-argument
   '((t (:inherit bold)))
   "Face used for the argument at point in a function's argument list.
-Note that this face has no effect unless the `eldoc-documentation-function'
-handles it explicitly."
+Note that this face has no effect unless the `eldoc-documentation-functions'
+handle it explicitly."
   :group 'eldoc)
 
 ;;; No user options below here.
@@ -186,7 +186,7 @@ eldoc-mode
   :group 'eldoc :lighter eldoc-minor-mode-string
   (setq eldoc-last-message nil)
   (cond
-   ((memq eldoc-documentation-function '(nil ignore))
+   ((not (eldoc-supported-p))
     (message "There is no ElDoc support in this buffer")
     (setq eldoc-mode nil))
    (eldoc-mode
@@ -211,7 +211,7 @@ global-eldoc-mode
 
 If Global Eldoc mode is on, `eldoc-mode' will be enabled in all
 buffers where it's applicable.  These are buffers that have modes
-that have enabled eldoc support.  See `eldoc-documentation-function'."
+that have enabled eldoc support.  See `eldoc-documentation-functions'."
   :group 'eldoc
   :global t
   :initialize 'custom-initialize-delay
@@ -236,9 +236,7 @@ eldoc-schedule-timer
 	     eldoc-idle-delay nil
 	     (lambda ()
                (when (or eldoc-mode
-                         (and global-eldoc-mode
-                              (not (memq eldoc-documentation-function
-                                         '(nil ignore)))))
+                         (and global-eldoc-mode (eldoc-supported-p)))
                  (eldoc-print-current-symbol-info))))))
 
   ;; If user has changed the idle delay, update the timer.
@@ -334,26 +332,29 @@ eldoc-display-message-no-interference-p
 
 \f
 ;;;###autoload
-(defvar eldoc-documentation-function #'ignore
-  "Function to call to return doc string.
-The function of no args should return a one-line string for displaying
-doc about a function etc. appropriate to the context around point.
-It should return nil if there's no doc appropriate for the context.
-Typically doc is returned if point is on a function-like name or in its
-arg list.
+(defvar eldoc-documentation-functions #'ignore
+  "Hook to run to obtain doc string.
+Each element of this variable should be a function of no args
+that returns a one-line string for displaying doc about a
+function etc. appropriate to the context around point.  It should
+return nil if there is no doc appropriate for the context.
+Typically, doc is returned if point is on a function-like name or
+in its arg list.
 
 The result is used as is, so the function must explicitly handle
 the variables `eldoc-argument-case' and `eldoc-echo-area-use-multiline-p',
 and the face `eldoc-highlight-function-argument', if they are to have any
 effect.
 
-Major modes should modify this variable using `add-function', for example:
-  (add-function :before-until (local \\='eldoc-documentation-function)
-                #\\='foo-mode-eldoc-function)
+Major modes should modify this variable using `add-hook', for example:
+  (add-hook \\='eldoc-documentation-functions #\\='foo-eldoc nil t)
 so that the global documentation function (i.e. the default value of the
 variable) is taken into account if the major mode specific function does not
 return any documentation.")
 
+(define-obsolete-variable-alias 'eldoc-documentation-function
+  'eldoc-documentation-functions "25.2")
+
 (defun eldoc-print-current-symbol-info ()
   ;; This is run from post-command-hook or some idle timer thing,
   ;; so we need to be careful that errors aren't ignored.
@@ -363,7 +364,20 @@ eldoc-print-current-symbol-info
              (when eldoc-last-message
                (eldoc-message nil)
                nil))
-	 (eldoc-message (funcall eldoc-documentation-function)))))
+	 (eldoc-message
+          (run-hook-with-args-until-success 'eldoc-documentation-functions)))))
+
+(defun eldoc-supported-p ()
+  "Return t if `eldoc-documentation-functions' has non-null elements."
+  (if (listp eldoc-documentation-functions)
+      (catch :eldoc-supported
+        (mapc
+         (lambda (fun)
+           (when (not (memq fun '(nil ignore)))
+             (throw :eldoc-supported t)))
+         eldoc-documentation-functions)
+        nil)
+    (not (memq eldoc-documentation-functions '(nil ignore)))))
 
 ;; If the entire line cannot fit in the echo area, the symbol name may be
 ;; truncated or eliminated entirely from the output to make room for the

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Dmitry Gutov]

On 06/13/2016 11:36 PM, Richard Stallman wrote:

> Thus, I ask people to take care not to install anything in the Emacs
> sources which creates advice -- and to replace any existing advice
> with hooks.

In this case, we're not really talking about advice. 
eldoc-documentation-function is modified using add-function, which makes 
the variable point to a function that's the result of a combination of 
the previous value of this variable with another function.

No function's definition is being changed this way.

Re: [PATCH] RFC: eldoc-documentation-functions hook

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > In this case, we're not really talking about advice. 
  > eldoc-documentation-function is modified using add-function, which makes 
  > the variable point to a function that's the result of a combination of 
  > the previous value of this variable with another function.

I stand corrected.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Mark Oteiza]

On 17/06/16 at 05:08pm, Mark Oteiza wrote:
> ---
> On 13/06/16 at 05:17pm, Mark Oteiza wrote:
> > On 12/06/16 at 02:24pm, Mark Oteiza wrote:
> > > On 12/06/16 at 10:09am, Eli Zaretskii wrote:
> > > > > Date: Sun, 12 Jun 2016 02:12:29 -0400
> > > > > From: Mark Oteiza <mvoteiza@udel.edu>
> > > > > 
> > > > > This is a draft patch changing eldoc-documentation-function into a hook
> > > > > variable, so instead of using add-function, one can instead use add-hook
> > > > > to control the behavior of eldoc.  It is backwards compatible.
> > > > 
> > > > Thanks.  Please be sure to update the ELisp manual accordingly.
> > > 
> > > <snip> changed the default value back to nil, since this isn't being
> > > unconditionally funcall'd anymore.
> > 
> > Ah, this was a mistake wrt backwards compatibility.  The default can be
> > changed sometime in the future, I suppose.
> > 
> > Also had to fix eldoc-supported-p...
> > 
> > > In the manual, it appears that the -functions suffix is to be used for
> > > abnormal hooks; however this is a normal hook, so it would seem the name
> > > should use the -hook suffix.
> > 
> > ... but I'll wait for some judgement on this before sending another
> > patch.
> 
> Nevermind, I missed the part of the manual that says -functions is used
> when a hooks result is used.
> 
> Here is the patch that I have been using.  No problems encountered.
> 
> diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi
> index 7b76e6a..b5d07da 100644
> --- a/doc/lispref/modes.texi
> +++ b/doc/lispref/modes.texi
> @@ -417,7 +417,7 @@ Major Mode Conventions
>  
>  @item
>  The mode can specify a local value for
> -@code{eldoc-documentation-function} to tell ElDoc mode how to handle
> +@code{eldoc-documentation-functions} to tell ElDoc mode how to handle
>  this mode.
>  
>  @item
> diff --git a/etc/NEWS b/etc/NEWS
> index e2c99a1..92bf8e4 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -212,6 +212,11 @@ viewing HTML files and the like.
>  breakpoint (e.g. with "f" and "o") by customizing the new option
>  'edebug-sit-on-break'.
>  
> +** ElDoc
> +
> ++++
> +*** 'eldoc-documentation-functions' replaces 'eldoc-documentation-function'.
> +
>  ** eww
>  
>  +++
> diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
> index 6c2f869..67d0a9e 100644
> --- a/lisp/emacs-lisp/eldoc.el
> +++ b/lisp/emacs-lisp/eldoc.el
> @@ -43,7 +43,7 @@
>  
>  ;; Major modes for other languages may use ElDoc by defining an
>  ;; appropriate function as the buffer-local value of
> -;; `eldoc-documentation-function'.
> +;; `eldoc-documentation-functions'.
>  
>  ;;; Code:
>  
> @@ -80,7 +80,7 @@ eldoc-argument-case
>  returns another string is acceptable.
>  
>  Note that this variable has no effect, unless
> -`eldoc-documentation-function' handles it explicitly."
> +`eldoc-documentation-functions' handle it explicitly."
>    :type '(radio (function-item upcase)
>  		(function-item downcase)
>                  function)
> @@ -103,7 +103,7 @@ eldoc-echo-area-use-multiline-p
>  truncated to make more of the arglist or documentation string visible.
>  
>  Note that this variable has no effect, unless
> -`eldoc-documentation-function' handles it explicitly."
> +`eldoc-documentation-functions' handle it explicitly."
>    :type '(radio (const :tag "Always" t)
>                  (const :tag "Never" nil)
>                  (const :tag "Yes, but truncate symbol names if it will\
> @@ -113,8 +113,8 @@ eldoc-echo-area-use-multiline-p
>  (defface eldoc-highlight-function-argument
>    '((t (:inherit bold)))
>    "Face used for the argument at point in a function's argument list.
> -Note that this face has no effect unless the `eldoc-documentation-function'
> -handles it explicitly."
> +Note that this face has no effect unless the `eldoc-documentation-functions'
> +handle it explicitly."
>    :group 'eldoc)
>  
>  ;;; No user options below here.
> @@ -186,7 +186,7 @@ eldoc-mode
>    :group 'eldoc :lighter eldoc-minor-mode-string
>    (setq eldoc-last-message nil)
>    (cond
> -   ((memq eldoc-documentation-function '(nil ignore))
> +   ((not (eldoc-supported-p))
>      (message "There is no ElDoc support in this buffer")
>      (setq eldoc-mode nil))
>     (eldoc-mode
> @@ -211,7 +211,7 @@ global-eldoc-mode
>  
>  If Global Eldoc mode is on, `eldoc-mode' will be enabled in all
>  buffers where it's applicable.  These are buffers that have modes
> -that have enabled eldoc support.  See `eldoc-documentation-function'."
> +that have enabled eldoc support.  See `eldoc-documentation-functions'."
>    :group 'eldoc
>    :global t
>    :initialize 'custom-initialize-delay
> @@ -236,9 +236,7 @@ eldoc-schedule-timer
>  	     eldoc-idle-delay nil
>  	     (lambda ()
>                 (when (or eldoc-mode
> -                         (and global-eldoc-mode
> -                              (not (memq eldoc-documentation-function
> -                                         '(nil ignore)))))
> +                         (and global-eldoc-mode (eldoc-supported-p)))
>                   (eldoc-print-current-symbol-info))))))
>  
>    ;; If user has changed the idle delay, update the timer.
> @@ -334,26 +332,29 @@ eldoc-display-message-no-interference-p
>  
>  \f
>  ;;;###autoload
> -(defvar eldoc-documentation-function #'ignore
> -  "Function to call to return doc string.
> -The function of no args should return a one-line string for displaying
> -doc about a function etc. appropriate to the context around point.
> -It should return nil if there's no doc appropriate for the context.
> -Typically doc is returned if point is on a function-like name or in its
> -arg list.
> +(defvar eldoc-documentation-functions #'ignore
> +  "Hook to run to obtain doc string.
> +Each element of this variable should be a function of no args
> +that returns a one-line string for displaying doc about a
> +function etc. appropriate to the context around point.  It should
> +return nil if there is no doc appropriate for the context.
> +Typically, doc is returned if point is on a function-like name or
> +in its arg list.
>  
>  The result is used as is, so the function must explicitly handle
>  the variables `eldoc-argument-case' and `eldoc-echo-area-use-multiline-p',
>  and the face `eldoc-highlight-function-argument', if they are to have any
>  effect.
>  
> -Major modes should modify this variable using `add-function', for example:
> -  (add-function :before-until (local \\='eldoc-documentation-function)
> -                #\\='foo-mode-eldoc-function)
> +Major modes should modify this variable using `add-hook', for example:
> +  (add-hook \\='eldoc-documentation-functions #\\='foo-eldoc nil t)
>  so that the global documentation function (i.e. the default value of the
>  variable) is taken into account if the major mode specific function does not
>  return any documentation.")
>  
> +(define-obsolete-variable-alias 'eldoc-documentation-function
> +  'eldoc-documentation-functions "25.2")
> +
>  (defun eldoc-print-current-symbol-info ()
>    ;; This is run from post-command-hook or some idle timer thing,
>    ;; so we need to be careful that errors aren't ignored.
> @@ -363,7 +364,20 @@ eldoc-print-current-symbol-info
>               (when eldoc-last-message
>                 (eldoc-message nil)
>                 nil))
> -	 (eldoc-message (funcall eldoc-documentation-function)))))
> +	 (eldoc-message
> +          (run-hook-with-args-until-success 'eldoc-documentation-functions)))))
> +
> +(defun eldoc-supported-p ()
> +  "Return t if `eldoc-documentation-functions' has non-null elements."
> +  (if (listp eldoc-documentation-functions)
> +      (catch :eldoc-supported
> +        (mapc
> +         (lambda (fun)
> +           (when (not (memq fun '(nil ignore)))
> +             (throw :eldoc-supported t)))
> +         eldoc-documentation-functions)
> +        nil)
> +    (not (memq eldoc-documentation-functions '(nil ignore)))))
>  
>  ;; If the entire line cannot fit in the echo area, the symbol name may be
>  ;; truncated or eliminated entirely from the output to make room for the

Applied with some wording changes as 5811404

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Leo Liu]

On 2016-07-06 23:30 -0400, Mark Oteiza wrote:
> Applied with some wording changes as 5811404

I don't think we have reached any consensus. There are legitimate
concerns that this introduces incompatibility. Please give better
explanation why reverting a decision made by ex-maintainer.

Cheers,
Leo

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

I'm neither for or against this change. But if this change is done in
eldoc.el, then shouldn't the add-function to add-hook change be done in all
the major modes too? Like in elisp-mode.el, etc.

On Thu, Jul 7, 2016, 12:12 AM Leo Liu <sdl.web@gmail.com> wrote:

> On 2016-07-06 23:30 -0400, Mark Oteiza wrote:
> > Applied with some wording changes as 5811404
>
> I don't think we have reached any consensus. There are legitimate
> concerns that this introduces incompatibility. Please give better
> explanation why reverting a decision made by ex-maintainer.
>
> Cheers,
> Leo
>
>
> --

-- 
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 969 bytes --]

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 308 bytes --]

Hi Mark,

On 2016-07-06 23:30, Mark Oteiza wrote:
> Applied with some wording changes as 5811404

Can you clarify why a hook is better than a function? Also, what's the expected transition path? Will all major modes that use eldoc-documentation-function need to be updated at some point?

Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Noam Postavsky]

On Thu, Jul 7, 2016 at 6:02 AM, Kaushal Modi <kaushal.modi@gmail.com> wrote:
> I'm neither for or against this change. But if this change is done in
> eldoc.el, then shouldn't the add-function to add-hook change be done in all
> the major modes too? Like in elisp-mode.el, etc.
>

Ping! I just bumped into this elisp-mode.el case while playing with
the bootstrap. Please revert this, or fix callers, thanks.

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Mark Oteiza]

[-- Attachment #1: Type: text/plain, Size: 492 bytes --]

On 17/07/16 at 11:17am, Noam Postavsky wrote:
> On Thu, Jul 7, 2016 at 6:02 AM, Kaushal Modi <kaushal.modi@gmail.com> wrote:
> > I'm neither for or against this change. But if this change is done in
> > eldoc.el, then shouldn't the add-function to add-hook change be done in all
> > the major modes too? Like in elisp-mode.el, etc.
> >
> 
> Ping! I just bumped into this elisp-mode.el case while playing with
> the bootstrap. Please revert this, or fix callers, thanks.

Pushed as 001d88b:



[-- Attachment #2: 0001-Use-eldoc-documentation-functions.patch --]
[-- Type: text/x-diff, Size: 7017 bytes --]

From 001d88b62ecb8163a148656acb103b354ce7613a Mon Sep 17 00:00:00 2001
From: Mark Oteiza <mvoteiza@udel.edu>
Date: Sun, 17 Jul 2016 12:49:57 -0400
Subject: [PATCH] Use eldoc-documentation-functions

* lisp/hexl.el (hexl-mode):
* lisp/ielm.el (inferior-emacs-lisp-mode):
* lisp/progmodes/cfengine.el (cfengine3-mode):
* lisp/progmodes/elisp-mode.el (emacs-lisp-mode):
* lisp/progmodes/octave.el (octave-mode, inferior-octave-mode):
* lisp/progmodes/python.el (python-mode):
* lisp/simple.el (read--expression): Add buffer-locally to hook
eldoc-documentation-functions.
---
 lisp/hexl.el                 |  4 ++--
 lisp/ielm.el                 |  4 ++--
 lisp/progmodes/cfengine.el   | 15 +++++++++------
 lisp/progmodes/elisp-mode.el |  4 ++--
 lisp/progmodes/octave.el     |  5 ++---
 lisp/progmodes/python.el     | 14 ++++++++------
 lisp/simple.el               |  4 ++--
 7 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/lisp/hexl.el b/lisp/hexl.el
index 5f099a5..61d7dd0 100644
--- a/lisp/hexl.el
+++ b/lisp/hexl.el
@@ -395,8 +395,8 @@ hexl-mode
     (add-hook 'change-major-mode-hook 'hexl-maybe-dehexlify-buffer nil t)
 
     ;; Set a callback function for eldoc.
-    (add-function :before-until (local 'eldoc-documentation-function)
-                  #'hexl-print-current-point-info)
+    (add-hook 'eldoc-documentation-functions
+              #'hexl-print-current-point-info nil t)
     (eldoc-add-command-completions "hexl-")
     (eldoc-remove-command "hexl-save-buffer"
 			  "hexl-current-address")
diff --git a/lisp/ielm.el b/lisp/ielm.el
index dd02778..278a637 100644
--- a/lisp/ielm.el
+++ b/lisp/ielm.el
@@ -541,8 +541,8 @@ inferior-emacs-lisp-mode
   (set (make-local-variable 'completion-at-point-functions)
        '(comint-replace-by-expanded-history
          ielm-complete-filename elisp-completion-at-point))
-  (add-function :before-until (local 'eldoc-documentation-function)
-                #'elisp-eldoc-documentation-function)
+  (add-hook 'eldoc-documentation-functions
+            #'elisp-eldoc-documentation-function nil t)
   (set (make-local-variable 'ielm-prompt-internal) ielm-prompt)
   (set (make-local-variable 'comint-prompt-read-only) ielm-prompt-read-only)
   (setq comint-get-old-input 'ielm-get-old-input)
diff --git a/lisp/progmodes/cfengine.el b/lisp/progmodes/cfengine.el
index 0830214..ace012f 100644
--- a/lisp/progmodes/cfengine.el
+++ b/lisp/progmodes/cfengine.el
@@ -1390,12 +1390,15 @@ cfengine3-mode
                  (when buffer-file-name
                    (shell-quote-argument buffer-file-name)))))
 
-  ;; For emacs < 25.1 where `eldoc-documentation-function' defaults to
-  ;; nil.
-  (or eldoc-documentation-function
-      (setq-local eldoc-documentation-function #'ignore))
-  (add-function :before-until (local 'eldoc-documentation-function)
-                #'cfengine3-documentation-function)
+  (if (boundp 'eldoc-documentation-functions)
+      (add-hook 'eldoc-documentation-functions
+                #'cfengine3-documentation-function nil t)
+    ;; For emacs < 25.1 where `eldoc-documentation-function' defaults to
+    ;; nil.
+    (or eldoc-documentation-function
+        (setq-local eldoc-documentation-function #'ignore))
+    (add-function :before-until (local 'eldoc-documentation-function)
+                  #'cfengine3-documentation-function))
 
   (add-hook 'completion-at-point-functions
             #'cfengine3-completion-function nil t)
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index f360791..5f9bdac 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -235,8 +235,8 @@ emacs-lisp-mode
               (append '((?\` . ?\') (?‘ . ?’)) electric-pair-text-pairs))
   (setq-local electric-quote-string t)
   (setq imenu-case-fold-search nil)
-  (add-function :before-until (local 'eldoc-documentation-function)
-                #'elisp-eldoc-documentation-function)
+  (add-hook 'eldoc-documentation-functions
+            #'elisp-eldoc-documentation-function nil t)
   (add-hook 'xref-backend-functions #'elisp--xref-backend nil t)
   (setq-local project-vc-external-roots-function #'elisp-load-path-roots)
   (add-hook 'completion-at-point-functions
diff --git a/lisp/progmodes/octave.el b/lisp/progmodes/octave.el
index 4f223f2..b9a86e7 100644
--- a/lisp/progmodes/octave.el
+++ b/lisp/progmodes/octave.el
@@ -596,8 +596,7 @@ octave-mode
   (add-hook 'before-save-hook 'octave-sync-function-file-names nil t)
   (setq-local beginning-of-defun-function 'octave-beginning-of-defun)
   (and octave-font-lock-texinfo-comment (octave-font-lock-texinfo-comment))
-  (add-function :before-until (local 'eldoc-documentation-function)
-                'octave-eldoc-function)
+  (add-hook 'eldoc-documentation-functions 'octave-eldoc-function nil t)
 
   (easy-menu-add octave-mode-menu))
 
@@ -733,7 +732,7 @@ inferior-octave-mode
   (setq font-lock-defaults '(inferior-octave-font-lock-keywords nil nil))
 
   (setq-local info-lookup-mode 'octave-mode)
-  (setq-local eldoc-documentation-function 'octave-eldoc-function)
+  (add-hook 'eldoc-documentation-functions 'octave-eldoc-function nil t)
 
   (setq-local comint-input-ring-file-name
               (or (getenv "OCTAVE_HISTFILE") "~/.octave_hist"))
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index ad69f87..ba3cdfe 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -5153,12 +5153,14 @@ python-mode
                                                  (current-column))))
          (^ '(- (1+ (current-indentation))))))
 
-  (if (null eldoc-documentation-function)
-      ;; Emacs<25
-      (set (make-local-variable 'eldoc-documentation-function)
-           #'python-eldoc-function)
-    (add-function :before-until (local 'eldoc-documentation-function)
-                  #'python-eldoc-function))
+  (if (boundp 'eldoc-documentation-functions)
+      (add-hook 'eldoc-documentation-functions #'python-eldoc-function nil t)
+    (if (null eldoc-documentation-function)
+        ;; Emacs<25
+        (set (make-local-variable 'eldoc-documentation-function)
+             #'python-eldoc-function)
+      (add-function :before-until (local 'eldoc-documentation-function)
+                    #'python-eldoc-function)))
 
   (add-to-list
    'hs-special-modes-alist
diff --git a/lisp/simple.el b/lisp/simple.el
index a757876..06d1b92 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1443,8 +1443,8 @@ read--expression
     (minibuffer-with-setup-hook
         (lambda ()
           ;; FIXME: call emacs-lisp-mode?
-          (add-function :before-until (local 'eldoc-documentation-function)
-                        #'elisp-eldoc-documentation-function)
+          (add-hook 'eldoc-documentation-functions
+                    #'elisp-eldoc-documentation-function nil t)
           (add-hook 'completion-at-point-functions
                     #'elisp-completion-at-point nil t)
           (run-hooks 'eval-expression-minibuffer-setup-hook))
-- 
2.9.0

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Stefan Monnier]

>> Applied with some wording changes as 5811404
> I don't think we have reached any consensus.

The problem is not just that it introduces a gratuitous incompatiblity,
but that it's a regression since you can't use things like :around nor
choose precedence (as in add-function's `depth') with add-hook.


        Stefan

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Stefan Monnier]

>>> Applied with some wording changes as 5811404
>> I don't think we have reached any consensus.
> The problem is not just that it introduces a gratuitous incompatiblity,
> but that it's a regression since you can't use things like :around nor
> choose precedence (as in add-function's `depth') with add-hook.

I know of 3 motivations to replace foo-function with foo-functions:

- habit and consistency: Emacs has used add-hook for many many years, so
  having to start using add-function is inconvenient.  That is true and
  I don't have a good argument against this, except that foo-function
  also have existed for many years so the fact that you can't use
  add-hook on them is not really new.  What is new is that you can use
  add-function on them.

- C-h v foo-function RET gives a value that's unreadable.   That is true
  and we should improve it.  I don't think there's anything really hard
  about doing so, so it's a transient motivation and it'd be better to
  fix `C-h v' than to circumvent the problem by using foo-functions.

- (add-function :before (local 'foo-function) #'toto) is more verbose
  than (add-hook 'foo-functions #'toto nil t).  That's true.  But the
  difference is not very large.  We could try and reduce it, but I'm not
  sure it's worth the trouble, especially since the fact that you can
  choose between (say) :before and :around is one of the main benefits of
  foo-function over foo-functions.


-- Stefan

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Dmitry Gutov]

On 07/17/2016 08:48 PM, Mark Oteiza wrote:

>> Ping! I just bumped into this elisp-mode.el case while playing with
>> the bootstrap. Please revert this, or fix callers, thanks.
>
> Pushed as 001d88b:

Both commits are now reverted.

You're welcome, everyone.

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Leo Liu]

On 2016-07-18 02:47 +0300, Dmitry Gutov wrote:
> Both commits are now reverted.
>
> You're welcome, everyone.

👍 Seems the only sensible thing to do before the situation gets worse.

Leo

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Mark Oteiza]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>> Applied with some wording changes as 5811404
>>> I don't think we have reached any consensus.
>> The problem is not just that it introduces a gratuitous incompatiblity,
>> but that it's a regression since you can't use things like :around nor
>> choose precedence (as in add-function's `depth') with add-hook.

There was never a need.  The state of eldoc shows this, as it is exactly
an emulation of what run-hook-with-args-until-success does.  Alas, I'm
repeating myself.

> I know of 3 motivations to replace foo-function with foo-functions:
>
> - habit and consistency: Emacs has used add-hook for many many years, so
>   having to start using add-function is inconvenient.  That is true and
>   I don't have a good argument against this, except that foo-function
>   also have existed for many years so the fact that you can't use
>   add-hook on them is not really new.  What is new is that you can use
>   add-function on them.

Usually foo-function holds a function symbol.  If one had a desire to
add-hook on foo-function whose value is #'bar, then perhaps bar should
run a hook; but then perhaps foo-function is just a layer of indirection
and you really should just have a hook.

> - C-h v foo-function RET gives a value that's unreadable.   That is true
>   and we should improve it.  I don't think there's anything really hard
>   about doing so, so it's a transient motivation and it'd be better to
>   fix `C-h v' than to circumvent the problem by using foo-functions.

Yes, we should not have to read bytecode or (at best) RTFS to decipher
what foo-function is doing.

> - (add-function :before (local 'foo-function) #'toto) is more verbose
>   than (add-hook 'foo-functions #'toto nil t).  That's true.  But the
>   difference is not very large.  We could try and reduce it, but I'm not
>   sure it's worth the trouble, especially since the fact that you can
>   choose between (say) :before and :around is one of the main benefits of
>   foo-function over foo-functions.

Which is great if that flexibility is even necessary.  Advice is useful,
no doubt; however, IME the only place I thought advice was the best
solution was tacking onto a process filter.  As I recall, there was an
interesting discussion on process API, but I can't find it now.

The verbosity of writing advice isn't so bad; using advice even when the
circumstance doesn't call for it is.  To cite an example, is the
following somehow different from just using setq-local?
http://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/textmodes/tex-mode.el#n1262

PS: I'd have suggested a more graceful change like that of
pre-redisplay-function(s)
http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=84e0b7d
But even then you end up with two (somewhat) disjoint APIs; for
instance, no degree of precedence will put your advice between function
symbols in the hook.

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Stefan Monnier]

>>>>> Applied with some wording changes as 5811404
>>>> I don't think we have reached any consensus.
>>> The problem is not just that it introduces a gratuitous incompatiblity,
>>> but that it's a regression since you can't use things like :around nor
>>> choose precedence (as in add-function's `depth') with add-hook.
> There was never a need.

But your change removes the possibility.  Just because it hasn't been
needed yet doesn't mean it won't be needed in the future.  After all,
eldoc has not seen much use so far.

> Usually foo-function holds a function symbol.  If one had a desire to
> add-hook on foo-function whose value is #'bar, then perhaps bar should
> run a hook; but then perhaps foo-function is just a layer of indirection
> and you really should just have a hook.

foo-function *is* a hook.

>> - C-h v foo-function RET gives a value that's unreadable.   That is true
>> and we should improve it.  I don't think there's anything really hard
>> about doing so, so it's a transient motivation and it'd be better to
>> fix `C-h v' than to circumvent the problem by using foo-functions.
> Yes, we should not have to read bytecode or (at best) RTFS to decipher
> what foo-function is doing.

100% Agreement.

> Which is great if that flexibility is even necessary.

The great thing about foo-function (along with add-function) is that you
don't need to guess beforehand if it's going to be necessary.

> The verbosity of writing advice isn't so bad; using advice even when the
> circumstance doesn't call for it is.  To cite an example, is the
> following somehow different from just using setq-local?
> http://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/textmodes/tex-mode.el#n1262

Yes, the use of add-function here is overkill.  But this exact same
setting would be just right for a minor mode (where using setq-local
and kill-local-variable is painful and brittle).

> PS: I'd have suggested a more graceful change like that of
> pre-redisplay-function(s)
> http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=84e0b7d

As you can see, I'm not completely dogmatic about forcing the use
of foo-function in place of foo-functions everywhere.  In the case of
pre-redisplay-function, the function does not return any value, so
there's not much point in using things like :around, or :before-until.


        Stefan

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Mark Oteiza]

On 18/07/16 at 10:47pm, Stefan Monnier wrote:
> >>>>> Applied with some wording changes as 5811404
> >>>> I don't think we have reached any consensus.
> >>> The problem is not just that it introduces a gratuitous incompatiblity,
> >>> but that it's a regression since you can't use things like :around nor
> >>> choose precedence (as in add-function's `depth') with add-hook.
> > There was never a need.
> 
> But your change removes the possibility.  Just because it hasn't been
> needed yet doesn't mean it won't be needed in the future.  After all,
> eldoc has not seen much use so far.

The possibility is still there, just different in that one would advise
a function instead of the single-function hook.   I still can't think of
a practical use for the different WHEREs, but until such a need arises
I suppose it's of no importance anyways.

> > Usually foo-function holds a function symbol.  If one had a desire to
> > add-hook on foo-function whose value is #'bar, then perhaps bar should
> > run a hook; but then perhaps foo-function is just a layer of indirection
> > and you really should just have a hook.
> 
> foo-function *is* a hook.

Oh, meant to distinguish single function hooks from the rest
(which I just called hooks).

> >> - C-h v foo-function RET gives a value that's unreadable.   That is true
> >> and we should improve it.  I don't think there's anything really hard
> >> about doing so, so it's a transient motivation and it'd be better to
> >> fix `C-h v' than to circumvent the problem by using foo-functions.
> > Yes, we should not have to read bytecode or (at best) RTFS to decipher
> > what foo-function is doing.
> 
> 100% Agreement.
> 
> > Which is great if that flexibility is even necessary.
> 
> The great thing about foo-function (along with add-function) is that you
> don't need to guess beforehand if it's going to be necessary.
> 
> > The verbosity of writing advice isn't so bad; using advice even when the
> > circumstance doesn't call for it is.  To cite an example, is the
> > following somehow different from just using setq-local?
> > http://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/textmodes/tex-mode.el#n1262
> 
> Yes, the use of add-function here is overkill.  But this exact same
> setting would be just right for a minor mode (where using setq-local
> and kill-local-variable is painful and brittle).
> 
> > PS: I'd have suggested a more graceful change like that of
> > pre-redisplay-function(s)
> > http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=84e0b7d
> 
> As you can see, I'm not completely dogmatic about forcing the use
> of foo-function in place of foo-functions everywhere.  In the case of
> pre-redisplay-function, the function does not return any value, so
> there's not much point in using things like :around, or :before-until.

Ok, then perhaps something to the effect of

(defun run-single-function-hook (hook)
  (let ((global-hook (default-value hook))
        (local-hook (when (local-variable-p hook) (symbol-value hook))))
    (or (and (functionp local-hook) (funcall local-hook))
        (and (functionp global-hook) (funcall global-hook)))))

can instead be used.  Haven't bothered looking to see if this is useful
outside of eldoc…

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 766 bytes --]

On 2016-07-19 19:20, Mark Oteiza wrote:
> Ok, then perhaps something to the effect of
> 
> (defun run-single-function-hook (hook)
>   (let ((global-hook (default-value hook))
>         (local-hook (when (local-variable-p hook) (symbol-value hook))))
>     (or (and (functionp local-hook) (funcall local-hook))
>         (and (functionp global-hook) (funcall global-hook)))))
> 
> can instead be used.  Haven't bothered looking to see if this is useful
> outside of eldoc…

Looks nice; does it call global-hook on purpose if local-hook is a function that returns false?

Btw, (I hope this isn't a silly suggestion) is there any way to extend add-hook and run-hook* to work with both traditional "list of functions" hooks and single-function hooks?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[John Wiegley]

>>>>> Mark Oteiza <mvoteiza@udel.edu> writes:

> Ok, then perhaps something to the effect of
> (defun run-single-function-hook (hook)
>   (let ((global-hook (default-value hook))
>         (local-hook (when (local-variable-p hook) (symbol-value hook))))
>     (or (and (functionp local-hook) (funcall local-hook))
>         (and (functionp global-hook) (funcall global-hook)))))

> can instead be used. Haven't bothered looking to see if this is useful
> outside of eldoc…

Hi Mark,

I'd like to steer this discussion away from -function vs. -functions, to get
at the underlying concern you've expressed. Is it that the use of nadvice in
cases like these seems heavyweight? I'm wonder if
eldoc-documentation-functions has become something of a red herring, when
really the point you were making is more fundamental.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [PATCH v3] RFC: eldoc-documentation-functions hook

[Mark Oteiza]

On 19/07/16 at 09:50pm, John Wiegley wrote:
> >>>>> Mark Oteiza <mvoteiza@udel.edu> writes:
> > Ok, then perhaps something to the effect of
> > (defun run-single-function-hook (hook)
> >   (let ((global-hook (default-value hook))
> >         (local-hook (when (local-variable-p hook) (symbol-value hook))))
> >     (or (and (functionp local-hook) (funcall local-hook))
> >         (and (functionp global-hook) (funcall global-hook)))))
> 
> > can instead be used. Haven't bothered looking to see if this is useful
> > outside of eldoc…
> 
>  Is it that the use of nadvice in
> cases like these seems heavyweight? 

Yes.

(Though, only now do I realize that what I have written above was
written a while ago in Bug#19338)