* bug#66542: Fix: locate-dominating-file predicate should receive dir not file
@ 2023-10-14 15:11 dalanicolai
2023-10-14 15:46 ` Eli Zaretskii
0 siblings, 1 reply; 5+ messages in thread
From: dalanicolai @ 2023-10-14 15:11 UTC (permalink / raw)
To: 66542
[-- Attachment #1.1: Type: text/plain, Size: 1153 bytes --]
Tags: patch
Tags: patch
The docstring of 'locate-dominating-file' mentions that its NAME argument
should be a dir, but currently it simply receives the FILE
argument. Therefore, using the function e.g. with the following predicate
for NAME
(lambda (dir)
(seq-filter (apply-partially #'string-match-p "paint")
(directory-files
dir)))
to check if a directory contains a file regexp-matching 'paint', throws
an error:
(file-error "Opening directory" "Not a directory" "/home/...")
This patch simply wraps the FILE argument in the (funcall NAME FILE) with
a 'file-name-directory' thereby fixing the function.
The commit log entry could read:
FIX: ensure that locate-dominating-file predicate recives dir
In GNU Emacs 29.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.38,
cairo version 1.17.8) of 2023-08-09 built on
2a02-a45d-af56-1-666c-72af-583a-b92d.fixed6.kpn.net
Repository revision: 31cef9a4eac01fff5ff4fcb89d7e2b7815e93bad
Repository branch: HEAD
System Description: Fedora Linux 38 (Workstation Edition)
Configured using:
'configure --with-tree-sitter --with-modules --with-cairo
--with-native-compilation --with-json --with-pgtk'
[-- Attachment #1.2: Type: text/html, Size: 1506 bytes --]
[-- Attachment #2: locate-dominating-file-patch --]
[-- Type: application/octet-stream, Size: 590 bytes --]
diff --git a/lisp/files.el b/lisp/files.el
index e1421b403bf..19c88e5bc8a 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1138,7 +1138,7 @@ locate-dominating-file
(setq try (if (stringp name)
(and (file-directory-p file)
(file-exists-p (expand-file-name name file)))
- (funcall name file)))
+ (funcall name (file-name-directory file))))
(cond (try (setq root file))
((equal file (setq file (file-name-directory
(directory-file-name file))))
^ permalink raw reply related [flat|nested] 5+ messages in thread* bug#66542: Fix: locate-dominating-file predicate should receive dir not file
2023-10-14 15:11 bug#66542: Fix: locate-dominating-file predicate should receive dir not file dalanicolai
@ 2023-10-14 15:46 ` Eli Zaretskii
2023-10-21 15:39 ` dalanicolai
0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2023-10-14 15:46 UTC (permalink / raw)
To: dalanicolai; +Cc: 66542
> From: dalanicolai <dalanicolai@gmail.com>
> Date: Sat, 14 Oct 2023 17:11:18 +0200
>
> The docstring of 'locate-dominating-file' mentions that its NAME argument
> should be a dir, but currently it simply receives the FILE
> argument. Therefore, using the function e.g. with the following predicate for NAME
>
> (lambda (dir)
> (seq-filter (apply-partially #'string-match-p "paint")
> (directory-files
> dir)))
>
> to check if a directory contains a file regexp-matching 'paint', throws
> an error:
>
> (file-error "Opening directory" "Not a directory" "/home/...")
>
> This patch simply wraps the FILE argument in the (funcall NAME FILE) with
> a 'file-name-directory' thereby fixing the function.
Sorry, I don't understand: the return value of file-name-directory is
not identical to its argument when the argument is a directory, so
this patch might change the behavior. Or what am I missing?
In addition, I don't think I understand the problem you are trying to
solve. Could you please describe the problem more completely, by
telling when and how was locate-dominating-file called with the name
of a file that is not a directory?
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#66542: Fix: locate-dominating-file predicate should receive dir not file
2023-10-14 15:46 ` Eli Zaretskii
@ 2023-10-21 15:39 ` dalanicolai
2023-10-25 13:15 ` Eli Zaretskii
0 siblings, 1 reply; 5+ messages in thread
From: dalanicolai @ 2023-10-21 15:39 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 66542
[-- Attachment #1.1: Type: text/plain, Size: 2198 bytes --]
You are right (of course :)
So I have attached another patch which only strips the file name (applies
file-name-directory) if the file is not a directory.
If the file is not a directory, then that stripped name should also be
returned by the function, i.e. in the 'cond`, root should be set to the
stripped file name. Therefore, the patched version simply 'checks and sets'
the file name first inside the setq.
Also, now we can remove the check in the first clause of the 'if' (of 'setq
try'), because 'file' is guaranteed to be a directory name, and the
existence of the directory name is already checked by the 'file-exists-p'.
This change does not affect other parts of the function (except that it
speeds it up a little, because it excludes the cycles, that only strip the
file names in case file is a 'real' file.
Indeed, the function should only check for 'parent directories' (not files).
I hope I did not miss anything.
On Sat, 14 Oct 2023 at 17:46, Eli Zaretskii <eliz@gnu.org> wrote:
> > From: dalanicolai <dalanicolai@gmail.com>
> > Date: Sat, 14 Oct 2023 17:11:18 +0200
> >
> > The docstring of 'locate-dominating-file' mentions that its NAME argument
> > should be a dir, but currently it simply receives the FILE
> > argument. Therefore, using the function e.g. with the following
> predicate for NAME
> >
> > (lambda (dir)
> > (seq-filter (apply-partially #'string-match-p "paint")
> > (directory-files
> > dir)))
> >
> > to check if a directory contains a file regexp-matching 'paint', throws
> > an error:
> >
> > (file-error "Opening directory" "Not a directory" "/home/...")
> >
> > This patch simply wraps the FILE argument in the (funcall NAME FILE) with
> > a 'file-name-directory' thereby fixing the function.
>
> Sorry, I don't understand: the return value of file-name-directory is
> not identical to its argument when the argument is a directory, so
> this patch might change the behavior. Or what am I missing?
>
> In addition, I don't think I understand the problem you are trying to
> solve. Could you please describe the problem more completely, by
> telling when and how was locate-dominating-file called with the name
> of a file that is not a directory?
>
[-- Attachment #1.2: Type: text/html, Size: 2966 bytes --]
[-- Attachment #2: locate-dominating-file-patch --]
[-- Type: application/octet-stream, Size: 819 bytes --]
diff --git a/lisp/files.el b/lisp/files.el
index e1421b403bf..4e3c9699852 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1135,9 +1135,11 @@ locate-dominating-file
(while (not (or root
(null file)
(string-match locate-dominating-stop-dir-regexp file)))
- (setq try (if (stringp name)
- (and (file-directory-p file)
- (file-exists-p (expand-file-name name file)))
+ (setq file (if (file-directory-p file)
+ file
+ (file-name-directory file))
+ try (if (stringp name)
+ (file-exists-p (expand-file-name name file))
(funcall name file)))
(cond (try (setq root file))
((equal file (setq file (file-name-directory
^ permalink raw reply related [flat|nested] 5+ messages in thread* bug#66542: Fix: locate-dominating-file predicate should receive dir not file
2023-10-21 15:39 ` dalanicolai
@ 2023-10-25 13:15 ` Eli Zaretskii
2023-10-26 13:27 ` dalanicolai
0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2023-10-25 13:15 UTC (permalink / raw)
To: dalanicolai; +Cc: 66542-done
> From: dalanicolai <dalanicolai@gmail.com>
> Date: Sat, 21 Oct 2023 17:39:17 +0200
> Cc: 66542@debbugs.gnu.org
>
> You are right (of course :)
> So I have attached another patch which only strips the file name (applies file-name-directory) if the file
> is not a directory.
>
> If the file is not a directory, then that stripped name should also be returned by the function, i.e. in the
> 'cond`, root should be set to the stripped file name. Therefore, the patched version simply 'checks
> and sets' the file name first inside the setq.
>
> Also, now we can remove the check in the first clause of the 'if' (of 'setq try'), because 'file' is
> guaranteed to be a directory name, and the existence of the directory name is already checked by
> the 'file-exists-p'.
>
> This change does not affect other parts of the function (except that it speeds it up a little, because it
> excludes the cycles, that only strip the file names in case file is a 'real' file.
> Indeed, the function should only check for 'parent directories' (not files).
Thanks, installed on master, and closing the bug.
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#66542: Fix: locate-dominating-file predicate should receive dir not file
2023-10-25 13:15 ` Eli Zaretskii
@ 2023-10-26 13:27 ` dalanicolai
0 siblings, 0 replies; 5+ messages in thread
From: dalanicolai @ 2023-10-26 13:27 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 66542-done
[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]
Thank you Eli!
On Wed, 25 Oct 2023 at 15:15, Eli Zaretskii <eliz@gnu.org> wrote:
> > From: dalanicolai <dalanicolai@gmail.com>
> > Date: Sat, 21 Oct 2023 17:39:17 +0200
> > Cc: 66542@debbugs.gnu.org
> >
> > You are right (of course :)
> > So I have attached another patch which only strips the file name
> (applies file-name-directory) if the file
> > is not a directory.
> >
> > If the file is not a directory, then that stripped name should also be
> returned by the function, i.e. in the
> > 'cond`, root should be set to the stripped file name. Therefore, the
> patched version simply 'checks
> > and sets' the file name first inside the setq.
> >
> > Also, now we can remove the check in the first clause of the 'if' (of
> 'setq try'), because 'file' is
> > guaranteed to be a directory name, and the existence of the directory
> name is already checked by
> > the 'file-exists-p'.
> >
> > This change does not affect other parts of the function (except that it
> speeds it up a little, because it
> > excludes the cycles, that only strip the file names in case file is a
> 'real' file.
> > Indeed, the function should only check for 'parent directories' (not
> files).
>
> Thanks, installed on master, and closing the bug.
>
[-- Attachment #2: Type: text/html, Size: 1800 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-10-26 13:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-14 15:11 bug#66542: Fix: locate-dominating-file predicate should receive dir not file dalanicolai
2023-10-14 15:46 ` Eli Zaretskii
2023-10-21 15:39 ` dalanicolai
2023-10-25 13:15 ` Eli Zaretskii
2023-10-26 13:27 ` dalanicolai
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).