bug#27982: 25.1.50; expand-file-name docstring on how to traverse the filesystem

bug#27982: 25.1.50; expand-file-name docstring on how to traverse the filesystem

[Nicolas Richard]

[-- Attachment #1: Type: text/plain, Size: 837 bytes --]

The docstring of expand-file-name suggest
(directory-file-name (file-name-directory dirname))
to traverse a directory

This looks wrong to me, e.g.
(list current-directory
      (directory-file-name (file-name-directory current-directory)))
 => ("/home/youngfrog/" "/home/youngfrog")

I think it should be (file-name-directory (directory-file-name dirname))
instead. Then the example becomes:

(list current-directory
      (file-name-directory (directory-file-name current-directory)))
 => ("/home/youngfrog/" "/home/")

Am I overlooking something ?
Is this patch ok to commit ?

In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu)
 of 2016-11-15 built on phie-fixe
Repository revision: f994c2046588b168c1a4a900879cdffaf9d02f01
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description:	Ubuntu 16.04.2 LTS


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-src-fileio.c-Fexpand_file_name-fix-suggestion-to-tra.patch --]
[-- Type: text/x-diff, Size: 959 bytes --]

From 607e1445cfb1294c14af38cb4c39a93683e5671d Mon Sep 17 00:00:00 2001
From: Nicolas Richard <youngfrog@members.fsf.org>
Date: Sun, 6 Aug 2017 13:14:41 +0200
Subject: [PATCH] * src/fileio.c (Fexpand_file_name): fix suggestion to
 traverse the filesystem

---
 src/fileio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/fileio.c b/src/fileio.c
