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 <address@hidden>
> > > > > 
> > > > > 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
> address@hidden to tell ElDoc mode how to handle
> address@hidden 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
>  
>  
>  ;;;###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