bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Drew Adams]

1. At a minimum, the doc string of `isearch-mode' should say something
like this:
 
FORWARD non-nil means forward search; nil means backward search.
REGEXP t means regexp search; nil means literal search.
OP-FUN means ???????
RECURSIVE-EDIT non-nil means recursive edit for a modal search.
WORD-P non-nil means word search; nil means ignore word boundaries.
 
And you can remove this sentence from the doc string - a function's doc
should, in general, not mention callers:
 
"It is called by the function `isearch-forward' and other related
functions."
 
(Also, some `isearch-mode' arguments should be renamed with `-P', to
indicate that they are boolean flags: FORWARD-P, REGEXP-P,
RECURSIVE-EDIT-P.)
 
OP-FUN: It corresponds to `isearch-op-fun', but there is no doc string
for `isearch-op-fun', and the accompanying source comment does not help
- it says only when `isearch-op-fun' is called, not what it is for or
how it is used.
 

2. Doc strings of `isearch-forward' etc. also need to describe their
args.  E.g. 
 
Non-interactively:
REGEXP-P means...
NO-RECURSIVE-EDIT means...
 
Again, NO-RECURSIVE-EDIT should be NO-RECURSIVE-EDIT-P, to indicate that
it is its truth value that is used (nil/non-nil).
 

3. More generally, isearch.el needs more and better doc strings.
 

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2011-05-16 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.5) --no-opt --cflags
-Ic:/build/include'
 

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Stefan Monnier]

> (Also, some `isearch-mode' arguments should be renamed with `-P', to
> indicate that they are boolean flags: FORWARD-P, REGEXP-P,
> RECURSIVE-EDIT-P.)
 
No.


        Stefan

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Drew Adams]

> > (Also, some `isearch-mode' arguments should be renamed with `-P', to
> > indicate that they are boolean flags: FORWARD-P, REGEXP-P,
> > RECURSIVE-EDIT-P.)
>  
> No.

Yes, I know your argument that you want to distinguish Boolean functions (even
nullary) from Boolean-valued variables.  Still, it is a useful convention that
conveys information.

If you don't like `-P' because you think it too closely conveys "predicate" (and
you don't want to consider a variable as a nullary predicate), then choose
another naming convention for this.  It is quite helpful to suggest by a
parameter's name that it represents a truth value: nil/non-nil.

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Stefan Monnier]

> If you don't like `-P' because you think it too closely conveys
> "predicate" (and you don't want to consider a variable as a nullary
> predicate), then choose another naming convention for this.  It is
> quite helpful to suggest by a parameter's name that it represents
> a truth value: nil/non-nil.

`forward' works just fine, `recursive' as well.
I agree that `regexp' isn't quite as clear because it's a substantive so
it could also be understood to carry a regular expression, so you may
prefer `use-regexp' or something like that.


        Stefan

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Drew Adams]

> > If you don't like `-P' because you think it too closely conveys
> > "predicate" (and you don't want to consider a variable as a nullary
> > predicate), then choose another naming convention for this.  It is
> > quite helpful to suggest by a parameter's name that it represents
> > a truth value: nil/non-nil.
> 
> `forward' works just fine, `recursive' as well.
> I agree that `regexp' isn't quite as clear because it's a 
> substantive so
> it could also be understood to carry a regular expression, so you may
> prefer `use-regexp' or something like that.

No, the point was to have a convention that suggests the Boolean-valuedness
irrespective of the rest of the parameter name.

Anyway, this is not the most important part of this bug report.

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Lars Magne Ingebrigtsen]

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

> 1. At a minimum, the doc string of `isearch-mode' should say something
> like this:
>
> FORWARD non-nil means forward search; nil means backward search.
> REGEXP t means regexp search; nil means literal search.
> OP-FUN means ???????
> RECURSIVE-EDIT non-nil means recursive edit for a modal search.
> WORD-P non-nil means word search; nil means ignore word boundaries.

It's an internal function.  If you're not an isearch hacker, you'll
never see it, and if you are, you will know what those parameters mean.
The only thing that's useful in that doc string is the thing that's
already there, and which you want removed:

> And you can remove this sentence from the doc string - a function's doc
> should, in general, not mention callers:
>
> "It is called by the function `isearch-forward' and other related
> functions."

