bug#8534: 24.0.50; Documentation bug in ns-read-file-name

bug#8534: 24.0.50; Documentation bug in ns-read-file-name

[Harald Hanche-Olsen]

The doc string for ns-read-file-name (defined in src/nsfns.m) says

Optional arg ISLOAD, if non-nil, means read a file name for saving.

However the exact opposite is true, as the name of the variable also
indicates.

On a side note, when the function is called (with ISLOAD = nil) and
you select an existing file, the usual warning pane about overwriting
an existing file pops up briefly before the dialog vanishes and the
function returns the chosen filename. It is not clear to me if this is
the intended behaviour, but I find it confusing. It would have been
better, I think, to let the save dialog do its usual thing, and then
for the caller of ns-read-file-name to assume that the user has
authorized the overwriting of a pre-existing file with the same name.

In GNU Emacs 24.0.50.1 (x86_64-apple-darwin10.7.0, NS apple-appkit-1038.35)
 of 2011-04-21 on mack
Windowing system distributor `Apple', version 10.3.1038
configured using `configure  '--with-ns''

- Harald

bug#8534: 24.0.50; Documentation bug in ns-read-file-name

[Chong Yidong]

Harald Hanche-Olsen <hanche@math.ntnu.no> writes:

> The doc string for ns-read-file-name (defined in src/nsfns.m) says
>
> Optional arg ISLOAD, if non-nil, means read a file name for saving.
>
> However the exact opposite is true, as the name of the variable also
> indicates.

