Thanks for the quick review! Revised patch is attached. On 25/06/2017 16:26, Eli Zaretskii wrote: >> From: "Charles A. Roelli" >> 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.