bug#10879: 24.0.93; doc for `find-file' et al is missing the return value

bug#10879: 24.0.93; doc for `find-file' et al is missing the return value

[Drew Adams]

There are several places in the Emacs source code where `find-file' and
similar commands are used programmatically - typically when defining
other commands, such as `find-file-read-only'.
 
Similarly, 3rd-party code can sometimes use such a command
programmatically, for similar reasons.
 
Code such as the definition of `find-file-read-only' uses the return
value of `find-file'.  The bug is that this return value is not
documented - it should be.
 
The return value is documented only for `find-file-noselect'.  It would
be sufficient for the `find-file' et al doc to say that the return value
of the function is the same as for `find-find-noselect'.

Please add this info to the doc strings and the Elisp manual
doc for the `find-file' family of commands.

In GNU Emacs 24.0.93.1 (i386-mingw-nt5.1.2600)
 of 2012-02-15 on MARVIN
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.6) --no-opt --enable-checking --cflags
 -ID:/devel/emacs/libs/libXpm-3.5.8/include
 -ID:/devel/emacs/libs/libXpm-3.5.8/src
 -ID:/devel/emacs/libs/libpng-dev_1.4.3-1/include
 -ID:/devel/emacs/libs/zlib-dev_1.2.5-2/include
 -ID:/devel/emacs/libs/giflib-4.1.4-1/include
 -ID:/devel/emacs/libs/jpeg-6b-4/include
 -ID:/devel/emacs/libs/tiff-3.8.2-1/include
 -ID:/devel/emacs/libs/gnutls-3.0.9/include'
 

bug#10879: 24.0.93; doc for `find-file' et al is missing the return value

[Chong Yidong]

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

> There are several places in the Emacs source code where `find-file' and
> similar commands are used programmatically - typically when defining
> other commands, such as `find-file-read-only'.
>  
> Similarly, 3rd-party code can sometimes use such a command
> programmatically, for similar reasons.
>  
> Code such as the definition of `find-file-read-only' uses the return
> value of `find-file'.  The bug is that this return value is not
> documented - it should be.

I think we've been through this before.  We don't want to have to commit
ourselves to crap like

  (defun foo ()
     "The return value is nil."
     ...
     (message "Some message we added later")
     nil)

If the return value of a function is not documented, don't rely on
it---doing so is a bug in your code.

bug#10879: 24.0.93; doc for `find-file' et al is missing the return value

[Drew Adams]

> > There are several places in the Emacs source code where 
> > `find-file' and similar commands are used programmatically
> > - typically when defining other commands, such as
> > `find-file-read-only'.
> >  
> > Similarly, 3rd-party code can sometimes use such a command
> > programmatically, for similar reasons.
> >  
> > Code such as the definition of `find-file-read-only' uses the return
> > value of `find-file'.  The bug is that this return value is not
> > documented - it should be.
> 
> I think we've been through this before.  We don't want to 
> have to commit ourselves to crap like
> 
>   (defun foo ()
>      "The return value is nil."
>      ...
>      (message "Some message we added later")
>      nil)
> 
> If the return value of a function is not documented, don't rely on
> it---doing so is a bug in your code.

Maybe you didn't notice the part about the Emacs sources doing this - and why
they do so?  

The same context in which Emacs source code might do such a thing applies to
3rd-party code.  The same reasons apply.

Or are you arguing that there are never any such reasons, and that all such
occurrences in the Emacs source code are "crap" and bugs?  If so, then please
change this bug to a code bug from a doc bug.

I don't think that the return value of these particular functions (yes,
commands) is fortuitous.  Do you?  Would you like to change the code of these
particular commands so they do not take advantage of the return value?  Even
mapping over it when a list of buffers is returned due to wildcards?

You cannot legitimately have it both ways: it is useful for Emacs source coders
but not for 3rd-party coders.  Either it is "crap" for both or useful for both.
In the former case, please correct the source code.  In the latter case, please
document the return value.

bug#10879: 24.0.93; doc for `find-file' et al is missing the return value

[Stefan Monnier]

> but not for 3rd-party coders.  Either it is "crap" for both or useful
> for both.

It's crap.

> In the former case, please correct the source code.

Patches welcome,
Even better if you install them yourself,


        Stefan

bug#10879: 24.0.93; doc for `find-file' et al is missing the return value

[Drew Adams]

> > but not for 3rd-party coders.  Either it is "crap" for both 
> > or useful for both.
> 
> It's crap.

Interesting view.  I guess the original Emacs coders were crap, then, since
*each* of the `find-file-*' commands makes a point of returning the buffer(s).
And this has been true since at least Emacs 20.

And each time the code was rewritten/updated, the maintainers again made sure to
return the buffer(s).  Not just some inadvertent mistake, this crap.

Here, for instance, is `find-file-read-only' for Emacs 20 & Emacs 24.  The other
`find-file-*' commands are all similar.

20:

(defun find-file-read-only (filename &optional wildcards)
  "Edit file FILENAME but don't allow changes.
Like \\[find-file] but marks buffer as read-only.
Use \\[toggle-read-only] to permit editing."
  (interactive "fFind file read-only: \np")
  (find-file filename wildcards)
  (toggle-read-only 1)
  (current-buffer)) ; <==== make sure we return the buffer

24:

(defun find-file-read-only (filename &optional wildcards)
  "Edit file FILENAME but don't allow changes.
Like \\[find-file], but marks buffer as read-only.
Use \\[toggle-read-only] to permit editing."
  (interactive
   (find-file-read-args "Find file read-only: "
                        (confirm-nonexistent-file-or-buffer)))
  (unless (or (and wildcards find-file-wildcards
		   (not (string-match "\\`/:" filename))
		   (string-match "[[*?]" filename))
	      (file-exists-p filename))
    (error "%s does not exist" filename))
  (let ((value (find-file filename wildcards))) ; <=== use return
    (mapc (lambda (b) (with-current-buffer b (toggle-read-only 1)))
	  (if (listp value) value (list value)))
    value)) ; <============ make sure we return the buffer

In the latter, the function not only takes pains to return the buffer(s).  It
also takes advantage of the fact that `find-file' does likewise.

Maybe you also think it's crap for `switch-to-buffer' & compagnie to return the
buffer, and that mention of that fact should be removed from the doc?

bug#10879: 24.0.93; doc for `find-file' et al is missing the return value

[Lars Ingebrigtsen]

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

> And each time the code was rewritten/updated, the maintainers again made sure to
> return the buffer(s).  Not just some inadvertent mistake, this crap.

`find-file' now apparently returns either a list or a buffer now, so the
missing return value is correct.  Closing.

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

bug#10879: 24.0.93; doc for `find-file' et al is missing the return value

[Drew Adams]

> > And each time the code was rewritten/updated, the maintainers
> > again made sure to return the buffer(s).  Not just some inadvertent
> > mistake, this crap.
> 
> `find-file' now apparently returns either a list or a buffer now, so
> the missing return value is correct.  Closing.

Huh?  It has _always_ returned a list of buffers or a buffer.

That changes nothing.  Look at the doc string of `find-file-noselect'.
That's what returns the list or buffer.  And we document its return
value as that list or buffer.  As we should.  And as we should for
`find-file' et al.  

Nothing changed; nothing fixed.  Reopening.

bug#10879: 24.0.93; doc for `find-file' et al is missing the return value

[Stefan Monnier]

> And as we should for `find-file' et al.

No, find-file should basically never be called from Elisp, so there's no
point documenting its return value.


        Stefan