I can patch up the docstring if you explain to me what the opposite of
"read a file name for saving" is, and what is the difference between the
two kinds of file dialogs (I don't use OSX).

bug#8534: 24.0.50; Documentation bug in ns-read-file-name

[Eli Zaretskii]

> X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00
> 	autolearn=unavailable version=3.3.1
> Resent-From: Chong Yidong <cyd@stupidchicken.com>
> Original-Sender: debbugs-submit-bounces@debbugs.gnu.org
> Resent-To: owner@debbugs.gnu.org
> Resent-CC: bug-gnu-emacs@gnu.org
> Resent-Sender: help-debbugs@gnu.org
> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Sun, 24 Apr 2011 13:51:21 -0400
> Cc: 8534@debbugs.gnu.org
> 
> Harald Hanche-Olsen <hanche@math.ntnu.no> writes:
> 
> > The doc string for ns-read-file-name (defined in src/nsfns.m) says
> >
> > Optional arg ISLOAD, if non-nil, means read a file name for saving.
> >
> > However the exact opposite is true, as the name of the variable also
> > indicates.
> 
> I can patch up the docstring if you explain to me what the opposite of
> "read a file name for saving" is, and what is the difference between the
> two kinds of file dialogs (I don't use OSX).

I guess it means that "non-nil" is backwards, as this part of
ns-read-file-name shows:

  panel = NILP (isLoad) ?
    (id)[EmacsSavePanel savePanel] : (id)[EmacsOpenPanel openPanel];

This seems to say that if isLoad is non-nil, we pop up a File Open
dialog, while when it's nil, we pop up a File Save dialog.

bug#8534: 24.0.50; Documentation bug in ns-read-file-name

[Harald Hanche-Olsen]

[Eli Zaretskii <eliz@gnu.org> (2011-04-24 18:28:42 UTC)]

> > I can patch up the docstring if you explain to me what the opposite of
> > "read a file name for saving" is, and what is the difference between the
> > two kinds of file dialogs (I don't use OSX).
> 
> I guess it means that "non-nil" is backwards, as this part of
> ns-read-file-name shows:
> 
>   panel = NILP (isLoad) ?
>     (id)[EmacsSavePanel savePanel] : (id)[EmacsOpenPanel openPanel];
> 
> This seems to say that if isLoad is non-nil, we pop up a File Open
> dialog, while when it's nil, we pop up a File Save dialog.

That is exactly correct. And the former dialog only lets you select an
existing file, while the latter lets you navigate to a folder and type
a filename.

- Harald

bug#8534: 24.0.50; Documentation bug in ns-read-file-name

[Chong Yidong]

Harald Hanche-Olsen <hanche@math.ntnu.no> writes:

>>   panel = NILP (isLoad) ?
>>     (id)[EmacsSavePanel savePanel] : (id)[EmacsOpenPanel openPanel];
>>
>> This seems to say that if isLoad is non-nil, we pop up a File Open
>> dialog, while when it's nil, we pop up a File Save dialog.
>
> That is exactly correct. And the former dialog only lets you select an
> existing file, while the latter lets you navigate to a folder and type
> a filename.

OK.  Does the following patch make sense?  It renames `isLoad' to
`mustmatch', to be like x-file-dialog.

(It sure would have been nice if ns-read-file-name had been named
x-file-dialog, and given the same arguments.  If anyone with access to
Mac OS wants to clean it up properly.)

=== modified file 'src/nsfns.m'
*** src/nsfns.m	2011-04-07 03:34:05 +0000
--- src/nsfns.m	2011-04-24 22:46:40 +0000
***************
*** 1416,1424 ****
  DEFUN ("ns-read-file-name", Fns_read_file_name, Sns_read_file_name, 1, 4, 0,
         doc: /* Use a graphical panel to read a file name, using prompt PROMPT.
  Optional arg DIR, if non-nil, supplies a default directory.
! Optional arg ISLOAD, if non-nil, means read a file name for saving.
  Optional arg INIT, if non-nil, provides a default file name to use.  */)
!      (Lisp_Object prompt, Lisp_Object dir, Lisp_Object isLoad, Lisp_Object init)
  {
    static id fileDelegate = nil;
    int ret;
--- 1416,1425 ----
  DEFUN ("ns-read-file-name", Fns_read_file_name, Sns_read_file_name, 1, 4, 0,
         doc: /* Use a graphical panel to read a file name, using prompt PROMPT.
  Optional arg DIR, if non-nil, supplies a default directory.
! Optional arg MUSTMATCH, if non-nil, means the returned file or
! directory must exist.
  Optional arg INIT, if non-nil, provides a default file name to use.  */)
!      (Lisp_Object prompt, Lisp_Object dir, Lisp_Object mustmatch, Lisp_Object init)
  {
    static id fileDelegate = nil;
    int ret;
***************
*** 1443,1449 ****
    if ([dirS characterAtIndex: 0] == '~')
      dirS = [dirS stringByExpandingTildeInPath];
  
!   panel = NILP (isLoad) ?
      (id)[EmacsSavePanel savePanel] : (id)[EmacsOpenPanel openPanel];
  
    [panel setTitle: promptS];
--- 1444,1450 ----
    if ([dirS characterAtIndex: 0] == '~')
      dirS = [dirS stringByExpandingTildeInPath];
  
!   panel = NILP (mustmatch) ?
      (id)[EmacsSavePanel savePanel] : (id)[EmacsOpenPanel openPanel];
  
    [panel setTitle: promptS];
***************
*** 1457,1463 ****
  
    panelOK = 0;
    BLOCK_INPUT;
!   if (NILP (isLoad))
      {
        ret = [panel runModalForDirectory: dirS file: initS];
      }
--- 1458,1464 ----
  
    panelOK = 0;
    BLOCK_INPUT;
!   if (NILP (mustmatch))
      {
        ret = [panel runModalForDirectory: dirS file: initS];
      }

bug#8534: 24.0.50; Documentation bug in ns-read-file-name

[Harald Hanche-Olsen]

[Chong Yidong <cyd@stupidchicken.com> (2011-04-24 22:52:39 UTC)]

> OK.  Does the following patch make sense?  It renames `isLoad' to
> `mustmatch', to be like x-file-dialog.

Yes, I think that is good.

> (It sure would have been nice if ns-read-file-name had been named
> x-file-dialog, and given the same arguments.  If anyone with access to
> Mac OS wants to clean it up properly.)

I have access, but lack the Objective-C-fu (for the time being, anyhow).

- Harald

bug#8534: 24.0.50; Documentation bug in ns-read-file-name

[Stefan Monnier]

> (It sure would have been nice if ns-read-file-name had been named
> x-file-dialog, and given the same arguments.  If anyone with access to

No, it should be ns-file-dialog.
I know we have a lot of "x-" used as "generic GUI functions", but that's
a bad habit which we should aim to fix.


        Stefan

bug#8534: 24.0.50; Documentation bug in ns-read-file-name

[Chong Yidong]

Harald Hanche-Olsen <hanche@math.ntnu.no> writes:

> [Chong Yidong <cyd@stupidchicken.com> (2011-04-24 22:52:39 UTC)]
>
>> OK.  Does the following patch make sense?  It renames `isLoad' to
>> `mustmatch', to be like x-file-dialog.
>
> Yes, I think that is good.

OK, committed to the emacs-23 branch.