bug#19421: 25.0.50; doc string of `browse-url' must describe parameter ARGS

bug#19421: 25.0.50; doc string of `browse-url' must describe parameter ARGS

[Drew Adams]

Subject line says it all.  Please respect the GNU Emacs convention of
specifying each of the parameters in the doc string.

In this case, the doc for ARGS should mention option `browse-url-new-window-flag', among other things.

In GNU Emacs 25.0.50.1 (i686-pc-mingw32)
 of 2014-10-20 on LEG570
Bzr revision: 118168 rgm@gnu.org-20141020195941-icp42t8ttcnud09g
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --enable-checking=yes,glyphs CPPFLAGS=-DGLYPH_DEBUG=1'

bug#19421: 25.0.50; doc string of `browse-url' must describe parameter ARGS

[Drew Adams]

Same thing for the other functions in browse-url.el that have an ARGS &rest parameter - e.g., `browse-url-default-browser'.

Function doc strings need to specify the function's calling sequence, describing each of its parameters.

bug#19421: 25.0.50; doc string of `browse-url' must describe parameter ARGS

[Lars Ingebrigtsen]

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

> Subject line says it all.  Please respect the GNU Emacs convention of
> specifying each of the parameters in the doc string.

The doc string says

"Passes any ARGS to the browser function."

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

bug#19421: 25.0.50; doc string of `browse-url' must describe parameter ARGS

[Drew Adams]

> > Please respect the GNU Emacs convention of
> > specifying each of the parameters in the doc string.
> > In this case, the doc for ARGS should mention option
> > `browse-url-new-window-flag', among other things.
> 
> The doc string says
> "Passes any ARGS to the browser function."

Yes, and that IS the bug.  The doc string does not specify
parameter ARGS properly, helpfully, usefully.

And there is this part of the same bug report, also ignored:

  > Same thing for the other functions in browse-url.el that have
  > an ARGS &rest parameter - e.g., `browse-url-default-browser'.

This bug has not been fixed - but you know that.

Why not fix it?  Why do you not respect the GNU Emacs conventions
and its high standards of self-documentation?  What possible good
reason do you have for not doing the right thing, here?


As for `browse-url-default-browser', its doc string does not
even have the lame excuse you used: 

  > The doc string says "Passes any ARGS to the browser function."

It says nothing at all about ARGS.  And it has even more problems.
At least in the latest Emacs 25 snapshot I have, which dates from
2015-12-04:

* This is the parameters lambda list: (URL &rest ARGS).

* This is how the parameters are described in the doc string:

   "When called non-interactively, optional second argument
    NEW-WINDOW is used instead of 'browse-url-new-window-flag'."

That's it!  Nothing about parameter URL.  Nothing about
&rest-parameter ARGS (the subject of this bug report).  And
yet something about an "optional second argument NEW-WINDOW",
which is not even present in the lambda list.

Worse yet: It says "When called non-interactively", suggesting
that the function could be called interactively.  But it cannot -
it is not a command.

This is a mess, even if it is a trivial mess to fix.  It is a
shame to deliberately ignore such a simple, and obvious bug.

bug#19421: 25.0.50; doc string of `browse-url' must describe parameter ARGS

[Eli Zaretskii]

> Date: Fri, 25 Dec 2015 15:07:06 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 19421@debbugs.gnu.org
> 
> > > Please respect the GNU Emacs convention of
> > > specifying each of the parameters in the doc string.
> > > In this case, the doc for ARGS should mention option
> > > `browse-url-new-window-flag', among other things.
> > 
> > The doc string says
> > "Passes any ARGS to the browser function."
> 
> Yes, and that IS the bug.  The doc string does not specify
> parameter ARGS properly, helpfully, usefully.
> 
> And there is this part of the same bug report, also ignored:
> 
>   > Same thing for the other functions in browse-url.el that have
>   > an ARGS &rest parameter - e.g., `browse-url-default-browser'.

What would you like us to say about ARGS?  If you looked at the
functions that can be invoked by browse-url, you know that they either
ignore ARGS or (in a few cases) use ARGS to pass the NEW-WINDOW flag,
in which case the corresponding function documents that.

So this ARGS thingy is really for custom-written browser functions, in
which case whoever writes them should know what ARGS are for, and
should document that in the doc string of the function she writes.

