* Re: [Emacs-diffs] master 3431e82: Ignore directory symlinks in directory-files-recursively
[not found] ` <E1XzNqL-000168-2E@vcs.savannah.gnu.org>
@ 2014-12-12 14:57 ` Stefan Monnier
2014-12-13 15:01 ` Lars Magne Ingebrigtsen
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2014-12-12 14:57 UTC (permalink / raw
To: emacs-devel; +Cc: Lars Magne Ingebrigtsen
> Ignore directory symlinks in directory-files-recursively
Could you work on merging file-tree-walk and
directory-files-recursively?
Stefan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] master 3431e82: Ignore directory symlinks in directory-files-recursively
2014-12-12 14:57 ` [Emacs-diffs] master 3431e82: Ignore directory symlinks in directory-files-recursively Stefan Monnier
@ 2014-12-13 15:01 ` Lars Magne Ingebrigtsen
2014-12-14 4:59 ` Stefan Monnier
0 siblings, 1 reply; 8+ messages in thread
From: Lars Magne Ingebrigtsen @ 2014-12-13 15:01 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Ignore directory symlinks in directory-files-recursively
>
> Could you work on merging file-tree-walk and
> directory-files-recursively?
As you pointed out, `file-tree-walk' has the nice property that it can
stop recursing based on a predicate, so implementing it on
`directory-files-recursively' would not be optimal.
On the other hand, the interface for `file-tree-walk' is not what people
usually want -- they just want a list of matching files. I grepped for
"defun.*recurs" and there's a lot of functions basically implementing
`directory-files-recursively'. Here are the first four:
cedet-files-list-recursively
find-lisp-find-files
gnus-recursive-directory-files
nnfolder-recursive-directory-files
Doing it the other way around would almost be possible, but
`directory-files-recursively' guarantees that leaves are returned before
directories so that implementing "rm -r" is just a `mapc' over the
returned values. And `file-tree-walk' doesn't call the callback
function in that order, so it's fiddly.
Besides, we don't have copyright paperwork for `file-tree-walk'.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] master 3431e82: Ignore directory symlinks in directory-files-recursively
2014-12-13 15:01 ` Lars Magne Ingebrigtsen
@ 2014-12-14 4:59 ` Stefan Monnier
2014-12-14 9:33 ` Lars Magne Ingebrigtsen
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2014-12-14 4:59 UTC (permalink / raw
To: Lars Magne Ingebrigtsen; +Cc: emacs-devel
> As you pointed out, `file-tree-walk' has the nice property that it can
> stop recursing based on a predicate, so implementing it on
> `directory-files-recursively' would not be optimal.
Right, what is needed is to extend directory-files-recursively such that
it can also use a predicate to decide whether to recurse or not.
Stefan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] master 3431e82: Ignore directory symlinks in directory-files-recursively
2014-12-14 4:59 ` Stefan Monnier
@ 2014-12-14 9:33 ` Lars Magne Ingebrigtsen
2014-12-14 14:27 ` Stefan Monnier
0 siblings, 1 reply; 8+ messages in thread
From: Lars Magne Ingebrigtsen @ 2014-12-14 9:33 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> As you pointed out, `file-tree-walk' has the nice property that it can
>> stop recursing based on a predicate, so implementing it on
>> `directory-files-recursively' would not be optimal.
>
> Right, what is needed is to extend directory-files-recursively such that
> it can also use a predicate to decide whether to recurse or not.
file-tree-walk uses the same function for the predicate and for the
... processing function. So that would be something like the patch
included below.
The interface isn't the same, though. I think we should perhaps ignore
the non-lexical case and get rid of the ARGS argument. Supplying a
lexical lambda predicate just makes more sense.
And I don't like passing in the DIR and LEAF separately. To check
whether the argument is a directory, the predicate can just use
`directory-name-p' .
diff --git a/lisp/files.el b/lisp/files.el
index d55ef1a..14be6c2 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -766,11 +766,17 @@ prevented. Directory entries are sorted with string-lessp."
(and (> (length name) 0)
(char-equal (aref name (1- (length name))) ?/)))
-(defun directory-files-recursively (dir match &optional include-directories)
+(defun directory-files-recursively (dir match &optional
+ include-directories predicate)
"Return all files under DIR that have file names matching MATCH (a regexp).
This function works recursively. Files are returned in \"depth first\"
and alphabetical order.
-If INCLUDE-DIRECTORIES, also include directories that have matching names."
+If INCLUDE-DIRECTORIES, also include directories that have matching names.
+
+If PREDICATE, this should be a function that will be called on
+each matching file and on all directories. When called on a
+directory, the predicate should return non-nil if the directory
+should be descended into."
(let ((result nil)
(files nil))
(dolist (file (sort (file-name-all-completions "" dir)
@@ -780,13 +786,17 @@ If INCLUDE-DIRECTORIES, also include directories that have matching names."
(let* ((leaf (substring file 0 (1- (length file))))
(path (expand-file-name leaf dir)))
;; Don't follow symlinks to other directories.
- (unless (file-symlink-p path)
+ (when (and (not (file-symlink-p path))
+ (or (null predicate)
+ (funcall predicate path)))
(setq result (nconc result (directory-files-recursively
path match include-directories))))
(when (and include-directories
(string-match match leaf))
(setq result (nconc result (list path)))))
(when (string-match match file)
+ (when predicate
+ (funcall predicate (expand-file-name file dir)))
(push (expand-file-name file dir) files)))))
(nconc result (nreverse files))))
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] master 3431e82: Ignore directory symlinks in directory-files-recursively
2014-12-14 9:33 ` Lars Magne Ingebrigtsen
@ 2014-12-14 14:27 ` Stefan Monnier
2014-12-21 11:57 ` Lars Ingebrigtsen
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2014-12-14 14:27 UTC (permalink / raw
To: Lars Magne Ingebrigtsen; +Cc: emacs-devel
> The interface isn't the same, though. I think we should perhaps ignore
> the non-lexical case and get rid of the ARGS argument. Supplying a
> lexical lambda predicate just makes more sense.
Agreed.
> And I don't like passing in the DIR and LEAF separately.
It can avoid calling expand-file-name only to call file-name-directory
(or file-name-nondirectory) right after.
> -(defun directory-files-recursively (dir match &optional include-directories)
> +(defun directory-files-recursively (dir match &optional
> + include-directories predicate)
Can we reduce the number of arguments?
> "Return all files under DIR that have file names matching MATCH (a regexp).
Please specify more precisely how MATCH is used (whether it's compared
to the "leaf" only, or the full absolute file name, or ...).
> (path (expand-file-name leaf dir)))
This is not a "path", it's a file name. Remember: by convention
a "path" is a list of directories, as in load-path, $PATH, etc...
Stefan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] master 3431e82: Ignore directory symlinks in directory-files-recursively
2014-12-14 14:27 ` Stefan Monnier
@ 2014-12-21 11:57 ` Lars Ingebrigtsen
2014-12-22 3:02 ` Stefan Monnier
0 siblings, 1 reply; 8+ messages in thread
From: Lars Ingebrigtsen @ 2014-12-21 11:57 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> And I don't like passing in the DIR and LEAF separately.
>
> It can avoid calling expand-file-name only to call file-name-directory
> (or file-name-nondirectory) right after.
It can, but in the use cases I've typically seen, you wouldn't do that,
I think? You'd say `(directory-name-p file)' and then... something...
>> -(defun directory-files-recursively (dir match &optional include-directories)
>> +(defun directory-files-recursively (dir match &optional
>> + include-directories predicate)
>
> Can we reduce the number of arguments?
That would be nice, but I'm not sure what to remove. The canonical,
that most of the callers use (from the other versions of this function
included in various packages in Emacs) is
(directory-files-recursively "~/" "~\\'")
or something. And then there are a couple that want the directories,
too. And then there's `file-tree-walk', which is the new thing.
Uhm... we could make MATCH be the predicate if it's not a string?
>> (path (expand-file-name leaf dir)))
>
> This is not a "path", it's a file name. Remember: by convention
> a "path" is a list of directories, as in load-path, $PATH, etc...
Right. But do we have a word for "string that designates either a file
or a directory"? That's usually called "a path"...
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog http://lars.ingebrigtsen.no/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] master 3431e82: Ignore directory symlinks in directory-files-recursively
2014-12-21 11:57 ` Lars Ingebrigtsen
@ 2014-12-22 3:02 ` Stefan Monnier
2015-01-16 0:08 ` Lars Magne Ingebrigtsen
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2014-12-22 3:02 UTC (permalink / raw
To: Lars Ingebrigtsen; +Cc: emacs-devel
>> It can avoid calling expand-file-name only to call file-name-directory
>> (or file-name-nondirectory) right after.
> It can, but in the use cases I've typically seen, you wouldn't do that,
> I think? You'd say `(directory-name-p file)' and then... something...
I don't understand what you mean here. I'm just pointing out that
calling expand-file-name only to pass the result to the function
parameter means that this work may be wasted.
So passing the two parameters separately is a way to be more lazy.
>> Can we reduce the number of arguments?
> That would be nice, but I'm not sure what to remove. The canonical,
> that most of the callers use (from the other versions of this function
> included in various packages in Emacs) is
> (directory-files-recursively "~/" "~\\'")
> or something. And then there are a couple that want the directories,
> too. And then there's `file-tree-walk', which is the new thing.
Exactly: the use cases are varied, so we need a very generic interface.
I actually find file-tree-walk not too bad in this respect.
> Uhm... we could make MATCH be the predicate if it's not a string?
But we need 2 answers: whether to include the file in the result, and
whether to recurse.
>>> (path (expand-file-name leaf dir)))
>> This is not a "path", it's a file name. Remember: by convention
>> a "path" is a list of directories, as in load-path, $PATH, etc...
> Right. But do we have a word for "string that designates either a file
> or a directory"?
Yes, we say "a file name". A directory is a kind of file.
> That's usually called "a path"...
No, there are people who like to use "path" for other things, but the
GNU convention is to only use it for "a list of directories, as in
load-path, $PATH, etc...".
Stefan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] master 3431e82: Ignore directory symlinks in directory-files-recursively
2014-12-22 3:02 ` Stefan Monnier
@ 2015-01-16 0:08 ` Lars Magne Ingebrigtsen
0 siblings, 0 replies; 8+ messages in thread
From: Lars Magne Ingebrigtsen @ 2015-01-16 0:08 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> It can avoid calling expand-file-name only to call file-name-directory
>>> (or file-name-nondirectory) right after.
>> It can, but in the use cases I've typically seen, you wouldn't do that,
>> I think? You'd say `(directory-name-p file)' and then... something...
>
> I don't understand what you mean here. I'm just pointing out that
> calling expand-file-name only to pass the result to the function
> parameter means that this work may be wasted.
>
> So passing the two parameters separately is a way to be more lazy.
The typical usage (in the callers that call versions of this function
other places in the Emacs tree) seems to be "give me a list of files
matching this regexp". Making the callers take a separate directory and
leaf name makes the caller do the work of joining those two things up
before anything can be done, so it's more lazy to have the
`directory-files-recursively' do this work.
If there are callers that also want directories, they will have to do
more work, but that extra work is just calling `directory-name-p', which
is a very cheap function.
So I don't really see any advantage to having two parameters in the
callback function.
>>> Can we reduce the number of arguments?
>> That would be nice, but I'm not sure what to remove. The canonical,
>> that most of the callers use (from the other versions of this function
>> included in various packages in Emacs) is
>> (directory-files-recursively "~/" "~\\'")
>> or something. And then there are a couple that want the directories,
>> too. And then there's `file-tree-walk', which is the new thing.
>
> Exactly: the use cases are varied, so we need a very generic interface.
> I actually find file-tree-walk not too bad in this respect.
>
>> Uhm... we could make MATCH be the predicate if it's not a string?
>
> But we need 2 answers: whether to include the file in the result, and
> whether to recurse.
Er, huhm, yes, I don't know what I was thinking.
Then I don't really see any practical way to reduce the number of
parameters below four. We could drop INCLUDE-DIRECTORIES, but then the
somewhat handy `(directory-files-recursively "/tmp/foo" "." t)' (to do
`rm r') wouldn't be as handy any more.
>> Right. But do we have a word for "string that designates either a file
>> or a directory"?
>
> Yes, we say "a file name". A directory is a kind of file.
Ok; I'll rename away.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-01-16 0:08 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20141212105304.4186.22515@vcs.savannah.gnu.org>
[not found] ` <E1XzNqL-000168-2E@vcs.savannah.gnu.org>
2014-12-12 14:57 ` [Emacs-diffs] master 3431e82: Ignore directory symlinks in directory-files-recursively Stefan Monnier
2014-12-13 15:01 ` Lars Magne Ingebrigtsen
2014-12-14 4:59 ` Stefan Monnier
2014-12-14 9:33 ` Lars Magne Ingebrigtsen
2014-12-14 14:27 ` Stefan Monnier
2014-12-21 11:57 ` Lars Ingebrigtsen
2014-12-22 3:02 ` Stefan Monnier
2015-01-16 0:08 ` Lars Magne Ingebrigtsen
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.