Subject: Re: bug#26649: 26.0.50; buffer-command: Return the cmd that started a process in buffer

From: Eli Zaretskii <[email-masked]>
Date: Wed, 26 Apr 2017 13:03:36 +0300

> From: Tino Calancha <[email-masked]>
> Date: Tue, 25 Apr 2017 17:15:26 +0900
> 
> Severity: wishlist
> 
> I define in my private customization a command `buffer-command', which
> calls `process-command' on the process associated with the current
> buffer.
> Interactively, it shows in the echo area the command as the last column
> of `list-processes' does.
> Is anyone interested in adding this feature?

I don't have an opinion on this, perhaps others will offer theirs.  I
do have a few comments:

> Subject: [PATCH] buffer-command: Return the cmd that started a process in
>  buffer                          ^^^^^^

Not "return", "display".

> * lisp/simple.el (buffer-command): Return command executed to
> start a process in BUFFER (Bug#26649).

Likewise.  Also, we generally use just "new command" in these cases.

> * doc/lispref/processes.texi (Process Information): Document new command.

It is better to mention the name of the command.

> +@deffn Command buffer-command &optional buffer

I think it would be better to have the command's name start with
"process-", so something like process-command-in-buffer.

> +This command calls @code{process-command} on the process associated
> +with @var{buffer}.

This describes the implementation; it should instead describe the
effect of the command.

> +If @var{buffer} is not associated with a running process, then returns
> +@code{nil}.

The value returned by a command is not interesting, you should
describe the effect of invoking this command in a buffer that has n o
associated process.

> +(defun buffer-command (&optional buffer)
> +  "Return the command that was executed to start the process in BUFFER.

Same comment here: describe the effect, not the return value.  If the
returned value is important for non-interactive invocations, it should
be described in addition to the effect of an interactive invocation.

> +Buffer defaults to the current buffer.
   ^^^^^^
BUFFER

> +    (error "Not a living buffer '%S'" buffer))

"Not a live buffer '%S'"

> +    (cond ((null cmd)
> +	       (message "No process in buffer '%s'" (buffer-name buf))

It is better to say something like

  Buffer '%s' is not associated with any process

Btw, why %s here and %S in the error message?

> +          (t
> +           (message "%s" (mapconcat #'identity cmd " "))

What will this produce if some of the command-line arguments include
embedded whitespace?

> +(ert-deftest subr-tests--buffer-command ()
> +  (let ((program (executable-find "sleep"))
> +        (timeout "10") proc)

Instead of bypassing the test where 'sleep' doesn't exist, how about
if you use the Emacs executable instead?  That will always exist.

Thanks.