> This bug has not been fixed - but you know that.
> 
> Why not fix it?  Why do you not respect the GNU Emacs conventions
> and its high standards of self-documentation?  What possible good
> reason do you have for not doing the right thing, here?

Do you want us to say that ARGS are ignored?  Is that what would in
your opinion fix this bug?

> As for `browse-url-default-browser', its doc string does not
> even have the lame excuse you used: 
> 
>   > The doc string says "Passes any ARGS to the browser function."
> 
> It says nothing at all about ARGS.

Because it is just a dispatcher -- it invokes other functions, which
mostly ignore ARGS altogether.

> At least in the latest Emacs 25 snapshot I have, which dates from
> 2015-12-04:
> 
> * This is the parameters lambda list: (URL &rest ARGS).
> 
> * This is how the parameters are described in the doc string:
> 
>    "When called non-interactively, optional second argument
>     NEW-WINDOW is used instead of 'browse-url-new-window-flag'."
> 
> That's it!  Nothing about parameter URL.

I don't think URL should need any explanations in a package that deals
with browsing URLs.  So this part is a red herring, IMO.

> This is a mess, even if it is a trivial mess to fix.  It is a
> shame to deliberately ignore such a simple, and obvious bug.

You should watch your language if you want your comments to be taken
seriously.  If every minor issue is "a mess" and "a shame", then what
words can you use for real problems?

bug#19421: 25.0.50; doc string of `browse-url' must describe parameter ARGS

[Drew Adams]

> What would you like us to say about ARGS?

Say what it is for?  Name and describe its structure or its parts,
if any of them (NEW-WINDOW?) are significant?  I don't know the
functions, so I can't really say what might be appropriate to say
about ARGS, here.

> If you looked at the
> functions that can be invoked by browse-url, you know that they either
> ignore ARGS or (in a few cases) use ARGS to pass the NEW-WINDOW flag,
> in which case the corresponding function documents that.

If a given function that has an ARGS &rest parameter does nothing
else with ARGS except pass it on to some other function, it is
enough for the doc of the first function to say that - as usual.

Certainly the function's doc should say nothing about "NEW-WINDOW"
unless either NEW-WINDOW is in the parameter list or the doc
describes it in terms of parameters that are in the list (e.g.,
as one of the members of argument-list ARGS).  The mention of
NEW-WINDOW comes out of the blue and is incomprehensible to a
user reading the doc string (this user, at least).

> So this ARGS thingy is really for custom-written browser functions, in
> which case whoever writes them should know what ARGS are for, and
> should document that in the doc string of the function she writes.

I have no complaints about imaginary doc strings of possible
user-written functions.  The bug report is about doc strings of
predefined Emacs functions that have ARGS as &rest parameter.

> > Why not fix it?  Why do you not respect the GNU Emacs conventions
> > and its high standards of self-documentation?  What possible good
> > reason do you have for not doing the right thing, here?
> 
> Do you want us to say that ARGS are ignored?  Is that what would in
> your opinion fix this bug?

I don't know the behavior, so I won't pretend to prescribe what
the doc should say.  IF the ARGS are always completely ignored by
a function that accepts &rest ARGS as arguments, then that is not
a doc problem - the function should not accept arguments if it in
fact always ignores them.

But if it passes the ARGS on to some other function (for example)
then that's what the doc string should say.  That is not ignoring
the arguments.

And in that case it need say nothing more than that: "ARGS are
passed to function ____" or "ARGS are passed to the function that
is the value of variable ____" or similar.  This is the usual
treatment, no?  I don't think I'm inventing anything, here.

