bug#2035: 23.0.60; doc string of dired-read-shell-command

bug#2035: 23.0.60; doc string of dired-read-shell-command

[Drew Adams]

Doc string:
 
"Read a dired shell command prompting with PROMPT (using read-string).
ARG is the prefix arg and may be used to indicate in the prompt which
  files are affected.
This is an extra function so that you can redefine it."
 
1. The part about the "prefix arg", at least, doesn't make sense. This
is not a command, and there is no reference in the code to
`current-prefix-arg'. The sentence is not clear to me, even ignoring
"prefix" - *how* does ARG indicate, in the prompt, which files are
affected?
 
2. Argument FILES is not described (should be uppercase).
 
3. It's not clear (to me) what the last sentence is supposed to mean.

In GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600) of 2009-01-04 on
 LENNART-69DE564 Windowing system distributor `Microsoft Corp.',
 version 5.1.2600 configured using `configure --with-gcc (3.4)
 --no-opt --cflags -Ic:/g/include -fno-crossjumping'
 

bug#2035: 23.0.60; doc string of dired-read-shell-command

[Juanma Barranquero]

> 1. The part about the "prefix arg", at least, doesn't make sense. This
> is not a command, and there is no reference in the code to
> `current-prefix-arg'. The sentence is not clear to me, even ignoring
> "prefix" - *how* does ARG indicate, in the prompt, which files are
> affected?

It makes somewhat more sense if you look at the original version in
dired-aux.el.

> 2. Argument FILES is not described (should be uppercase).

It is fixed in the dired-aux version :(

> 3. It's not clear (to me) what the last sentence is supposed to mean.

Is part of a comment in the original version.

All in all, I think the docstring should say: "This is an internal
function." and little more. The info in the docstring should have
stayed a comment IMHO.

    Juanma

bug#2035: 23.0.60; doc string of dired-read-shell-command

[Drew Adams]

> From: Juanma Barranquero Sent: Friday, February 13, 2009 3:49 AM
> > 1. The part about the "prefix arg", at least, doesn't make 
> > sense. This is not a command, and there is no reference in the code to
> > `current-prefix-arg'. The sentence is not clear to me, even ignoring
> > "prefix" - *how* does ARG indicate, in the prompt, which files are
> > affected?
> 
> It makes somewhat more sense if you look at the original version in
> dired-aux.el.
> 
> > 2. Argument FILES is not described (should be uppercase).
> 
> It is fixed in the dired-aux version :(
> 
> > 3. It's not clear (to me) what the last sentence is 
> > supposed to mean.
> 
> Is part of a comment in the original version.
> 
> All in all, I think the docstring should say: "This is an internal
> function." and little more. The info in the docstring should have
> stayed a comment IMHO.

It's not necessarily an internal function. It's a non-interactive function, but
there is no reason that other code can't also call it. 

The doc string should explain what the function does, as always (but not how it
does it).

bug#2035: 23.0.60; doc string of dired-read-shell-command

[Lars Magne Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

> "Read a dired shell command prompting with PROMPT (using read-string).
> ARG is the prefix arg and may be used to indicate in the prompt which
>   files are affected.
> This is an extra function so that you can redefine it."

This has apparently already been fixed.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#2035: 23.0.60; doc string of dired-read-shell-command

[Drew Adams]

> This has apparently already been fixed.

No, it has not.  The doc string has been changed, but the bug report still
applies.  Please read it.  What about the prefix argument?  This is not a
command.  How can it have a prefix argument? Etc.

bug#2035: 23.0.60; doc string of dired-read-shell-command

[Lars Magne Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

>> This has apparently already been fixed.
>
> No, it has not.  The doc string has been changed, but the bug report still
> applies.  Please read it.  What about the prefix argument?  This is not a
> command.  How can it have a prefix argument? Etc.

It is passed `current-prefix-arg' in the use cases, so I think that's
clear enough.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#2035: 23.0.60; doc string of dired-read-shell-command

[Chong Yidong]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> "Drew Adams" <drew.adams@oracle.com> writes:
>
>>> This has apparently already been fixed.
>>
>> No, it has not.  The doc string has been changed, but the bug report still
>> applies.  Please read it.  What about the prefix argument?  This is not a
>> command.  How can it have a prefix argument? Etc.
>
> It is passed `current-prefix-arg' in the use cases, so I think that's
> clear enough.

I fixed the remaining doc issues; closing.

bug#2035: 23.0.60; doc string of dired-read-shell-command

[Drew Adams]

> >>> This has apparently already been fixed.
> >>
> >> No, it has not.  The doc string has been changed, but the 
> >> bug report still applies.  Please read it.  What about the
> >> prefix argument?  This is not a command.  How can it have a
> >> prefix argument? Etc.
> >
> > It is passed `current-prefix-arg' in the use cases, so I 
> > think that's clear enough.
> 
> I fixed the remaining doc issues; closing.

Haven't yet seen your changes, Yidong, but the reply below to Lars apparently
did not make it to the thread - perhaps I forgot to use `Reply All'.  I hope its
points were proactively addressed by your fix.

It is incorrect to speak here (at all) about "prefix arg" or
`current-prefix-arg'.

Thx.

> Sent: Monday, July 11, 2011 9:13 AM  To: 'Lars Magne Ingebrigtsen'
> Subject: RE: 23.0.60; doc string of dired-read-shell-command
> 
> > >> This has apparently already been fixed.
> > >
> > > No, it has not.  The doc string has been changed, but the 
> > > bug report still applies.  Please read it.  What about the
> > > prefix argument?  This is not a command.  How can it have a
> > > prefix argument? Etc.
> > 
> > It is passed `current-prefix-arg' in the use cases, so I 
> > think that's clear enough.
> 
> Only a caller knows what it passes to `dired-read-shell-command'.
> The code for this function, and the doc string for this function,
> make no reference to `current-prefix-arg'.
> 
> If the code makes no reference to `current-prefix-arg' and 
> yet this function is always supposed to use 
> `current-prefix-arg', then it should not be passed as an arg 
> but should be hard-coded in the body.
> 
> If, OTOH, we want the argument ARG to be able to be something 
> other than the value of `current-prefix-arg', then we should 
> keep that parameter.  And if it is kept it needs to be 
> documented.  And it must not, in that case, be documented as 
> the "prefix arg" or `current-prefix-arg' or any such thing.  
> It must be documented in its own right: what is it for?  What 
> are its possible values?
> 
> This should be obvious.  This is the approach for any 
> function.  If we want always the same value, then don't pass 
> it as a paramter.  If we define a parameter then document it.