unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#27230: eldoc doc
@ 2017-06-04 10:38 Charles A. Roelli
  2017-06-05 22:08 ` Dmitry Gutov
  0 siblings, 1 reply; 22+ messages in thread
From: Charles A. Roelli @ 2017-06-04 10:38 UTC (permalink / raw)
  To: 27230

 From the top of eldoc.el:

;; One useful way to enable this minor mode is to put the following in your
;; .emacs:
;;
;;      (add-hook 'emacs-lisp-mode-hook 'eldoc-mode)
;;      (add-hook 'lisp-interaction-mode-hook 'eldoc-mode)
;;      (add-hook 'ielm-mode-hook 'eldoc-mode)
;;      (add-hook 'eval-expression-minibuffer-setup-hook 'eldoc-mode)

In 25.2, none of these lines are needed.  Maybe
these lines could be replaced with a mention of global-eldoc-mode,
and how the "globalized" minor mode and eldoc-mode interact (not sure
how they do).

Also, most of the functions in eldoc have no docstring (seems ironic).
Would there be any interest in a patch to fix that?






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

* bug#27230: eldoc doc
  2017-06-04 10:38 bug#27230: eldoc doc Charles A. Roelli
@ 2017-06-05 22:08 ` Dmitry Gutov
  2017-06-06 18:33   ` Charles A. Roelli
  0 siblings, 1 reply; 22+ messages in thread
From: Dmitry Gutov @ 2017-06-05 22:08 UTC (permalink / raw)
  To: Charles A. Roelli, 27230

On 6/4/17 1:38 PM, Charles A. Roelli wrote:
>  From the top of eldoc.el:
> 
> ;; One useful way to enable this minor mode is to put the following in your
> ;; .emacs:
> ;;
> ;;      (add-hook 'emacs-lisp-mode-hook 'eldoc-mode)
> ;;      (add-hook 'lisp-interaction-mode-hook 'eldoc-mode)
> ;;      (add-hook 'ielm-mode-hook 'eldoc-mode)
> ;;      (add-hook 'eval-expression-minibuffer-setup-hook 'eldoc-mode)
> 
> In 25.2, none of these lines are needed.  Maybe
> these lines could be replaced with a mention of global-eldoc-mode,
> and how the "globalized" minor mode and eldoc-mode interact (not sure
> how they do).

Do you mean that how define-globalized-minor-mode works is unclear to 
you, or some aspects of how it applies to eldoc-mode?

> Also, most of the functions in eldoc have no docstring (seems ironic).
> Would there be any interest in a patch to fix that?

Sure!





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

* bug#27230: eldoc doc
  2017-06-05 22:08 ` Dmitry Gutov
@ 2017-06-06 18:33   ` Charles A. Roelli
  2017-06-06 20:19     ` Dmitry Gutov
  0 siblings, 1 reply; 22+ messages in thread
From: Charles A. Roelli @ 2017-06-06 18:33 UTC (permalink / raw)
  To: Dmitry Gutov, 27230

I'm confused about how the command `define-globalized-minor-mode'
defines will handle buffers that already have the minor mode turned
on.  Say buffers A and B have simple `eldoc-mode' switched on, and
buffers C and D don't (and global-eldoc-mode is off).  If I then
switch global-eldoc-mode on, is every buffer's value of eldoc-mode
now /on/, or do the values get toggled instead (leaving A and B off, C
and D on)?  And after that, if I toggle global-eldoc-mode off again,
are the previous values remembered and restored, or does every buffer
now have eldoc-mode switched off?


Thanks for your help with this.  I've probably missed a paragraph in the 
docs somewhere.


On 06/06/2017 00:08, Dmitry Gutov wrote:
> On 6/4/17 1:38 PM, Charles A. Roelli wrote:
>>  From the top of eldoc.el:
>>
>> ;; One useful way to enable this minor mode is to put the following 
>> in your
>> ;; .emacs:
>> ;;
>> ;;      (add-hook 'emacs-lisp-mode-hook 'eldoc-mode)
>> ;;      (add-hook 'lisp-interaction-mode-hook 'eldoc-mode)
>> ;;      (add-hook 'ielm-mode-hook 'eldoc-mode)
>> ;;      (add-hook 'eval-expression-minibuffer-setup-hook 'eldoc-mode)
>>
>> In 25.2, none of these lines are needed.  Maybe
>> these lines could be replaced with a mention of global-eldoc-mode,
>> and how the "globalized" minor mode and eldoc-mode interact (not sure
>> how they do).
>
> Do you mean that how define-globalized-minor-mode works is unclear to 
> you, or some aspects of how it applies to eldoc-mode?
>
>> Also, most of the functions in eldoc have no docstring (seems ironic).
>> Would there be any interest in a patch to fix that?
>
> Sure!






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

* bug#27230: eldoc doc
  2017-06-06 18:33   ` Charles A. Roelli
@ 2017-06-06 20:19     ` Dmitry Gutov
  2017-06-25  9:14       ` Charles A. Roelli
  0 siblings, 1 reply; 22+ messages in thread
From: Dmitry Gutov @ 2017-06-06 20:19 UTC (permalink / raw)
  To: Charles A. Roelli, 27230

On 6/6/17 9:33 PM, Charles A. Roelli wrote:
> I'm confused about how the command `define-globalized-minor-mode'
> defines will handle buffers that already have the minor mode turned
> on.

Indeed, the docstring is a bit ambiguous.

Maybe you want to improve the documentation of the said function, or the 
auto-generated docstring that it puts on the created minor modes.

> Say buffers A and B have simple `eldoc-mode' switched on, and
> buffers C and D don't (and global-eldoc-mode is off).  If I then
> switch global-eldoc-mode on, is every buffer's value of eldoc-mode
> now /on/, or do the values get toggled instead (leaving A and B off, C
> and D on)?

Not toggled, of course. On everywhere (where appropriate).

> And after that, if I toggle global-eldoc-mode off again,
> are the previous values remembered and restored, or does every buffer
> now have eldoc-mode switched off?

Not remembered, no. Off everywhere.





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

* bug#27230: eldoc doc
  2017-06-06 20:19     ` Dmitry Gutov
