bug#56355: 29.0.50; Implement file-parent-directory

bug#56355: 29.0.50; Implement file-parent-directory

[daanturo]

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

A way to get the parent directory of a file.

This is inspired by f.el's f-dirname, but only returns the absolute
path, also doesn't ignore `file-name-handler-alist`, doesn't strip the
final "/".

-- 
Daanturo.

[-- Attachment #2: 0001-Define-file-parent-directory.patch --]
[-- Type: text/x-patch, Size: 2969 bytes --]

From 69e7863d95fca8fb979b0182560f02b243edf172 Mon Sep 17 00:00:00 2001
From: Daanturo <daanturo@gmail.com>
Date: Sat, 2 Jul 2022 17:55:57 +0700
Subject: [PATCH] Define file-parent-directory

Get parent directory of a file.
* doc/lispref/files.texi: Document the function.
* etc/NEWS: Add its entry.
* lisp/emacs-lisp/shortdoc.el: Add it to 'file-name' group.
* lisp/files.el: implementation
---
 doc/lispref/files.texi      |  6 ++++++
 etc/NEWS                    |  3 +++
 lisp/emacs-lisp/shortdoc.el |  4 ++++
 lisp/files.el               | 10 ++++++++++
 4 files changed, 23 insertions(+)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index ea8683a6d8..e1182dc191 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -2445,6 +2445,12 @@ Directory Names
 because it recognizes abbreviations even as part of the name.
 @end defun
 
+@defun file-parent-directory filename
+This function returns the parent directory of @var{filename}.  If
+@var{filename} is at the top-level: return nil.  @var{filename} can be
+relative to `default-directory'.
+@end defun
+
 @node File Name Expansion
 @subsection Functions that Expand Filenames
 @cindex expansion of file names
diff --git a/etc/NEWS b/etc/NEWS
index 30404cc13c..3bfebd6bb7 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -317,6 +317,9 @@ startup.  Previously, these functions ignored
 \f
 * Changes in Emacs 29.1
 
+** New function 'file-parent-directory'
+Get the parent directory of a file.
+
 ** New config variable 'syntax-wholeline-max' to reduce the cost of long lines.
 This variable is used by some operations (mostly syntax-propertization
 and font-locking) to treat lines longer than this variable as if they
diff --git a/lisp/emacs-lisp/shortdoc.el b/lisp/emacs-lisp/shortdoc.el
index c82aa3365c..68500c43d3 100644
--- a/lisp/emacs-lisp/shortdoc.el
+++ b/lisp/emacs-lisp/shortdoc.el
@@ -353,6 +353,10 @@ file-name
   (abbreviate-file-name
    :no-eval (abbreviate-file-name "/home/some-user")
    :eg-result "~some-user")
+  (file-parent-directory
+   :eval (file-parent-directory "~")
+   :eval (file-parent-directory "foo")
+   :eval (file-parent-directory "/"))
   "Quoted File Names"
   (file-name-quote
    :args (name)
diff --git a/lisp/files.el b/lisp/files.el
index 1295c24c93..289d92a9dc 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5145,6 +5145,16 @@ file-name-split
           (setq filename nil))))
     components))
 
+(defun file-parent-directory (filename)
+  "Return the parent directory of FILENAME.
+If FILENAME is at the top-level: return nil.  FILENAME can be
+relative to `default-directory'."
+  (let* ((parent (file-name-directory
+                  (directory-file-name (expand-file-name filename)))))
+    (if (file-equal-p parent filename)
+        nil
+      parent)))
+
 (defcustom make-backup-file-name-function
   #'make-backup-file-name--default-function
   "A function that `make-backup-file-name' uses to create backup file names.
-- 
2.37.0

bug#56355: 29.0.50; Implement file-parent-directory

[Eli Zaretskii]

> Date: Sat, 2 Jul 2022 18:06:01 +0700
> From: daanturo <daanturo@gmail.com>
> 
> A way to get the parent directory of a file.
> 
> This is inspired by f.el's f-dirname, but only returns the absolute
> path, also doesn't ignore `file-name-handler-alist`, doesn't strip the
> final "/".

Thanks, but it looks as a very thin wrapper around
file-name-directory, so I wonder whether we really need a separate
function fir this.

bug#56355: 29.0.50; Implement file-parent-directory

[daanturo]

On 7/2/22 18:54, Eli Zaretskii wrote:
> Thanks, but it looks as a very thin wrapper around
> file-name-directory, so I wonder whether we really need a separate
> function fir this.

file-name-directory doesn't work when the input has a trailing "/", such as "/home/" when we expect "/" as the return value.

Also this one handles "~" => "/home/".

-- 
Daanturo.

bug#56355: 29.0.50; Implement file-parent-directory

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, but it looks as a very thin wrapper around
> file-name-directory, so I wonder whether we really need a separate
> function fir this.

I think it looks like a handy utility function -- we have quite a few
instances of

  (file-name-directory (directory-file-name filename))

(possibly with an expand-file-name in there, too) and using
`file-parent-directory' would both express the intention of the code
better, and be less error prone (because many people forget the
`directory-file-name' in the first attempt).

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

bug#56355: 29.0.50; Implement file-parent-directory

[Drew Adams]

> > Thanks, but it looks as a very thin wrapper around
> > file-name-directory, so I wonder whether we really 
> > need a separate function fir this.

Need?  How easy is it for a user to find and figure
out that a combination of `file-name-directory',
`directory-file-name', and `expand-file-name' can
give you the parent directory name?

Maybe your "we" is focused mainly on Emacs developers?
Such a function is helpful for users more generally,
even if not for those with greater familiarity with
the available set of functions.

And yes, how to do this has been asked multiple times
by users.  Here's one such:

https://emacs.stackexchange.com/q/9554/105

> I think it looks like a handy utility function -- we have quite a few
> instances of (file-name-directory (directory-file-name filename))
> 
> (possibly with an expand-file-name in there, too) and using
> `file-parent-directory' would both express the intention of the code
> better, and be less error prone (because many people forget the
> `directory-file-name' in the first attempt).

I proposed it long, long ago.  This version, from
Dired+, allows an optional argument to return the
relative name, i.e., just the parent component.

(defun diredp-parent-dir (file &optional relativep)
  "Return the parent directory of FILE, or nil if none.
Optional arg RELATIVEP non-nil means return a relative name, that is,
just the parent component."
  (let ((parent  (file-name-directory
                   (directory-file-name (expand-file-name file))))
        relparent)
    (when relativep
      (setq relparent  (file-name-nondirectory
                         (directory-file-name parent))))
    (and (not (equal parent file))  (or relparent  parent))))

bug#56355: 29.0.50; Implement file-parent-directory

[daanturo]

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

Another revision: this time I check if the found parent is null before comparing
with the expanded file name by `file-equal-p` (haven't come to this case,
though). And return the relative parent when input is relative.

Current cases as in shortdoc.el:

    (file-parent-directory "/foo/bar")
    ⇒ "/foo/"
    (file-parent-directory "~")
    ⇒ "/home/"
    (file-parent-directory "/tmp/")
    ⇒ "/"
    (file-parent-directory "foo/bar")
    ⇒ "foo/"
    (file-parent-directory "foo")
    ⇒ "./"
    (file-parent-directory "/")
    ⇒ nil

Also, about the comparator to check if the input is already at top-level;
`file-equal-p` looks like the most comprehensive one, but it makes use of
`file-truename` which is reported to be slow; on the other hand for `f.el`,
`dired+.el`, just comparing two strings with `equal` is
sufficient.

So I wonder in this case, with both FILENAME and the found parent's names
expanded, we can opt to use `equal`/`string=` instead of `file-equal-p` for
better performance?

-- 
Daanturo.

[-- Attachment #2: 0001-Define-file-parent-directory.patch --]
[-- Type: text/x-patch, Size: 3386 bytes --]

From 19be132871d4eb6e12e632454d016726d0b48d8b Mon Sep 17 00:00:00 2001
From: Daanturo <daanturo@gmail.com>
Date: Sat, 2 Jul 2022 17:55:57 +0700
Subject: [PATCH] Define file-parent-directory

Get parent directory of a file.
* doc/lispref/files.texi: Document the function.
* etc/NEWS: Add its entry.
* lisp/emacs-lisp/shortdoc.el: Add it to 'file-name' group.
* lisp/files.el: implementation
---
 doc/lispref/files.texi      |  6 ++++++
 etc/NEWS                    |  3 +++
 lisp/emacs-lisp/shortdoc.el |  7 +++++++
 lisp/files.el               | 17 +++++++++++++++++
 4 files changed, 33 insertions(+)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index ea8683a6d8..437a3001d3 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -2445,6 +2445,12 @@ Directory Names
 because it recognizes abbreviations even as part of the name.
 @end defun
 
+@defun file-parent-directory filename
+This function returns the parent directory of @var{filename}.  If
+@var{filename} is at the top-level, return nil.  @var{filename} can be
+relative to `default-directory'.
+@end defun
+
 @node File Name Expansion
 @subsection Functions that Expand Filenames
 @cindex expansion of file names
diff --git a/etc/NEWS b/etc/NEWS
index 30404cc13c..3bfebd6bb7 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -317,6 +317,9 @@ startup.  Previously, these functions ignored
 \f
 * Changes in Emacs 29.1
 
+** New function 'file-parent-directory'
+Get the parent directory of a file.
+
 ** New config variable 'syntax-wholeline-max' to reduce the cost of long lines.
 This variable is used by some operations (mostly syntax-propertization
 and font-locking) to treat lines longer than this variable as if they
diff --git a/lisp/emacs-lisp/shortdoc.el b/lisp/emacs-lisp/shortdoc.el
index c82aa3365c..a410d6c2fb 100644
--- a/lisp/emacs-lisp/shortdoc.el
+++ b/lisp/emacs-lisp/shortdoc.el
@@ -353,6 +353,13 @@ file-name
   (abbreviate-file-name
    :no-eval (abbreviate-file-name "/home/some-user")
    :eg-result "~some-user")
+  (file-parent-directory
+   :eval (file-parent-directory "/foo/bar")
+   :eval (file-parent-directory "~")
+   :eval (file-parent-directory "/tmp/")
+   :eval (file-parent-directory "foo/bar")
+   :eval (file-parent-directory "foo")
+   :eval (file-parent-directory "/"))
   "Quoted File Names"
   (file-name-quote
    :args (name)
diff --git a/lisp/files.el b/lisp/files.el
index 1295c24c93..ccb3520d8c 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5145,6 +5145,23 @@ file-name-split
           (setq filename nil))))
     components))
 
+(defun file-parent-directory (filename)
+  "Return the parent directory of FILENAME.
+If FILENAME is at the top-level, return nil.  FILENAME can be
+relative to `default-directory'."
+  (let* ((expanded-filename (expand-file-name filename))
+         (parent (file-name-directory (directory-file-name expanded-filename))))
+    (cond
+     ;; filename is at top-level, therefore no parent
+     ((or (null parent)
+          (file-equal-p parent expanded-filename))
+      nil)
+     ;; filename is relative, return relative parent
+     ((not (file-name-absolute-p filename))
+      (file-relative-name parent))
+     (t
+      parent))))
+
 (defcustom make-backup-file-name-function
   #'make-backup-file-name--default-function
   "A function that `make-backup-file-name' uses to create backup file names.
-- 
2.37.0

bug#56355: 29.0.50; Implement file-parent-directory

[Lars Ingebrigtsen]

daanturo <daanturo@gmail.com> writes:

> So I wonder in this case, with both FILENAME and the found parent's names
> expanded, we can opt to use `equal`/`string=` instead of `file-equal-p` for
> better performance?

I think file-equal-p is safer here, so I've pushed your patch to Emacs
29.

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

bug#56355: 29.0.50; Implement file-parent-directory

[daanturo]

About the comparator again, I hope that we use equal instead of file-equal-p.

The former gives far better performance.

Let my-parent-directory be a version of file-parent-directory with file-equal-p
replaced by equal, both are natively compiled.

```
(length recentf-list)
=> 659

(benchmark 1 '(mapcar 'my-parent-directory recentf-list))
=> "Elapsed time: 0.006954s"

(benchmark 1 '(mapcar 'file-parent-directory recentf-list))
=> "Elapsed time: 0.230073s"

```

That's about a 33-time difference in speed.

Also it's not like that (file-name-directory (directory-file-name <>)) is
supposed to resolve symbolic/hard links anyway.

-- 
Daanturo.

bug#56355: 29.0.50; Implement file-parent-directory

[Lars Ingebrigtsen]

daanturo <daanturo@gmail.com> writes:

> About the comparator again, I hope that we use equal instead of file-equal-p.

OK; pushed to Emacs 29.

bug#56355: 29.0.50; Implement file-parent-directory

[daanturo]

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

In the case of a circular symlink, technically it's still correct to return
non-nil.

```
ls -l /tmp/foo

total 0
lrwxrwxrwx 1 me me 9 Sep  1 10:59 foo -> /tmp/foo/
```

In this case:
```
;;; because
(file-equal-p "/tmp/foo/foo/" "/tmp/foo/")
=> t

;;; therefore

(my-parent-directory "/tmp/foo/foo/") ; (the equal version)
=> "/tmp/foo/"

(file-parent-directory "/tmp/foo/foo/")
=> nil

```

Obviously, the `equal` version fits our expectation better, although that's a
mistake at the time of link creation, "/tmp/foo/foo"'s parent still exists and
not a nil value.

Not to mention `file-equal-p` needs an active TRAMP connection for remote files.

-- 
Daanturo.

[-- Attachment #2: 0001-Change-file-parent-directory-s-behavior.patch --]
[-- Type: text/x-patch, Size: 1039 bytes --]

From 76dcd1814afd2ae5b83b43790d54e351577d6249 Mon Sep 17 00:00:00 2001
From: Daanturo <daanturo@gmail.com>
Date: Thu, 1 Sep 2022 09:42:05 +0700
Subject: [PATCH] Change file-parent-directory's behavior

* lisp/files.el (file-parent-directory): use equal instead of
file-equal-p to compare the found parent and the child
---
 lisp/files.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/files.el b/lisp/files.el
index 740e09055b..b084dca8b7 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5196,7 +5196,9 @@ to `default-directory', and the result will also be relative."
     (cond
      ;; filename is at top-level, therefore no parent
      ((or (null parent)
-          (file-equal-p parent expanded-filename))
+          ;; `equal' is enough, we don't need to resolve symlinks here
+          ;; with `file-equal-p', also for performance
+          (equal parent expanded-filename))
       nil)
      ;; filename is relative, return relative parent
      ((not (file-name-absolute-p filename))
-- 
2.37.3

bug#56355: 29.0.50; Implement file-parent-directory

[Michael Albinus]

daanturo <daanturo@gmail.com> writes:

Hi,

> Not to mention `file-equal-p` needs an active TRAMP connection for remote files.

Not necessarily. If you let-bind non-essential to t, Tramp tolerates
inactive connections.

> Daanturo.

Best regards, Michael.