> > As for `browse-url-default-browser', its doc string does not
> > even have the lame excuse you used:
> >   > The doc string says "Passes any ARGS to the browser function."
> > It says nothing at all about ARGS.
> 
> Because it is just a dispatcher -- it invokes other functions,
> which mostly ignore ARGS altogether.

Then that's what its doc should say: it passes ARGS to other
functions (and name or otherwise specify what those functions
are or can be).  And whether or not those other functions ignore
ARGS is irrelevant to _this_ doc string for _this_ function.

> > At least in the latest Emacs 25 snapshot I have, which dates from
> > 2015-12-04:
> >
> > * This is the parameters lambda list: (URL &rest ARGS).
> > * This is how the parameters are described in the doc string:
> >    "When called non-interactively, optional second argument
> >     NEW-WINDOW is used instead of 'browse-url-new-window-flag'."
> >
> > That's it!  Nothing about parameter URL.
> 
> I don't think URL should need any explanations in a package that
> deals with browsing URLs.  So this part is a red herring, IMO.

It need not say that parameter URL is a url, granted.  But it
might want to say something about it being a string, or about
it being passed to ____, or something else - I don't know.  If
you think there is nothing at all to say about URL, I'm OK with
that.

Frankly, I'm comfortable with you, Eli, just taking a look at
the bug report.  I'm not very happy with it having been summarily
closed without any consideration.  I'm not really keen to jump
back in on this and argue about it - I really don't care that
much.

I would hope that someone would either take a considered look
at it and DTRT or that it would simply remain open until
someone does that.  That's the proper respect to show to bug
reports and to the users who took the time to report them -
even when the reports might be mistaken or foolish.

Users who report bugs are trying to improve Emacs.  They are
not trying to make life miserable for the volunteers who
maintain Emacs and who are also, like the users who report
bugs, trying to improve Emacs.

If you don't have the time to consider a report carefully,
then please leave it open for now.  Closing bug reports
summarily, just to get the numbers down (?), is the kind of
misguided thing seen sometimes in non-free software circles.

Back to the report...  You dropped this:

  > And yet something about an "optional second argument
  > NEW-WINDOW", which is not even present in the lambda list.

What about that?  No doc bug?  And this:

  > Worse yet: It says "When called non-interactively",
  > suggesting that the function could be called interactively.
  > But it cannot - it is not a command.

No acknowledgment that I might have a point and there are
indeed some problems with this doc string, there at least.

Instead, a sermon that if I want help then I should not say
that this doc string is a "mess" and it is a "shame" to
summarily close such bugs instead of at least acknowledging
them or (better) making a minimal attempt to improve the doc.

It's perfectly understandable that such a bug is not regarded
as high-priority.  But if you recognize it as a bug then why
close it?

And if you do _not_ recognize that a doc string that talks
about NEW-WINDOW and calls the function a command, when
NEW-WINDOW is undefined wrt the parameter list and the
function is not interactive, then I disagree - it is a bug.

However insignificant one might think such a doc bug is, at
least one user (and I hardly use browse-url) thinks it is
important enough to report.

Whether you want to spend time looking into it or fixing it
is another question - I make no argument that this bug is
very important.  But why close it, if it is a bug?

You have not heard me complaining that this bug has not been
fixed.  My complaint is about it being closed summarily.
That's all.

bug#19421: 25.0.50; doc string of `browse-url' must describe parameter ARGS

[Eli Zaretskii]

> Date: Sat, 26 Dec 2015 08:40:36 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: larsi@gnus.org, 19421@debbugs.gnu.org
> 
> > If you looked at the
> > functions that can be invoked by browse-url, you know that they either
> > ignore ARGS or (in a few cases) use ARGS to pass the NEW-WINDOW flag,
> > in which case the corresponding function documents that.
> 
> If a given function that has an ARGS &rest parameter does nothing
> else with ARGS except pass it on to some other function, it is
> enough for the doc of the first function to say that - as usual.

I have now done that.

> Certainly the function's doc should say nothing about "NEW-WINDOW"
> unless either NEW-WINDOW is in the parameter list or the doc
> describes it in terms of parameters that are in the list (e.g.,
> as one of the members of argument-list ARGS).  The mention of
> NEW-WINDOW comes out of the blue and is incomprehensible to a
> user reading the doc string (this user, at least).

Fixed.

> > > As for `browse-url-default-browser', its doc string does not
> > > even have the lame excuse you used:
> > >   > The doc string says "Passes any ARGS to the browser function."
> > > It says nothing at all about ARGS.
> > 
> > Because it is just a dispatcher -- it invokes other functions,
> > which mostly ignore ARGS altogether.
> 
> Then that's what its doc should say: it passes ARGS to other
> functions (and name or otherwise specify what those functions
> are or can be).  And whether or not those other functions ignore
> ARGS is irrelevant to _this_ doc string for _this_ function.

Done.

> Back to the report...  You dropped this:
> 
>   > And yet something about an "optional second argument
>   > NEW-WINDOW", which is not even present in the lambda list.
> 
> What about that?  No doc bug?

Fixed.

> And this:
> 
>   > Worse yet: It says "When called non-interactively",
>   > suggesting that the function could be called interactively.
>   > But it cannot - it is not a command.
> 
> No acknowledgment that I might have a point and there are
> indeed some problems with this doc string, there at least.

The previous paragraph of the doc string describes the interactive
behavior:

  When called interactively, if variable `browse-url-new-window-flag' is
  non-nil, load the document in a new window, if possible, otherwise use
  a random existing one.  A non-nil interactive prefix argument reverses
  the effect of `browse-url-new-window-flag'.

So this part was already okay (in other functions as well).

bug#19421: 25.0.50; doc string of `browse-url' must describe parameter ARGS