@ 2017-06-25  9:14       ` Charles A. Roelli
  2017-06-25 14:26         ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Charles A. Roelli @ 2017-06-25  9:14 UTC (permalink / raw)
  To: Dmitry Gutov, 27230

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

Here's a doc patch for ElDoc, with some minor readability fixes.

I'm unsure how to improve the doc for globalized minor modes, so I'll
leave that for another time.


On 06/06/2017 22:19, Dmitry Gutov wrote:
> On 6/6/17 9:33 PM, Charles A. Roelli wrote:
>> I'm confused about how the command `define-globalized-minor-mode'
>> defines will handle buffers that already have the minor mode turned
>> on.
>
> Indeed, the docstring is a bit ambiguous.
>
> Maybe you want to improve the documentation of the said function, or 
> the auto-generated docstring that it puts on the created minor modes.
>
>> Say buffers A and B have simple `eldoc-mode' switched on, and
>> buffers C and D don't (and global-eldoc-mode is off).  If I then
>> switch global-eldoc-mode on, is every buffer's value of eldoc-mode
>> now /on/, or do the values get toggled instead (leaving A and B off, C
>> and D on)?
>
> Not toggled, of course. On everywhere (where appropriate).
>
>> And after that, if I toggle global-eldoc-mode off again,
>> are the previous values remembered and restored, or does every buffer
>> now have eldoc-mode switched off?
>
> Not remembered, no. Off everywhere.


[-- Attachment #2: 0001-ElDoc-add-docstrings-and-minor-refactoring.patch --]
[-- Type: text/x-patch, Size: 8140 bytes --]

From 744b20db5524b32302e9cd90cb71dcaec580430e Mon Sep 17 00:00:00 2001
From: Charles A. Roelli <charles@aurox.ch>
Date: Thu, 22 Jun 2017 21:04:09 +0200
Subject: [PATCH] ElDoc: add docstrings and minor refactoring

* lisp/emacs-lisp/eldoc.el (eldoc-edit-message-commands): Add
docstring.
(turn-on-eldoc-mode): Fix capitalization.
(eldoc--supported-p): Add docstring.
(eldoc-schedule-timer): Add docstring and use
'eldoc--supported-p'.
(eldoc-message): Add docstring and make calling convention
clearer.
(eldoc--message-command-p):
(eldoc-pre-command-refresh-echo-area):
(eldoc-display-message-p):
(eldoc-display-message-no-interference-p):
(eldoc-print-current-symbol-info):
(eldoc-docstring-format-sym-doc):
(eldoc-add-command, eldoc-add-command-completions):
(eldoc-remove-command, eldoc-remove-command-completions):
Add docstring.
---
 lisp/emacs-lisp/eldoc.el |   53 ++++++++++++++++++++++++++++++++++++---------
 1 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index a05bd7c..2d8e029 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -160,6 +160,10 @@ eldoc-message-function
 It should receive the same arguments as `message'.")
 
 (defun eldoc-edit-message-commands ()
+  "Return an obarray containing common editing commands.
+
+When `eldoc-print-after-edit' is non-nil, ElDoc messages are only
+printed after commands contained in this obarray."
   (let ((cmds (make-vector 31 0))
 	(re (regexp-opt '("delete" "insert" "edit" "electric" "newline"))))
     (mapatoms (lambda (s)
@@ -211,16 +215,21 @@ global-eldoc-mode
 
 ;;;###autoload
 (defun turn-on-eldoc-mode ()
-  "Turn on `eldoc-mode' if the buffer has eldoc support enabled.
+  "Turn on `eldoc-mode' if the buffer has ElDoc support enabled.
 See `eldoc-documentation-function' for more detail."
   (when (eldoc--supported-p)
     (eldoc-mode 1)))
 
 (defun eldoc--supported-p ()
+  "Non-nil if an ElDoc function is set for this buffer."
   (not (memq eldoc-documentation-function '(nil ignore))))
 
 \f
 (defun eldoc-schedule-timer ()
+  "Ensure `eldoc-timer' is running.
+
+If the user has changed `eldoc-idle-delay', update the timer to
+reflect the change."
   (or (and eldoc-timer
            (memq eldoc-timer timer-idle-list)) ;FIXME: Why?
       (setq eldoc-timer
@@ -229,8 +238,7 @@ eldoc-schedule-timer
 	     (lambda ()
                (when (or eldoc-mode
                          (and global-eldoc-mode
-                              (not (memq eldoc-documentation-function
-                                         '(nil ignore)))))
+                              (eldoc--supported-p)))
                  (eldoc-print-current-symbol-info))))))
 
   ;; If user has changed the idle delay, update the timer.
@@ -268,16 +276,23 @@ eldoc-minibuffer-message
           (force-mode-line-update)))
     (apply 'message format-string args)))
 
-(defun eldoc-message (&rest args)
+(defun eldoc-message (&optional format-string &rest args)
+  "Store and display the given message.
+
+FORMAT-STRING and ARGS, if given, are passed to `format-message',
+the output of which is stored in `eldoc-last-message'.
+
+`eldoc-last-message' is then displayed (using
+`eldoc-message-function') and returned."
   (let ((omessage eldoc-last-message))
     (setq eldoc-last-message
-	  (cond ((eq (car args) eldoc-last-message) eldoc-last-message)
-		((null (car args)) nil)
+	  (cond ((eq format-string eldoc-last-message) eldoc-last-message)
+		((null format-string) nil)
 		;; If only one arg, no formatting to do, so put it in
 		;; eldoc-last-message so eq test above might succeed on
 		;; subsequent calls.
-		((null (cdr args)) (car args))
-		(t (apply #'format-message args))))
+		((null args) format-string)
+		(t (apply #'format-message format-string args))))
     ;; In emacs 19.29 and later, and XEmacs 19.13 and later, all messages
     ;; are recorded in a log.  Do not put eldoc messages in that log since
     ;; they are Legion.
@@ -289,6 +304,7 @@ eldoc-message
   eldoc-last-message)
 
 (defun eldoc--message-command-p (command)
+  "Non-nil if COMMAND is a command in `eldoc-message-commands'."
   (and (symbolp command)
        (intern-soft (symbol-name command) eldoc-message-commands)))
 