> 2. Doc strings of `isearch-forward' etc. also need to describe their
> args.  E.g. 
>
> Non-interactively:
> REGEXP-P means...
> NO-RECURSIVE-EDIT means...

I think any reasonable person would guess that REGEXP-P means "it's a
regexp", and NO-RECURSIVE-EDIT means "we're not in a recursive edit".

So I'm closing this report.

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

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Drew Adams]

> It's an internal function.

"Internal" functions too deserve doc strings, in general.

And nothing in Emacs is truly "internal".  Emacs purposefully documents itself,
at all levels.  That documentation is available to all users interactively.

Skip creating reasonable doc strings and you stab Emacs, the "self-documenting
editor", in the back.

It was only in the Dark Ages, when doc strings were expensive because hardware
was expensive, that we used comments instead of doc strings for many "internal"
functions.

We should not be lazy and cop out wrt "internal" functions.  There is no strict
separation in Emacs (or GNU generally) between users and developers, between
"internal" code and external use of that code.

> The only thing that's useful in that doc string is the thing that's
> already there, and which you want removed:
> 
> > And you can remove this sentence from the doc string - a 
> > function's doc should, in general, not mention callers:
> > "It is called by the function `isearch-forward' and other related
> > functions."

No, that is not "useful".  More importantly, it is generally a bad idea for a
callee to call out who calls it.  There are exceptional cases, but this is not
one.

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Juanma Barranquero]

On Fri, Jul 15, 2011 at 17:06, Drew Adams <drew.adams@oracle.com> wrote:

> "Internal" functions too deserve doc strings, in general.
>
> And nothing in Emacs is truly "internal".  Emacs purposefully documents itself,
> at all levels.  That documentation is available to all users interactively.

Of course there are internal elisp functions. Many of them even use a
convention of having two consecutive dashes in the name to mark that
they are internal, but not all.

And what makes them internal is that they are an implementation detail
and could be removed, or changed beyond all recognition, at any
moment. Documenting them past the simple "This is an internal
function" leads the user-developer to falsely believe that they are
part of the interface, but they are not. Otherwise, nothing could ever
be changed but painstakingly and with lot of ugly compatibility code.
Which is fine for the advertised interfaces, but not for every single
function in every single package in lisp/**/*.el.

First someone asks to document them all, then s/he will use them in
some package, and finally s/he will complain harshly when they are
deleted in some future release. Better to say clearly: don't do that,
or do it under you own responsability, after looking at the code, and
knowing full well that if you do you'll likely need to workaround
changes in the future.

(I'm not talking about the specific changes in this bug thread, but
comments like "Nothing in Emacs is truly 'internal'" cannot be let
pass unchallenged...)

    Juanma

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Drew Adams]

> > "Internal" functions too deserve doc strings, in general.
> >
> > And nothing in Emacs is truly "internal".  Emacs 
> > purposefully documents itself, at all levels.  That documentation
> > is available to all users interactively.
> 
> Of course there are internal elisp functions. Many of them even use a
> convention of having two consecutive dashes in the name to mark that
> they are internal, but not all.
> 
> And what makes them internal is that they are an implementation detail
> and could be removed, or changed beyond all recognition, at any
> moment. Documenting them past the simple "This is an internal
> function" leads the user-developer to falsely believe that they are
> part of the interface, but they are not. Otherwise, nothing could ever
> be changed but painstakingly and with lot of ugly compatibility code.
> Which is fine for the advertised interfaces, but not for every single
> function in every single package in lisp/**/*.el.
> 
> First someone asks to document them all, then s/he will use them in
> some package, and finally s/he will complain harshly when they are
> deleted in some future release. Better to say clearly: don't do that,
> or do it under you own responsability, after looking at the code, and
> knowing full well that if you do you'll likely need to workaround
> changes in the future.