index c3b2be7..28d619c 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -776,7 +776,7 @@ See also the function `substitute-in-file-name'.
 For technical reasons, this function can return correct but
 non-intuitive results for the root directory; for instance,
 \(expand-file-name ".." "/") returns "/..".  For this reason, use
-\(directory-file-name (file-name-directory dirname)) to traverse a
+\(file-name-directory (directory-file-name dirname)) to traverse a
 filesystem tree, not (expand-file-name ".."  dirname).  */)
   (Lisp_Object name, Lisp_Object default_directory)
 {
-- 
2.7.4

bug#27982: 25.1.50; expand-file-name docstring on how to traverse the filesystem

[Eli Zaretskii]

> From: Nicolas Richard <youngfrog@members.fsf.org>
> Date: Sun, 06 Aug 2017 13:38:32 +0200
> 
> The docstring of expand-file-name suggest
> (directory-file-name (file-name-directory dirname))
> to traverse a directory

No, it only suggests that as a more reliable alternative to

  (expand-file-name ".." dirname)

> This looks wrong to me, e.g.
> (list current-directory
>       (directory-file-name (file-name-directory current-directory)))
>  => ("/home/youngfrog/" "/home/youngfrog")

This is not the use case against which the doc string tries to warn,

Thanks.

bug#27982: 25.1.50; expand-file-name docstring on how to traverse the filesystem

[Nicolas Richard]

Hello,

Thanks for your comment.

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Nicolas Richard <youngfrog@members.fsf.org>
>> Date: Sun, 06 Aug 2017 13:38:32 +0200
>>
>> The docstring of expand-file-name suggest
>> (directory-file-name (file-name-directory dirname))
>> to traverse a directory
>
> No, it only suggests that as a more reliable alternative to
>
>   (expand-file-name ".." dirname)

If it's an alternative, shouldn't it have an equivalent effect, except
for cases where it's better? Or perhaps my english is lacking and I
misunderstand the suggestion completely.

Let me rephrase my example just to make sure I explained myself
correctly:

(let ((dirname current-directory))
   (format "%s versus %s"
      (directory-file-name (file-name-directory dirname))
      (expand-file-name ".." dirname)))
=> "/home/youngfrog versus /home"
=> Effect is clearly different.

With my suggestion this would become:

(let ((dirname current-directory))
   (format "%s versus %s"
      (file-name-directory (directory-file-name dirname))
      (expand-file-name ".." dirname)))
=> "/home/ versus /home"
=> Effect is somewhat similar


Nicolas.

bug#27982: 25.1.50; expand-file-name docstring on how to traverse the filesystem

[Eli Zaretskii]

> From: Nicolas Richard <youngfrog@members.fsf.org>
> Cc: 27982@debbugs.gnu.org
> Date: Wed, 23 Aug 2017 14:18:24 +0200
> 
> (let ((dirname current-directory))
>    (format "%s versus %s"
>       (directory-file-name (file-name-directory dirname))
>       (expand-file-name ".." dirname)))
> => "/home/youngfrog versus /home"
> => Effect is clearly different.

Crystal ball says that current-directory (which you probably meant to
be default-directory instead) ends in a slash, in which case remove
it, and the 2 results will match.

bug#27982: 25.1.50; expand-file-name docstring on how to traverse the filesystem

[Nicolas Richard]

Eli Zaretskii <eliz@gnu.org> writes:
> Crystal ball says that current-directory (which you probably meant to
> be default-directory instead) ends in a slash, in which case remove
> it, and the 2 results will match.

Your crystal ball is working fine. Sorry for leaving that detail out.

FWIW I have (defvaralias 'current-directory 'default-directory) because
I used to forget the name of the variable (and I still do, it seems).

Note however that default-directory ends in a slash when using "emacs
-Q" too.

Ok so now I'm saying that, with the suggestion in the docstring, output
will be different if "dirname" ends in a slash. IOW if it is a
"directory name" as opposed to "directory's file name".

(let ((dirname "/home/youngfrog/"))
   (format "%s versus %s"
      (directory-file-name (file-name-directory dirname))
      (expand-file-name ".." dirname)))

=> output is different

(let ((dirname "/home/youngfrog"))
   (format "%s versus %s"
      (directory-file-name (file-name-directory dirname))
      (expand-file-name ".." dirname)))

=> output the same

With the function calls swapped the output is always similar:

(let ((dirname "/home/youngfrog/"))
   (format "%s versus %s"
      (file-name-directory (directory-file-name dirname))
      (expand-file-name ".." dirname)))

=> output is similar

(let ((dirname "/home/youngfrog"))
   (format "%s versus %s"
      (file-name-directory (directory-file-name dirname))
      (expand-file-name ".." dirname)))

=> output is similar

Nicolas.

bug#27982: 25.1.50; expand-file-name docstring on how to traverse the filesystem

[Eli Zaretskii]

> From: Nicolas Richard <youngfrog@members.fsf.org>
> Cc: 27982@debbugs.gnu.org
> Date: Wed, 23 Aug 2017 21:18:53 +0200
> 
> Ok so now I'm saying that, with the suggestion in the docstring, output
> will be different if "dirname" ends in a slash.

Of course.  directory-file-name and file-name-directory look at the
slashes, so having one more slash at the end changes everything.
There's nothing new here.

So where do we go from here, in the context of this bug report?  What
would you propose to change, where, and why?

Thanks.

bug#27982: 25.1.50; expand-file-name docstring on how to traverse the filesystem

[Nicolas Richard]

Hi Eli,

Thanks for your help but let me close this.

I initially thought it was "obviously wrong", but it is not. I think the
wording is slightly misleading (e.g. "dirname" vs "dir-as-file-name"),
but nothing important.

Nicolas.

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Nicolas Richard <youngfrog@members.fsf.org>
>> Cc: 27982@debbugs.gnu.org
>> Date: Wed, 23 Aug 2017 21:18:53 +0200
>>
>> Ok so now I'm saying that, with the suggestion in the docstring, output
>> will be different if "dirname" ends in a slash.
>
> Of course.  directory-file-name and file-name-directory look at the
> slashes, so having one more slash at the end changes everything.
> There's nothing new here.
>
> So where do we go from here, in the context of this bug report?  What
> would you propose to change, where, and why?
>
> Thanks.


--

bug#27982: 25.1.50; expand-file-name docstring on how to traverse the filesystem

[Eli Zaretskii]

> From: Nicolas Richard <youngfrog@members.fsf.org>
> Cc: 27982-done@debbugs.gnu.org
> Date: Wed, 30 Aug 2017 17:43:51 +0200
> 
> Thanks for your help but let me close this.
> 
> I initially thought it was "obviously wrong", but it is not. I think the
> wording is slightly misleading (e.g. "dirname" vs "dir-as-file-name"),
> but nothing important.

I added a note to the doc string to make sure the trailing-slash issue
is not overlooked.

Thanks.