@@ -299,6 +315,7 @@ eldoc--message-command-p
 ;; before the next command executes, which does away with the flicker.
 ;; This doesn't seem to be required for Emacs 19.28 and earlier.
 (defun eldoc-pre-command-refresh-echo-area ()
+  "Reprint `eldoc-last-message' to the echo area."
   (and eldoc-last-message
        (not (minibufferp))      ;We don't use the echo area when in minibuffer.
        (if (and (eldoc-display-message-no-interference-p)
@@ -310,6 +327,7 @@ eldoc-pre-command-refresh-echo-area
 
 ;; Decide whether now is a good time to display a message.
 (defun eldoc-display-message-p ()
+  "Non-nil when appropriate to display an ElDoc message."
   (and (eldoc-display-message-no-interference-p)
        ;; If this-command is non-nil while running via an idle
        ;; timer, we're still in the middle of executing a command,
@@ -322,6 +340,8 @@ eldoc-display-message-p
 ;; Check various conditions about the current environment that might make
 ;; it undesirable to print eldoc messages right this instant.
 (defun eldoc-display-message-no-interference-p ()
+  "Nil when displaying an ElDoc message would cause interference
+with other features."
   (not (or executing-kbd-macro (bound-and-true-p edebug-active))))
 
 \f
@@ -347,6 +367,7 @@ eldoc-documentation-function
 return any documentation.")
 
 (defun eldoc-print-current-symbol-info ()
+  "Print the output of `eldoc-documentation-function'."
   ;; This is run from post-command-hook or some idle timer thing,
   ;; so we need to be careful that errors aren't ignored.
   (with-demoted-errors "eldoc error: %s"
@@ -361,6 +382,12 @@ eldoc-print-current-symbol-info
 ;; truncated or eliminated entirely from the output to make room for the
 ;; description.
 (defun eldoc-docstring-format-sym-doc (prefix doc &optional face)
+  "Concatenate PREFIX and DOC, returning the largest part of the
+resultant string that can fit in the minibuffer window.
+
+When PREFIX is a symbol, apply FACE to it before concatenating.
+
+See also: `eldoc-echo-area-use-multiline-p'."
   (when (symbolp prefix)
     (setq prefix (concat (propertize (symbol-name prefix) 'face face) ": ")))
   (let* ((ea-multi eldoc-echo-area-use-multiline-p)
@@ -390,22 +417,26 @@ eldoc-docstring-format-sym-doc
 ;; These functions do display-command table management.
 
 (defun eldoc-add-command (&rest cmds)
+  "Add each of CMDS to the obarray `eldoc-message-commands'."
   (dolist (name cmds)
     (and (symbolp name)
          (setq name (symbol-name name)))
     (set (intern name eldoc-message-commands) t)))
 
 (defun eldoc-add-command-completions (&rest names)
+  "Pass every prefix completion of NAMES to `eldoc-add-command'."
   (dolist (name names)
     (apply #'eldoc-add-command (all-completions name obarray 'commandp))))
 
 (defun eldoc-remove-command (&rest cmds)
+  "Remove each of CMDS from the obarray `eldoc-message-commands'."
   (dolist (name cmds)
     (and (symbolp name)
          (setq name (symbol-name name)))
     (unintern name eldoc-message-commands)))
 
 (defun eldoc-remove-command-completions (&rest names)
+  "Pass every prefix completion of NAMES to `eldoc-remove-command'."
   (dolist (name names)
     (apply #'eldoc-remove-command
            (all-completions name eldoc-message-commands))))
@@ -418,9 +449,9 @@ eldoc-remove-command-completions
  "down-list" "end-of-" "exchange-point-and-mark" "forward-" "goto-"
  "handle-select-window" "indent-for-tab-command" "left-" "mark-page"
  "mark-paragraph" "mouse-set-point" "move-" "move-beginning-of-"
- "move-end-of-" "newline" "next-" "other-window" "pop-global-mark" "previous-"
- "recenter" "right-" "scroll-" "self-insert-command" "split-window-"
- "up-list")
+ "move-end-of-" "newline" "next-" "other-window" "pop-global-mark"
+ "previous-" "recenter" "right-" "scroll-" "self-insert-command"
+ "split-window-" "up-list")
 
 (provide 'eldoc)
 
-- 
1.7.4.4


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

* bug#27230: eldoc doc
  2017-06-25  9:14       ` Charles A. Roelli
@ 2017-06-25 14:26         ` Eli Zaretskii
  2017-06-25 19:47           ` Charles A. Roelli
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2017-06-25 14:26 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: dgutov, 27230

> From: "Charles A. Roelli" <charles@aurox.ch>
> Date: Sun, 25 Jun 2017 11:14:23 +0200
> 
> Here's a doc patch for ElDoc, with some minor readability fixes.

Thanks.  Please allow me a few comments below.

> -(defun eldoc-message (&rest args)
> +(defun eldoc-message (&optional format-string &rest args)
> +  "Store and display the given message.

The first line of a doc string should ideally mention the arguments.

> +FORMAT-STRING and ARGS, if given, are passed to `format-message',
> +the output of which is stored in `eldoc-last-message'.

This leaves me wondering what happens if no arguments are supplied.

>  (defun eldoc--message-command-p (command)
> +  "Non-nil if COMMAND is a command in `eldoc-message-commands'."

"Return non-nil if ...".  The way you wrote it is appropriate for a
variable, not for a function.

>  (defun eldoc-pre-command-refresh-echo-area ()
> +  "Reprint `eldoc-last-message' to the echo area."

Are you sure about the "to" part?  I'd say "in" sounds more correct.

>  (defun eldoc-display-message-p ()
> +  "Non-nil when appropriate to display an ElDoc message."

"Return non-nil"

>  (defun eldoc-display-message-no-interference-p ()
> +  "Nil when displaying an ElDoc message would cause interference
> +with other features."

Likewise.  Also, the first line of a doc string should be a complete
sentence.

>  (defun eldoc-print-current-symbol-info ()
> +  "Print the output of `eldoc-documentation-function'."

"Print the output" sounds confusing.  How about this instead:

  Print the text produced by `eldoc-documentation-function'.

>  (defun eldoc-docstring-format-sym-doc (prefix doc &optional face)
> +  "Concatenate PREFIX and DOC, returning the largest part of the
> +resultant string that can fit in the minibuffer window.

First line not a complete sentence again.

> +When PREFIX is a symbol, apply FACE to it before concatenating.

But FACE is optional, so what if it isn't given?

Thanks for working on this.





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

* bug#27230: eldoc doc
  2017-06-25 14:26         ` Eli Zaretskii
@ 2017-06-25 19:47           ` Charles A. Roelli
  2017-06-26  1:04             ` Dmitry Gutov
  0 siblings, 1 reply; 22+ messages in thread
From: Charles A. Roelli @ 2017-06-25 19:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: dgutov, 27230

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

Thanks for the quick review!  Revised patch is attached.


On 25/06/2017 16:26, Eli Zaretskii wrote:
>> From: "Charles A. Roelli" <charles@aurox.ch>
>> Date: Sun, 25 Jun 2017 11:14:23 +0200
>>
>> Here's a doc patch for ElDoc, with some minor readability fixes.
> Thanks.  Please allow me a few comments below.
>
>> -(defun eldoc-message (&rest args)
>> +(defun eldoc-message (&optional format-string &rest args)
>> +  "Store and display the given message.
> The first line of a doc string should ideally mention the arguments.

   "Store and possibly display FORMAT-STRING formatted with ARGS.

FORMAT-STRING (or nil, if not given) is stored in
`eldoc-last-message'.  If ARGS are given, FORMAT-STRING is first
formatted through `format-message'.

If `eldoc-last-message' is non-nil, display it using
`eldoc-message-function'.  If it is nil, clear the echo area if
there was recently a message from ElDoc there.

Return `eldoc-last-message'."

>
>> +FORMAT-STRING and ARGS, if given, are passed to `format-message',
>> +the output of which is stored in `eldoc-last-message'.
> This leaves me wondering what happens if no arguments are supplied.

See above.

>
>>   (defun eldoc--message-command-p (command)
>> +  "Non-nil if COMMAND is a command in `eldoc-message-commands'."
> "Return non-nil if ...".  The way you wrote it is appropriate for a
> variable, not for a function.

Fixed.

>
>>   (defun eldoc-pre-command-refresh-echo-area ()
>> +  "Reprint `eldoc-last-message' to the echo area."
> Are you sure about the "to" part?  I'd say "in" sounds more correct.

Agreed, it's fixed.

>
>>   (defun eldoc-display-message-p ()
>> +  "Non-nil when appropriate to display an ElDoc message."
> "Return non-nil"

Fixed.

>
>>   (defun eldoc-display-message-no-interference-p ()
>> +  "Nil when displaying an ElDoc message would cause interference
>> +with other features."
> Likewise.  Also, the first line of a doc string should be a complete
> sentence.

Fixed.

>
>>   (defun eldoc-print-current-symbol-info ()
>> +  "Print the output of `eldoc-documentation-function'."
> "Print the output" sounds confusing.  How about this instead:
>
>    Print the text produced by `eldoc-documentation-function'.

Fixed.

>
>>   (defun eldoc-docstring-format-sym-doc (prefix doc &optional face)
>> +  "Concatenate PREFIX and DOC, returning the largest part of the
>> +resultant string that can fit in the minibuffer window.
> First line not a complete sentence again.

How about this?

   "Combine PREFIX and DOC, and shorten the result to fit in the echo area.

When PREFIX is a symbol, propertize its symbol name with FACE
before combining it with DOC.  If FACE is not provided, just
apply the nil face.

See also: `eldoc-echo-area-use-multiline-p'."

>> +When PREFIX is a symbol, apply FACE to it before concatenating.
> But FACE is optional, so what if it isn't given?

See above.

>
> Thanks for working on this.

Thanks again for your advice.


[-- Attachment #2: 0001-ElDoc-add-docstrings-and-minor-refactoring-v2.patch --]
[-- Type: text/x-patch, Size: 8336 bytes --]

From 18fe8d8a80be5966c4b9141afc4fc83748c85c9f Mon Sep 17 00:00:00 2001
From: Charles A. Roelli <charles@aurox.ch>
Date: Thu, 22 Jun 2017 21:04:09 +0200
Subject: [PATCH] ElDoc: add docstrings and minor refactoring

* lisp/emacs-lisp/eldoc.el (eldoc-edit-message-commands): Add
docstring.
(turn-on-eldoc-mode): Fix capitalization.
(eldoc--supported-p): Add docstring.
(eldoc-schedule-timer): Add docstring and use
'eldoc--supported-p'.
(eldoc-message): Add docstring and make calling convention
clearer.
(eldoc--message-command-p):
(eldoc-pre-command-refresh-echo-area):
(eldoc-display-message-p):
(eldoc-display-message-no-interference-p):
(eldoc-print-current-symbol-info):
(eldoc-docstring-format-sym-doc):
(eldoc-add-command, eldoc-add-command-completions):
(eldoc-remove-command, eldoc-remove-command-completions):
Add docstring.
---
 lisp/emacs-lisp/eldoc.el |   57 +++++++++++++++++++++++++++++++++++++---------
 1 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index a05bd7c..9b4a2d4 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -160,6 +160,10 @@ eldoc-message-function
 It should receive the same arguments as `message'.")
 
 (defun eldoc-edit-message-commands ()
+  "Return an obarray containing common editing commands.
+
+When `eldoc-print-after-edit' is non-nil, ElDoc messages are only
+printed after commands contained in this obarray."
   (let ((cmds (make-vector 31 0))
 	(re (regexp-opt '("delete" "insert" "edit" "electric" "newline"))))
     (mapatoms (lambda (s)
@@ -211,16 +215,21 @@ global-eldoc-mode
 
 ;;;###autoload
 (defun turn-on-eldoc-mode ()
-  "Turn on `eldoc-mode' if the buffer has eldoc support enabled.
+  "Turn on `eldoc-mode' if the buffer has ElDoc support enabled.
 See `eldoc-documentation-function' for more detail."
   (when (eldoc--supported-p)
     (eldoc-mode 1)))
 
 (defun eldoc--supported-p ()
+  "Non-nil if an ElDoc function is set for this buffer."
   (not (memq eldoc-documentation-function '(nil ignore))))
 
 \f
 (defun eldoc-schedule-timer ()
+  "Ensure `eldoc-timer' is running.
+
+If the user has changed `eldoc-idle-delay', update the timer to
+reflect the change."
   (or (and eldoc-timer
            (memq eldoc-timer timer-idle-list)) ;FIXME: Why?
       (setq eldoc-timer
@@ -229,8 +238,7 @@ eldoc-schedule-timer
 	     (lambda ()
                (when (or eldoc-mode
                          (and global-eldoc-mode
-                              (not (memq eldoc-documentation-function
-                                         '(nil ignore)))))
+                              (eldoc--supported-p)))
                  (eldoc-print-current-symbol-info))))))
 
   ;; If user has changed the idle delay, update the timer.
@@ -268,16 +276,27 @@ eldoc-minibuffer-message
           (force-mode-line-update)))
     (apply 'message format-string args)))
 
-(defun eldoc-message (&rest args)
+(defun eldoc-message (&optional format-string &rest args)
+  "Store and possibly display FORMAT-STRING formatted with ARGS.
+
+FORMAT-STRING (or nil, if not given) is stored in
+`eldoc-last-message'.  If ARGS are given, FORMAT-STRING is first
+formatted through `format-message'.
+
+If `eldoc-last-message' is non-nil, display it using
+`eldoc-message-function'.  If it is nil, clear the echo area if
+there was recently a message from ElDoc there.
+
+Return `eldoc-last-message'."
   (let ((omessage eldoc-last-message))
     (setq eldoc-last-message
-	  (cond ((eq (car args) eldoc-last-message) eldoc-last-message)
-		((null (car args)) nil)
+	  (cond ((eq format-string eldoc-last-message) eldoc-last-message)
+		((null format-string) nil)
 		;; If only one arg, no formatting to do, so put it in
 		;; eldoc-last-message so eq test above might succeed on
 		;; subsequent calls.
-		((null (cdr args)) (car args))
-		(t (apply #'format-message args))))
+		((null args) format-string)
+		(t (apply #'format-message format-string args))))
     ;; In emacs 19.29 and later, and XEmacs 19.13 and later, all messages
     ;; are recorded in a log.  Do not put eldoc messages in that log since
     ;; they are Legion.
@@ -289,6 +308,7 @@ eldoc-message
   eldoc-last-message)
 
 (defun eldoc--message-command-p (command)
+  "Return non-nil if COMMAND is in `eldoc-message-commands'."
   (and (symbolp command)
        (intern-soft (symbol-name command) eldoc-message-commands)))
 
@@ -299,6 +319,7 @@ eldoc--message-command-p
 ;; before the next command executes, which does away with the flicker.
 ;; This doesn't seem to be required for Emacs 19.28 and earlier.
 (defun eldoc-pre-command-refresh-echo-area ()
+  "Reprint `eldoc-last-message' in the echo area."
   (and eldoc-last-message
        (not (minibufferp))      ;We don't use the echo area when in minibuffer.
        (if (and (eldoc-display-message-no-interference-p)
@@ -310,6 +331,7 @@ eldoc-pre-command-refresh-echo-area
 
 ;; Decide whether now is a good time to display a message.
 (defun eldoc-display-message-p ()
+  "Return non-nil when it is appropriate to display an ElDoc message."
   (and (eldoc-display-message-no-interference-p)
        ;; If this-command is non-nil while running via an idle
        ;; timer, we're still in the middle of executing a command,
@@ -322,6 +344,7 @@ eldoc-display-message-p
 ;; Check various conditions about the current environment that might make
 ;; it undesirable to print eldoc messages right this instant.
 (defun eldoc-display-message-no-interference-p ()
+  "Return nil if displaying a message would cause interference."
   (not (or executing-kbd-macro (bound-and-true-p edebug-active))))
 
 \f
@@ -347,6 +370,7 @@ eldoc-documentation-function
 return any documentation.")
 
 (defun eldoc-print-current-symbol-info ()
+  "Print the text produced by `eldoc-documentation-function'."
   ;; This is run from post-command-hook or some idle timer thing,
   ;; so we need to be careful that errors aren't ignored.
   (with-demoted-errors "eldoc error: %s"
@@ -361,6 +385,13 @@ eldoc-print-current-symbol-info
 ;; truncated or eliminated entirely from the output to make room for the
 ;; description.
 (defun eldoc-docstring-format-sym-doc (prefix doc &optional face)
+  "Combine PREFIX and DOC, and shorten the result to fit in the echo area.
+
+When PREFIX is a symbol, propertize its symbol name with FACE
+before combining it with DOC.  If FACE is not provided, just
+apply the nil face.
+
+See also: `eldoc-echo-area-use-multiline-p'."
   (when (symbolp prefix)
     (setq prefix (concat (propertize (symbol-name prefix) 'face face) ": ")))
   (let* ((ea-multi eldoc-echo-area-use-multiline-p)
@@ -390,22 +421,26 @@ eldoc-docstring-format-sym-doc
 ;; These functions do display-command table management.
 
 (defun eldoc-add-command (&rest cmds)
+  "Add each of CMDS to the obarray `eldoc-message-commands'."
   (dolist (name cmds)
     (and (symbolp name)
          (setq name (symbol-name name)))
     (set (intern name eldoc-message-commands) t)))
 
 (defun eldoc-add-command-completions (&rest names)
+  "Pass every prefix completion of NAMES to `eldoc-add-command'."
   (dolist (name names)
     (apply #'eldoc-add-command (all-completions name obarray 'commandp))))
 
 (defun eldoc-remove-command (&rest cmds)
+  "Remove each of CMDS from the obarray `eldoc-message-commands'."
   (dolist (name cmds)
     (and (symbolp name)
          (setq name (symbol-name name)))
     (unintern name eldoc-message-commands)))
 
 (defun eldoc-remove-command-completions (&rest names)
+  "Pass every prefix completion of NAMES to `eldoc-remove-command'."
   (dolist (name names)
     (apply #'eldoc-remove-command
            (all-completions name eldoc-message-commands))))
@@ -418,9 +453,9 @@ eldoc-remove-command-completions
  "down-list" "end-of-" "exchange-point-and-mark" "forward-" "goto-"
  "handle-select-window" "indent-for-tab-command" "left-" "mark-page"
  "mark-paragraph" "mouse-set-point" "move-" "move-beginning-of-"
- "move-end-of-" "newline" "next-" "other-window" "pop-global-mark" "previous-"
- "recenter" "right-" "scroll-" "self-insert-command" "split-window-"
- "up-list")
+ "move-end-of-" "newline" "next-" "other-window" "pop-global-mark"
+ "previous-" "recenter" "right-" "scroll-" "self-insert-command"
+ "split-window-" "up-list")
 
 (provide 'eldoc)
 
-- 
1.7.4.4


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

* bug#27230: eldoc doc
  2017-06-25 19:47           ` Charles A. Roelli
@ 2017-06-26  1:04             ` Dmitry Gutov
  2017-06-27 19:51               ` Charles A. Roelli
  2017-09-14 11:47               ` Peder O. Klingenberg
  0 siblings, 2 replies; 22+ messages in thread
From: Dmitry Gutov @ 2017-06-26  1:04 UTC (permalink / raw)
  To: Charles A. Roelli, Eli Zaretskii; +Cc: 27230

On 6/25/17 10:47 PM, Charles A. Roelli wrote:

> FORMAT-STRING (or nil, if not given) is stored in
> `eldoc-last-message'.  If ARGS are given, FORMAT-STRING is first
> formatted through `format-message'.

I wonder if we ever call this function with more than one argument. If 
not, the code and the doc call for simplification. We can avoid 
advertising this possibility, at least.

> If `eldoc-last-message' is non-nil, display it using
> `eldoc-message-function'.  If it is nil, clear the echo area if
> there was recently a message from ElDoc there.

I think this needlessly prioritizes the implementation over the 
intention. And the latter is to display whatever the documentation 
function returns (if non-nil). Saving to eldoc-last-message is 
secondary, and can be mentioned later.

> Return `eldoc-last-message'."

This is probably non-essential, and we can avoid mentioning it.





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

* bug#27230: eldoc doc
  2017-06-26  1:04             ` Dmitry Gutov
@ 2017-06-27 19:51               ` Charles A. Roelli
  2017-06-27 23:50                 ` Dmitry Gutov
  2017-09-14 11:47               ` Peder O. Klingenberg
  1 sibling, 1 reply; 22+ messages in thread
From: Charles A. Roelli @ 2017-06-27 19:51 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: 27230

On 26/06/2017 03:04, Dmitry Gutov wrote:
> On 6/25/17 10:47 PM, Charles A. Roelli wrote:
>
>> FORMAT-STRING (or nil, if not given) is stored in
>> `eldoc-last-message'.  If ARGS are given, FORMAT-STRING is first
>> formatted through `format-message'.
>
> I wonder if we ever call this function with more than one argument. If 
> not, the code and the doc call for simplification. We can avoid 
> advertising this possibility, at least.

I think it follows the tradition of other *-message functions, which
normally use the calling convention of `message'/`format-message'.

>
>> If `eldoc-last-message' is non-nil, display it using
>> `eldoc-message-function'.  If it is nil, clear the echo area if
>> there was recently a message from ElDoc there.
>
> I think this needlessly prioritizes the implementation over the 
> intention. And the latter is to display whatever the documentation 
> function returns (if non-nil). Saving to eldoc-last-message is 
> secondary, and can be mentioned later.

Agreed. Can you suggest how to word it?  Here's what I come up with:

   "Display FORMAT-STRING formatted with ARGS as an ElDoc message.

Store the message (if any) in `eldoc-last-message', and return it."

>
>> Return `eldoc-last-message'."
>
> This is probably non-essential, and we can avoid mentioning it.

I think it once again follows the convention of other *-message
functions, probably for consistency.






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

* bug#27230: eldoc doc
  2017-06-27 19:51               ` Charles A. Roelli
@ 2017-06-27 23:50                 ` Dmitry Gutov
  2017-06-28 19:16                   ` Charles A. Roelli
  0 siblings, 1 reply; 22+ messages in thread
From: Dmitry Gutov @ 2017-06-27 23:50 UTC (permalink / raw)
  To: Charles A. Roelli, Eli Zaretskii; +Cc: 27230

On 6/27/17 10:51 PM, Charles A. Roelli wrote:

> I think it follows the tradition of other *-message functions, which
> normally use the calling convention of `message'/`format-message'.

Do we really need a function in "the tradition of other *-message 
functions" if we don't ever use it like that?

> Agreed. Can you suggest how to word it?  Here's what I come up with:
> 
>    "Display FORMAT-STRING formatted with ARGS as an ElDoc message.
> 
> Store the message (if any) in `eldoc-last-message', and return it."

Looks okay to me, aside from what I mentioned above.

>>> Return `eldoc-last-message'."
>>
>> This is probably non-essential, and we can avoid mentioning it.
> 
> I think it once again follows the convention of other *-message
> functions, probably for consistency.

Yeah, ok.






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

* bug#27230: eldoc doc
  2017-06-27 23:50                 ` Dmitry Gutov
@ 2017-06-28 19:16                   ` Charles A. Roelli
  2017-07-22  8:11                     ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Charles A. Roelli @ 2017-06-28 19:16 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: 27230

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

On 28/06/2017 01:50, Dmitry Gutov wrote:
> On 6/27/17 10:51 PM, Charles A. Roelli wrote:
>
>> I think it follows the tradition of other *-message functions, which
>> normally use the calling convention of `message'/`format-message'.
>
> Do we really need a function in "the tradition of other *-message 
> functions" if we don't ever use it like that?

I don't know.  It might be helpful in the future, and it makes sense
to follow the existing convention even if we don't make use of it yet
(patch v3 attached).



[-- Attachment #2: 0001-ElDoc-add-docstrings-and-minor-refactoring-v3.patch --]
[-- Type: text/x-patch, Size: 8049 bytes --]

From 078f0a28c22708c73b8362b3d19fe4679f5f1a00 Mon Sep 17 00:00:00 2001
From: Charles A. Roelli <charles@aurox.ch>
Date: Thu, 22 Jun 2017 21:04:09 +0200
Subject: [PATCH] ElDoc: add docstrings and minor refactoring

* lisp/emacs-lisp/eldoc.el (eldoc-edit-message-commands): Add
docstring.
(turn-on-eldoc-mode): Fix capitalization.
(eldoc--supported-p): Add docstring.
(eldoc-schedule-timer): Add docstring and use
'eldoc--supported-p'.
(eldoc-message): Add docstring and make calling convention
clearer.
(eldoc--message-command-p):
(eldoc-pre-command-refresh-echo-area):
(eldoc-display-message-p):
(eldoc-display-message-no-interference-p):
(eldoc-print-current-symbol-info):
(eldoc-docstring-format-sym-doc):
(eldoc-add-command, eldoc-add-command-completions):
(eldoc-remove-command, eldoc-remove-command-completions):
Add docstring.
---
 lisp/emacs-lisp/eldoc.el |   49 +++++++++++++++++++++++++++++++++++----------
 1 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index a05bd7c..bca40ab 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -160,6 +160,10 @@ eldoc-message-function
 It should receive the same arguments as `message'.")
 
 (defun eldoc-edit-message-commands ()
+  "Return an obarray containing common editing commands.
+
+When `eldoc-print-after-edit' is non-nil, ElDoc messages are only
+printed after commands contained in this obarray."
   (let ((cmds (make-vector 31 0))
 	(re (regexp-opt '("delete" "insert" "edit" "electric" "newline"))))
     (mapatoms (lambda (s)
@@ -211,16 +215,21 @@ global-eldoc-mode
 
 ;;;###autoload
 (defun turn-on-eldoc-mode ()
-  "Turn on `eldoc-mode' if the buffer has eldoc support enabled.
+  "Turn on `eldoc-mode' if the buffer has ElDoc support enabled.
 See `eldoc-documentation-function' for more detail."
   (when (eldoc--supported-p)
     (eldoc-mode 1)))
 
 (defun eldoc--supported-p ()
+  "Non-nil if an ElDoc function is set for this buffer."
   (not (memq eldoc-documentation-function '(nil ignore))))
 
 \f
 (defun eldoc-schedule-timer ()
+  "Ensure `eldoc-timer' is running.
+
+If the user has changed `eldoc-idle-delay', update the timer to
+reflect the change."
   (or (and eldoc-timer
            (memq eldoc-timer timer-idle-list)) ;FIXME: Why?
       (setq eldoc-timer
@@ -229,8 +238,7 @@ eldoc-schedule-timer
 	     (lambda ()
                (when (or eldoc-mode
                          (and global-eldoc-mode
-                              (not (memq eldoc-documentation-function
-                                         '(nil ignore)))))
+                              (eldoc--supported-p)))
                  (eldoc-print-current-symbol-info))))))
 
   ;; If user has changed the idle delay, update the timer.
@@ -268,16 +276,19 @@ eldoc-minibuffer-message
           (force-mode-line-update)))
     (apply 'message format-string args)))
 
-(defun eldoc-message (&rest args)
+(defun eldoc-message (&optional format-string &rest args)
+  "Display FORMAT-STRING formatted with ARGS as an ElDoc message.
+
+Store the message (if any) in `eldoc-last-message', and return it."
   (let ((omessage eldoc-last-message))
     (setq eldoc-last-message
-	  (cond ((eq (car args) eldoc-last-message) eldoc-last-message)
-		((null (car args)) nil)
+	  (cond ((eq format-string eldoc-last-message) eldoc-last-message)
+		((null format-string) nil)
 		;; If only one arg, no formatting to do, so put it in
 		;; eldoc-last-message so eq test above might succeed on
 		;; subsequent calls.
-		((null (cdr args)) (car args))
-		(t (apply #'format-message args))))
+		((null args) format-string)
+		(t (apply #'format-message format-string args))))
     ;; In emacs 19.29 and later, and XEmacs 19.13 and later, all messages
     ;; are recorded in a log.  Do not put eldoc messages in that log since
     ;; they are Legion.
@@ -289,6 +300,7 @@ eldoc-message
   eldoc-last-message)
 
 (defun eldoc--message-command-p (command)
+  "Return non-nil if COMMAND is in `eldoc-message-commands'."
   (and (symbolp command)
        (intern-soft (symbol-name command) eldoc-message-commands)))
 
@@ -299,6 +311,7 @@ eldoc--message-command-p
 ;; before the next command executes, which does away with the flicker.
 ;; This doesn't seem to be required for Emacs 19.28 and earlier.
 (defun eldoc-pre-command-refresh-echo-area ()
+  "Reprint `eldoc-last-message' in the echo area."
   (and eldoc-last-message
        (not (minibufferp))      ;We don't use the echo area when in minibuffer.
        (if (and (eldoc-display-message-no-interference-p)
@@ -310,6 +323,7 @@ eldoc-pre-command-refresh-echo-area
 
 ;; Decide whether now is a good time to display a message.
 (defun eldoc-display-message-p ()
+  "Return non-nil when it is appropriate to display an ElDoc message."
   (and (eldoc-display-message-no-interference-p)
        ;; If this-command is non-nil while running via an idle
        ;; timer, we're still in the middle of executing a command,
@@ -322,6 +336,7 @@ eldoc-display-message-p
 ;; Check various conditions about the current environment that might make
 ;; it undesirable to print eldoc messages right this instant.
 (defun eldoc-display-message-no-interference-p ()
+  "Return nil if displaying a message would cause interference."
   (not (or executing-kbd-macro (bound-and-true-p edebug-active))))
 
 \f
@@ -347,6 +362,7 @@ eldoc-documentation-function
 return any documentation.")
 
 (defun eldoc-print-current-symbol-info ()
+  "Print the text produced by `eldoc-documentation-function'."
   ;; This is run from post-command-hook or some idle timer thing,
   ;; so we need to be careful that errors aren't ignored.
   (with-demoted-errors "eldoc error: %s"
@@ -361,6 +377,13 @@ eldoc-print-current-symbol-info
 ;; truncated or eliminated entirely from the output to make room for the
 ;; description.
 (defun eldoc-docstring-format-sym-doc (prefix doc &optional face)
+  "Combine PREFIX and DOC, and shorten the result to fit in the echo area.
+
+When PREFIX is a symbol, propertize its symbol name with FACE
+before combining it with DOC.  If FACE is not provided, just
+apply the nil face.
+
+See also: `eldoc-echo-area-use-multiline-p'."
   (when (symbolp prefix)
     (setq prefix (concat (propertize (symbol-name prefix) 'face face) ": ")))
   (let* ((ea-multi eldoc-echo-area-use-multiline-p)
@@ -390,22 +413,26 @@ eldoc-docstring-format-sym-doc
 ;; These functions do display-command table management.
 
 (defun eldoc-add-command (&rest cmds)
+  "Add each of CMDS to the obarray `eldoc-message-commands'."
   (dolist (name cmds)
     (and (symbolp name)
          (setq name (symbol-name name)))
     (set (intern name eldoc-message-commands) t)))
 
 (defun eldoc-add-command-completions (&rest names)
+  "Pass every prefix completion of NAMES to `eldoc-add-command'."
   (dolist (name names)
     (apply #'eldoc-add-command (all-completions name obarray 'commandp))))
 
 (defun eldoc-remove-command (&rest cmds)
+  "Remove each of CMDS from the obarray `eldoc-message-commands'."
   (dolist (name cmds)
     (and (symbolp name)
          (setq name (symbol-name name)))
     (unintern name eldoc-message-commands)))
 
 (defun eldoc-remove-command-completions (&rest names)
+  "Pass every prefix completion of NAMES to `eldoc-remove-command'."
   (dolist (name names)
     (apply #'eldoc-remove-command
            (all-completions name eldoc-message-commands))))
@@ -418,9 +445,9 @@ eldoc-remove-command-completions
  "down-list" "end-of-" "exchange-point-and-mark" "forward-" "goto-"
  "handle-select-window" "indent-for-tab-command" "left-" "mark-page"
  "mark-paragraph" "mouse-set-point" "move-" "move-beginning-of-"
- "move-end-of-" "newline" "next-" "other-window" "pop-global-mark" "previous-"
- "recenter" "right-" "scroll-" "self-insert-command" "split-window-"
- "up-list")
+ "move-end-of-" "newline" "next-" "other-window" "pop-global-mark"
+ "previous-" "recenter" "right-" "scroll-" "self-insert-command"
+ "split-window-" "up-list")
 
 (provide 'eldoc)
 
-- 
1.7.4.4


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

* bug#27230: eldoc doc
  2017-06-28 19:16                   ` Charles A. Roelli
@ 2017-07-22  8:11                     ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2017-07-22  8:11 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: 27230-done, dgutov

> Cc: 27230@debbugs.gnu.org
> From: "Charles A. Roelli" <charles@aurox.ch>
> Date: Wed, 28 Jun 2017 21:16:32 +0200
> 
> (patch v3 attached).

Thanks, pushed.

In the future, please mention the bug number in the commit log
message, so that whoever pushes the changes won't need to do that by
hand.





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

* bug#27230: eldoc doc
  2017-06-26  1:04             ` Dmitry Gutov
  2017-06-27 19:51               ` Charles A. Roelli
@ 2017-09-14 11:47               ` Peder O. Klingenberg
  2017-09-14 12:02                 ` Dmitry Gutov
  1 sibling, 1 reply; 22+ messages in thread
From: Peder O. Klingenberg @ 2017-09-14 11:47 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Charles A. Roelli, 27230

On Mon, Jun 26 2017 at 04:04, Dmitry Gutov wrote:

> On 6/25/17 10:47 PM, Charles A. Roelli wrote:
>
>> FORMAT-STRING (or nil, if not given) is stored in
>> `eldoc-last-message'.  If ARGS are given, FORMAT-STRING is first
>> formatted through `format-message'.
>
> I wonder if we ever call this function with more than one argument. If
> not, the code and the doc call for simplification. We can avoid
> advertising this possibility, at least.

As I discovered today, when I updated my tree, there are callers outside
emacs core that do call eldoc-message with more than one argument.
slime-autodoc.el is one such package, which I use.

The slime project has an open issue on this, with a patch, but it hasn't
been applied yet, after sitting in the discussion for 17 days.

IMO commit 7ef0b5f611c2d56ac2edb8de287190f04c4b8f32 was an ill-advised
change.  The code worked fine before, the cleanup afforded by breaking
the api was negligible.  I think it should be reverted.

(Slime would benefit from cleaing up their callers as well, no doubt,
but not every slime user should be forced to do that on their own.  And
there may be more third-party, infrequently maintained packages using
eldoc.)

...Peder...
-- 
This must be Thursday.  I never could get the hang of Thursdays.





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

* bug#27230: eldoc doc
  2017-09-14 11:47               ` Peder O. Klingenberg
@ 2017-09-14 12:02                 ` Dmitry Gutov
  2017-09-14 19:39                   ` Charles A. Roelli
  0 siblings, 1 reply; 22+ messages in thread
From: Dmitry Gutov @ 2017-09-14 12:02 UTC (permalink / raw)
  To: Peder O. Klingenberg; +Cc: Charles A. Roelli, 27230

On 9/14/17 2:47 PM, Peder O. Klingenberg wrote:

> As I discovered today, when I updated my tree, there are callers outside
> emacs core that do call eldoc-message with more than one argument.
> slime-autodoc.el is one such package, which I use.

Thanks for letting us know.

> The slime project has an open issue on this, with a patch, but it hasn't
> been applied yet, after sitting in the discussion for 17 days.

I've commented on it: https://github.com/slime/slime/issues/400

For one thing, third-party packages should *not* use eldoc-message.

> IMO commit 7ef0b5f611c2d56ac2edb8de287190f04c4b8f32 was an ill-advised
> change.  The code worked fine before, the cleanup afforded by breaking
> the api was negligible.  I think it should be reverted.

Aside from breaking the "obviously wrong" callers like explained above, 
the code had a subtle bug where calling it with no format arguments, and 
then with some arguments, wouldn't update the message area.

> (Slime would benefit from cleaing up their callers as well, no doubt,
> but not every slime user should be forced to do that on their own.

Of course, the project maintainers should do it.

All the best,
Dmitry.





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

* bug#27230: eldoc doc
  2017-09-14 12:02                 ` Dmitry Gutov
@ 2017-09-14 19:39                   ` Charles A. Roelli
  2017-09-14 22:03                     ` Dmitry Gutov
  0 siblings, 1 reply; 22+ messages in thread
From: Charles A. Roelli @ 2017-09-14 19:39 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: pok, 27230

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 14 Sep 2017 15:02:11 +0300
> 
> For one thing, third-party packages should *not* use eldoc-message.

Right.  But why should we break its calling convention without good
reason, when we can see that now that other packages have been using
it?  Moreover, the function has probably been around for awhile, and
without good documentation, it might not have been clear that it's
best suited for "internal" use.

This change might also be mentioned under "Incompatible Lisp Changes"
in NEWS, and it should be discussed in a separate bug report since it
doesn't have much to do with "eldoc doc" (which this bug was about).





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

* bug#27230: eldoc doc
  2017-09-14 19:39                   ` Charles A. Roelli
@ 2017-09-14 22:03                     ` Dmitry Gutov
  2017-09-19 20:02                       ` Charles A. Roelli
  0 siblings, 1 reply; 22+ messages in thread
From: Dmitry Gutov @ 2017-09-14 22:03 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: pok, 27230

On 9/14/17 10:39 PM, Charles A. Roelli wrote:

> Right.  But why should we break its calling convention without good
> reason, when we can see that now that other packages have been using
> it?

Now they know they should fix it, though.

> This change might also be mentioned under "Incompatible Lisp Changes"
> in NEWS, and it should be discussed in a separate bug report since it
> doesn't have much to do with "eldoc doc" (which this bug was about).

Sometimes we go off tangent. It's not the end of the world.

But yes, it's a good idea, thanks.





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

* bug#27230: eldoc doc
  2017-09-14 22:03                     ` Dmitry Gutov
@ 2017-09-19 20:02                       ` Charles A. Roelli
  2017-09-20 18:12                         ` Charles A. Roelli
  0 siblings, 1 reply; 22+ messages in thread
From: Charles A. Roelli @ 2017-09-19 20:02 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: pok, 27230

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

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 15 Sep 2017 01:03:35 +0300
> 
> > Right.  But why should we break its calling convention without good
> > reason, when we can see that now that other packages have been using
> > it?
> 
> Now they know they should fix it, though.

Unfortunately it means there are probably other users of the function,
and if we don't document the change, the function should stay as is.
Attached is a patch that brings back the old behavior, without the
convoluted buggy logic for setting eldoc-last-message.  What do you
think?

And from an earlier message:

> Aside from breaking the "obviously wrong" callers like explained above, 
> the code had a subtle bug where calling it with no format arguments, and 
> then with some arguments, wouldn't update the message area.

Should be fixed with this patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Keep-old-calling-convention-of-eldoc-message.patch --]
[-- Type: text/x-patch, Size: 1410 bytes --]

From f872315286949e9b428ee108c66a8bb5f1131953 Mon Sep 17 00:00:00 2001
From: "Charles A. Roelli" <charles@aurox.ch>
Date: Tue, 19 Sep 2017 22:00:54 +0200
Subject: [PATCH] Keep old calling convention of 'eldoc-message'

* lisp/emacs-lisp/eldoc.el (eldoc-message): Keep its old calling
convention, as it's used by other packages.  See the discussion in
Bug#27230, and the previous change 7ef0b5f6 ("Simplify
eldoc-message") of 2017-08-21.
---
 lisp/emacs-lisp/eldoc.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index cba9a00..7176eb8 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -276,12 +276,13 @@ eldoc-minibuffer-message
           (force-mode-line-update)))
     (apply 'message format-string args)))
 
-(defun eldoc-message (&optional string)
+(defun eldoc-message (&optional format-string &rest args)
   "Display STRING as an ElDoc message if it's non-nil.
 
 Also store it in `eldoc-last-message' and return that value."
   (let ((omessage eldoc-last-message))
-    (setq eldoc-last-message string)
+    (setq eldoc-last-message
+          (if args (apply #'format-message format-string args) format-string))
     ;; In emacs 19.29 and later, and XEmacs 19.13 and later, all messages
     ;; are recorded in a log.  Do not put eldoc messages in that log since
     ;; they are Legion.
-- 
2.9.4


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

* bug#27230: eldoc doc
  2017-09-19 20:02                       ` Charles A. Roelli
@ 2017-09-20 18:12                         ` Charles A. Roelli
  2017-09-21 14:23                           ` Dmitry Gutov
  0 siblings, 1 reply; 22+ messages in thread
From: Charles A. Roelli @ 2017-09-20 18:12 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: dgutov, pok, 27230

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

Sorry, I had forgotten to update the docstring.  Updated patch
attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Keep-old-calling-convention-of-eldoc-message.patch --]
[-- Type: text/x-patch, Size: 1563 bytes --]

From 576fced66c8960a91a1fcfd51efc700413a810b4 Mon Sep 17 00:00:00 2001
From: "Charles A. Roelli" <charles@aurox.ch>
Date: Tue, 19 Sep 2017 22:00:54 +0200
Subject: [PATCH] Keep old calling convention of 'eldoc-message'

* lisp/emacs-lisp/eldoc.el (eldoc-message): Keep its old calling
convention, as it's used by other packages.  See the discussion in
Bug#27230, and the previous change 7ef0b5f6 ("Simplify
eldoc-message") of 2017-08-21.
---
 lisp/emacs-lisp/eldoc.el | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index cba9a00..1440fb1 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -276,12 +276,16 @@ eldoc-minibuffer-message
           (force-mode-line-update)))
     (apply 'message format-string args)))
 
-(defun eldoc-message (&optional string)
-  "Display STRING as an ElDoc message if it's non-nil.
+(defun eldoc-message (&optional format-string &rest args)
+  "Display FORMAT-STRING as an ElDoc message if it's non-nil.
+
+If ARGS are given, FORMAT-STRING and ARGS are first passed to
+`format-message'.
 
 Also store it in `eldoc-last-message' and return that value."
   (let ((omessage eldoc-last-message))
-    (setq eldoc-last-message string)
+    (setq eldoc-last-message
+          (if args (apply #'format-message format-string args) format-string))
     ;; In emacs 19.29 and later, and XEmacs 19.13 and later, all messages
     ;; are recorded in a log.  Do not put eldoc messages in that log since
     ;; they are Legion.
-- 
2.9.4


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

* bug#27230: eldoc doc
  2017-09-20 18:12                         ` Charles A. Roelli
@ 2017-09-21 14:23                           ` Dmitry Gutov
  2017-09-21 18:33                             ` Charles A. Roelli
  0 siblings, 1 reply; 22+ messages in thread
From: Dmitry Gutov @ 2017-09-21 14:23 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: pok, 27230

Hey Charles,

On 9/20/17 9:12 PM, Charles A. Roelli wrote:
> Sorry, I had forgotten to update the docstring.  Updated patch
> attached.

Is there a reason you so strongly insist on returning to the original 
calling convention? I'd rather add a NEWS entry instead.





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

* bug#27230: eldoc doc
  2017-09-21 14:23                           ` Dmitry Gutov
@ 2017-09-21 18:33                             ` Charles A. Roelli
  2017-09-21 23:05                               ` Dmitry Gutov
  2017-09-25 23:26                               ` Dmitry Gutov
  0 siblings, 2 replies; 22+ messages in thread
From: Charles A. Roelli @ 2017-09-21 18:33 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: pok, 27230

> Cc: pok@netfonds.no, 27230@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 21 Sep 2017 17:23:22 +0300
> 
> Hey Charles,
> 
> On 9/20/17 9:12 PM, Charles A. Roelli wrote:
> > Sorry, I had forgotten to update the docstring.  Updated patch
> > attached.
> 
> Is there a reason you so strongly insist on returning to the original 
> calling convention?

Yes, and another user gave an opinion too.  What do others think?





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

* bug#27230: eldoc doc
  2017-09-21 18:33                             ` Charles A. Roelli
@ 2017-09-21 23:05                               ` Dmitry Gutov
  2017-09-25 23:26                               ` Dmitry Gutov
  1 sibling, 0 replies; 22+ messages in thread
From: Dmitry Gutov @ 2017-09-21 23:05 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: pok, 27230

On 9/21/17 9:33 PM, Charles A. Roelli wrote:

> Yes, and another user gave an opinion too.  What do others think?

Just FYI, the SLIME project has merged the patch a week ago. It 
shouldn't be a problem anymore.





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

* bug#27230: eldoc doc
  2017-09-21 18:33                             ` Charles A. Roelli
  2017-09-21 23:05                               ` Dmitry Gutov
@ 2017-09-25 23:26                               ` Dmitry Gutov
  1 sibling, 0 replies; 22+ messages in thread
From: Dmitry Gutov @ 2017-09-25 23:26 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: pok, 27230

On 9/21/17 9:33 PM, Charles A. Roelli wrote:
> What do others think?

Since nobody else is chiming in, I've updated the "Incompatible Lisp 
Changes" in etc/NEWS.





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

end of thread, other threads:[~2017-09-25 23:26 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-04 10:38 bug#27230: eldoc doc Charles A. Roelli
2017-06-05 22:08 ` Dmitry Gutov
2017-06-06 18:33   ` Charles A. Roelli
2017-06-06 20:19     ` Dmitry Gutov
2017-06-25  9:14       ` Charles A. Roelli
2017-06-25 14:26         ` Eli Zaretskii
2017-06-25 19:47           ` Charles A. Roelli
2017-06-26  1:04             ` Dmitry Gutov
2017-06-27 19:51               ` Charles A. Roelli
2017-06-27 23:50                 ` Dmitry Gutov
2017-06-28 19:16                   ` Charles A. Roelli
2017-07-22  8:11                     ` Eli Zaretskii
2017-09-14 11:47               ` Peder O. Klingenberg
2017-09-14 12:02                 ` Dmitry Gutov
2017-09-14 19:39                   ` Charles A. Roelli
2017-09-14 22:03                     ` Dmitry Gutov
2017-09-19 20:02                       ` Charles A. Roelli
2017-09-20 18:12                         ` Charles A. Roelli
2017-09-21 14:23                           ` Dmitry Gutov
2017-09-21 18:33                             ` Charles A. Roelli
2017-09-21 23:05                               ` Dmitry Gutov
2017-09-25 23:26                               ` Dmitry Gutov

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

	https://git.savannah.gnu.org/cgit/emacs.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).