Subject: bug#7159: closed (Re: bug#7159: 24.0.50; (1) `file-name-(non)directory': bad return values, (2) `directory-sep-char')

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 04 Oct 2010 19:27:02 +0000

Your bug report

#7159: 24.0.50; (1) `file-name-(non)directory': bad return values, (2) `directory-sep-char'

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
7159: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=7159
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#7159: 24.0.50; (1) `file-name-(non)directory': bad return values, (2) `directory-sep-char'
From: Eli Zaretskii <[email-masked]>
To: Drew Adams <[email-masked]>
Cc: [email-masked]
Date: Mon, 04 Oct 2010 21:29:40 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

> From: "Drew Adams" <[email-masked]>
> Date: Mon, 4 Oct 2010 10:59:12 -0700
> Cc: 
> 
> BUT:
>  
> (file-name-directory    titi) ; gives "c:/foo/bar/b[^^@]*\\.el\\"
> (file-name-nondirectory titi) ; gives "'"
>  
> These functions should know how to parse titi to produce "c:/foo/bar/"
> and "b[^^@]*\\.el\\'", respectively (where ^@ is the control char).

You are forgetting the backslashes that wildcard-to-regexp inserted.
On DOS and Windows, Emacs treats backslashes as directory separators,
as you'd expect.  So "c:/foo/bar/b[^^@]*\\.el\\" looks like a leading
directory of a file whose basename is "'".

In other words, don't pass a regexp with backslashes to these
functions, because you won't get what you think you will.

> It is not expected that these functions return names that necessarily
> map to actual directories or files.

And indeed, they don't.

> So I suspect that the `file-name-nondirectory' part of this bug
> is at least in part a Windows problem.  The code seems to be
> interpreting the backslash (?\) near the end as a directory
> separator.

It does, by design.

> If so, that is definitely wrong.  Even on Windows, the
> code should use the value of `directory-sep-char', which is ?/,
> not ?\.

On Windows, we support both, and we always will.  Anything else means
a terrible breakage, believe me.  For example, it would be very hard
to parse output of programs that emit file name with backslashes.
With the current setup, this is seamless, even if the file names use
mixed forward- and back-slashes (yes, it happens with GCC and GDB, for
example, or even with Make sometimes).

> However, I see from the doc string that `directory-sep-char' has
> been made obsolete:

In fact, just yesterday it was removed altogether, because it has not
effect on what Emacs does.  That's been like that for years, and we
saw no complains.

I'm closing this bug.

Subject: 24.0.50; (1) `file-name-(non)directory': bad return values, (2) `directory-sep-char'
From: "Drew Adams" <[email-masked]>
To: <[email-masked]>
Date: Mon, 4 Oct 2010 10:59:12 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

emacs -Q
 
1.

(setq toto (wildcard-to-regexp "c:/foo/bar/b*.el"))
 gives "\\`c:/foo/bar/b[^^@]*\\.el\\'" (where ^@ is a control char)
 
(setq titi (substring 2 toto))
 gives "c:/foo/bar/b[^^@]*\\.el\\'"  (^@ is a control char)
 
(file-name-absolute-p toto) ; -> t
(file-name-absolute-p titi) ; -> t
 
That is all as one would expect.  `file-name-absolute-p' has no
problem with either file-name string, even though neither string
is a legitimate file name and both contain a control char.
This is normal (IMO).
 
BUT:
 
(file-name-directory    titi) ; gives "c:/foo/bar/b[^^@]*\\.el\\"
(file-name-nondirectory titi) ; gives "'"
 
These functions should know how to parse titi to produce "c:/foo/bar/"
and "b[^^@]*\\.el\\'", respectively (where ^@ is the control char).
 
It is not expected that these functions return names that necessarily
map to actual directories or files.  What is expected is that they
remove the non-directory and directory components of the strings they
are passed.  That is not happening here.

Also:

(setq baz "c:/foo/bar/*\\.el\\'")
(file-name-nondirectory baz) ; gives "'"
(setq baz "c:/foo/bar/*\\.el\\ABC")
(file-name-nondirectory baz) ; gives "ABC"

So I suspect that the `file-name-nondirectory' part of this bug
is at least in part a Windows problem.  The code seems to be
interpreting the backslash (?\) near the end as a directory
separator.  If so, that is definitely wrong.  Even on Windows, the
code should use the value of `directory-sep-char', which is ?/,
not ?\.

2.

However, I see from the doc string that `directory-sep-char' has
been made obsolete:

 directory-sep-char is a variable defined in `subr.el'.
 Its value is 47

   This variable is obsolete since 21.1;
   do not use it, just use `/'.
   This variable is potentially risky when used as a file local variable.

 Documentation:
 Directory separator character for built-in functions that return file names.
 The value is always ?/. 

That seems misguided, and the buggy behavior noted above is a good
example of why.  The correct way to handle this would be to make
`directory-sep-char' a defconst with value ?/.  And code should always
use this named constant, NOT a literal ?/.  The bugged behavior here
shows why: someone coding  `file-name-nondirectory' seems to have
treated (hard-coded) ?\ as the directory separator on Windows (just a
guess).

Note too that the code has another minor bug: The call to
`make-obsolete-variable' (which should anyway be removed, and the
defvar simply replaced by a defconst) incorrectly uses "`/'" instead
of "?/".  The doc string itself is correct in referring to "?/".

(defconst directory-sep-char ?/
  "Directory separator character for built-in functions that return file names.
The value is always ?/.")
(make-obsolete-variable 'directory-sep-char "do not use it, just use `/'."
"21.1")
                                                                     ^^^

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600) of 2010-09-20 on 3249CTO
 Windowing system distributor `Microsoft Corp.', version 5.1.2600
 configured using `configure --with-gcc (4.4) --no-opt --cflags
 -Ic:/imagesupport/include'