I actually agree with most of what you wrote, Juanma.  And well put.

I nevertheless maintain that nothing in Emacs is "_truly_" internal.
And that even "internal" functions deserve doc strings, in general.

Nothing is truly internal because Emacs is open.  More than that - it is
intentionally, proudly, exceptionally, emphatically, in-your-face open.  Emacs
_wants_ to expose itself, at all levels, to _anyone_ interested.  It is the
self-documenting editor.  There is no rigid developer/user dichotomy.

This is the essential contribution of Emacs (and GNU).  We say "self-documenting
AND customizable/modifiable", but the former - more generally open source code,
implies the latter (modifiable).  This is to be contrasted with non-free
software, where everything that is not advertised via an external API _is_ truly
internal.

That nothing is truly internal in Emacs does not preclude those responsible for
the Emacs mainline development from wanting to let users know that function XYZ
is "internal" in the sense that it is better for them not to count on the
current behavior, or the implementation, remaining the same in the future.

That is orthogonal to doc, in general (and please note when I say "in general",
like "truly").  It is not because we want to convey the message that a function
or variable is internal in the sense that it might easily be changed, that we
should not, in general, document that function or variable.

Emacs code is open.  Doc strings are like easy-to-get-to code comments - and
intentionally so.  Instead of digging into the source code to see what a
function's parameters and purpose/effect/use are, you can just hit `C-h f'.

But not all code comments, of course.  Only those comments that describe what a
thing is for and what its interface (e.g. signature) is.  Information about
implementation, which can be found in some code comments, is generally not
appropriate for doc strings.  That is another part of the raison d'etre of doc
strings: describe just the API and purpose/effect/use, not the implementation.

And not documenting some internal function does _not_ obviate the potential
problem you cite, of users using it and unwisely expecting it not to change etc.
On the contrary, documentation can, in addition to describing the parameters and
purpose, explicitly point out that the function is likely to be changed easily.
Or as you put it:

> Better to say clearly: don't do that, or do it under you own
> responsability, after looking at the code, and knowing full
> well that if you do you'll likely need to workaround
> changes in the future.

Almost right, but it is not either/or.  What's wrong is your "Better to".
Better to document the function AND say that it might well change ("internal").
This is open software - we don't win by trying to hide things or get people to
not notice them.

Ignorance or misunderstanding about a function or variable is no Chinese wall
against misuse of it.  Quite the contrary.