[Drew Adams]

> I have now done that.
 
> Fixed.
 
> Done.
 
> Fixed.

Great.  Thank you for all of those changes, sight unseen.

> > And this:
> >
> >   > Worse yet: It says "When called non-interactively",
> >   > suggesting that the function could be called interactively.
> >   > But it cannot - it is not a command.
> >
> The previous paragraph of the doc string describes the interactive
> behavior:
> 
>   When called interactively, if variable `browse-url-new-window-flag' is
>   non-nil, load the document in a new window, if possible, otherwise use
>   a random existing one.  A non-nil interactive prefix argument reverses
>   the effect of `browse-url-new-window-flag'.
> 
> So this part was already okay (in other functions as well).

Sorry; I do not understand.  The doc string speaks about behavior
for when it is called interactively and when it is called
non-interactively.

I fail to see how it is called interactively, since it is not a
command - it has no `interactive' spec.  Are you perhaps counting
something like `M-: (browse-url-default-browser...)' as an
interactive call?  Normally that is not what the doc means by
"interactive".

Am I missing something?  The doc string still seems wacko, to me.
I admit that I have not studied all of the code in browse-url.el,
but I shouldn't have to, just to grasp what a doc string is saying.

This doc string talks about situations where the function is
called interactively, which I do not see as a possibility.

And it refers to an "optional second argument NEW-WINDOW", but
the lambda list is (URL &rest ARGS), and the doc string in no
way introduces NEW WINDOW in terms of those parameters.
Sure, users can figure out that NEW-WINDOW, if present, must be
the first element of list ARGS.  But it would be a lot clearer
if the doc string just talked about an optional second argument,
and said directly what its effect is.

I don't want to belabor this any more that we have already.  If
you really think this doc string is clear now, great.  It's not
so important that it be clear to me.  Thanks for having taken a
second look at it and having made the changes you made.

bug#19421: 25.0.50; doc string of `browse-url' must describe parameter ARGS

[Eli Zaretskii]

> Date: Sat, 26 Dec 2015 14:19:05 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: larsi@gnus.org, 19421@debbugs.gnu.org
> 
> >   When called interactively, if variable `browse-url-new-window-flag' is
> >   non-nil, load the document in a new window, if possible, otherwise use
> >   a random existing one.  A non-nil interactive prefix argument reverses
> >   the effect of `browse-url-new-window-flag'.
> > 
> > So this part was already okay (in other functions as well).
> 
> Sorry; I do not understand.  The doc string speaks about behavior
> for when it is called interactively and when it is called
> non-interactively.
> 
> I fail to see how it is called interactively, since it is not a
> command - it has no `interactive' spec.

It describes what will happen with the functions it calls, depending
on the platform.  Those functions do have an interactive spec.

> And it refers to an "optional second argument NEW-WINDOW", but
> the lambda list is (URL &rest ARGS), and the doc string in no
> way introduces NEW WINDOW in terms of those parameters.

Please look at the current sources, you are talking about stuff I
changed already.

bug#19421: 25.0.50; doc string of `browse-url' must describe parameter ARGS

[Drew Adams]

>> > I fail to see how it is called interactively, since it is not a
> > command - it has no `interactive' spec.
> 
> It describes what will happen with the functions it calls, depending
> on the platform.  Those functions do have an interactive spec.

That is something entirely different from "when called interactively".
Such language is quite misleading.  At least it misled and confused me.

And is this doc string really the place to describe that?  (I don't
say that it is not, but certainly such info is secondary to describing
this function and its own arguments.)

> > And it refers to an "optional second argument NEW-WINDOW", but
> > the lambda list is (URL &rest ARGS), and the doc string in no
> > way introduces NEW WINDOW in terms of those parameters.
> 
> Please look at the current sources, you are talking about stuff I
> changed already.

OK great.  Please ignore any such ignorance.  I was referring to
the state before your changes, which I have not seen.