> (I'm not talking about the specific changes in this bug thread

Which is what we should get back to - specifics about _these_ doc strings.

Still, the general discussion can be useful, since "internal" seems to be the
excuse more and more for not documenting things correctly.  No sense fixing an
incorrect statement, since a function is "internal".  No sense documenting a
function at all, since it is "internal".

There are a wide range of "internal" functions and variables.  In general, we
should improve their doc and, in some cases, the code comments.

An excuse such as `no user should care about this unless they are hacking the
Isearch code' ("if you're not an isearch hacker, you'll never see it") is
misguided.  _Any_ user potentially hacks Isearch code.  Not in the distributed
Emacs sources (isearch.el) perhaps, but in user code.

The goal should not be to discourage Emacs users from borrowing, adapting,
hacking the Isearch code or any other Emacs source code.  Quite the contrary.
This is what Emacs is about.  It is the self-documenting, customizable editor.
It is fine to let users know that a given function might change.  It is not fine
to try to discourage its use or reuse by obfuscating it or not helping to make
clear what it does.  It's just code, and code wants to be copied, changed, and
reused.

Isearch should make itself clear to users of all stripes, facilitating their
use, reuse, adaptation, and (who knows?) improvement or extension.  Just as we
should make the code clear to facilitate the job of Emacs Dev maintainers, so we
should make it clear for Emacs users.  And users are a larger and more varied
group than Emacs Dev.  They deserve clear, well commented, well documented code.

This is regardless of the internal needs of Emacs Dev itself.  It's not about
the bureaucracy (volunteer or not, and I include myself here) that serves the
people - it's about serving the people.  It's not (only or mainly) about making
life easier for Emacs Dev.  That's only a subsidiary requirement/goal.

Yes, there are reasons for some functions to be more or less internal, subject
to unplanned implementation/interface change, etc.  There are not often reasons
to not document what a function does and what its parameters are.

Emacs source code is for everyone.  When writing it, including its doc, the
target should be Emacs users, not just the code author or the relatively small
group of Emacs Dev maintainers.  Yes, this is a radically different perspective
from non-free software development.  We have met the user, and s?he is us - and
more.

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Drew Adams]

Reopening.  None of the issues raised in the bug report have been addressed.
The thread was simply side-tracked by Stefan concentrating on suggested
parameter names.  Then along comes Lars and just closes the bug, apparently not
reading the report or paying no attention to what it says.

The bug report is not about the parameter names used - there were only
parenthetical remarks about the names.  Call the parameters whatever you like.

The bug is about undocumented parameters, poorly documented parameters,
inappropriate statements about calling functions, missing doc, and inadequate
doc.  Please read the report.  This should be a no-brainer.  I even suggested
text for some of the individual parameter descriptions.

Doc for a function needs to describe its parameters and say what it does.  In
many cases it also needs to describe the return value.  There is nothing new
about this.

> 1. At a minimum, the doc string of `isearch-mode' should say something
> like this:
>  
> FORWARD non-nil means forward search; nil means backward search.
> REGEXP t means regexp search; nil means literal search.
> OP-FUN means ???????
> RECURSIVE-EDIT non-nil means recursive edit for a modal search.
> WORD-P non-nil means word search; nil means ignore word boundaries.
>  
> And you can remove this sentence from the doc string - a 
> function's doc should, in general, not mention callers:
>  
> "It is called by the function `isearch-forward' and other related
> functions."
 
> OP-FUN: It corresponds to `isearch-op-fun', but there is no doc
> string for `isearch-op-fun', and the accompanying source comment 
> does not help - it says only when `isearch-op-fun' is called, not
> what it is for or how it is used.

 
> 2. Doc strings of `isearch-forward' etc. also need to describe
> their args.  E.g. 
>  
> Non-interactively:
> REGEXP-P means...
> NO-RECURSIVE-EDIT means...


> 3. More generally, isearch.el needs more and better doc strings.

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Lars Magne Ingebrigtsen]

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

> Reopening.  None of the issues raised in the bug report have been
> addressed.  The thread was simply side-tracked by Stefan concentrating
> on suggested parameter names.  Then along comes Lars and just closes
> the bug, apparently not reading the report or paying no attention to
> what it says.

I did read the report, and I disagreed that this was something that
should be documented.

Please don't reopen reports that I have closed.  If the Emacs
maintainers disagree about the closing, they can open it again.

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

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Lars Magne Ingebrigtsen]

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

> Reopening.  None of the issues raised in the bug report have been
> addressed.  The thread was simply side-tracked by Stefan concentrating
> on suggested parameter names.  Then along comes Lars and just closes
> the bug, apparently not reading the report or paying no attention to
> what it says.

I did read the report, and I disagreed that this was something that
should be documented.

Please don't reopen reports that I have closed.  If the Emacs
maintainers disagree about the closing, they can open it again.

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

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Stefan Monnier]

> The bug is about undocumented parameters, poorly documented parameters,

Sure.  Please send us a suggested patch,


        Stefan

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Drew Adams]

> > The bug is about undocumented parameters, poorly documented 
> > parameters,
> 
> Sure.  Please send us a suggested patch,

I don't know what the info about the functions and parameters etc. should be.
That's why I'm asking for real doc strings.

Someone familiar with this code should be able to supply the necessary info.  If
you then need help with wording (unlikely), I can try to help.  It is the info
content that is missing/incomplete/unclear.

bug#8682: 24.0.50; doc strings of `isearch-mode', `isearch-forward', etc.

[Juri Linkov]

Version: 24.3.50

This was fixed recently in bug#13923, so I'm closing this report.