* bug#976: 23.0.60; incorrect code for filesets-get-filelist
2008-09-13 16:40 Drew Adams
@ 2008-09-16 20:44 ` Drew Adams
2008-09-16 21:28 ` Drew Adams
2015-12-30 1:16 ` Andrew Hyatt
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Drew Adams @ 2008-09-16 20:44 UTC (permalink / raw)
To: 976, emacs-pretest-bug
[-- Attachment #1: Type: text/plain, Size: 2861 bytes --]
The attached code might help you in fixing this - if not, ignore it. I haven't
tested all of the code that calls `filesets-get-filelist', but the version of
`filesets-get-filelist' attached seems to DTRT, AFAICT.
The problem with the original code, and with the suggestion I sent earlier, is
that `filesets-get-filelist' does not DTRT for type :tree. The original code is
completely broken - does nothing here. In my previously suggested code,
`filesets-get-filelist' does the same thing for type :tree as for type :pattern
- it returns only the files in the directory, not also the files in its
subdirectories.
See attached file for proposed new definition (with new function
filesets-files-under).
HTH
> From: Drew Adams Sent: Saturday, September 13, 2008 9:41 AM
> The part that treats a :tree of the code defining
> `filesets-get-filelist' is not correct and never could have been
> correct. And it does not correspond to the (correct) code from the
> filesets author. One wonders if the GNU Emacs code was ever tested.
>
> This is the `case' clause that treats :tree in the definition
> of `filesets-get-filelist':
>
> ((:tree)
> (let ((dir (nth 0 entry))
> (patt (nth 1 entry)))
> (filesets-directory-files dir patt ':files t)))
>
> But `entry' here is a complete fileset, which is of the form
> ("my-fs" (:tree "/some/directory" "^.+\.suffix$"))
>
> The above code thus tries to use "my-fs" as the directory, whereas it
> should use "/some/directory".
>
> This is the (correct) code in the latest version from the author
> (http://members.a1.net/t.link/CompEmacsFilesets.html). (The comment is
> from the author.)
>
> ((:tree)
> ;;well, the way trees are handled is a mess +++
> (let* ((dirpatt (if (consp (nth 1 entry))
> (filesets-entry-get-tree entry)
> entry))
> (dir (nth 0 dirpatt))
> (patt (nth 1 dirpatt)))
> (filesets-list-dir dir patt ':files t)))
>
> However, I think the following would be sufficient:
>
> ((:tree)
> (let* ((dirpatt (filesets-entry-get-tree entry))
> (dir (nth 0 dirpatt))
> (patt (nth 1 dirpatt)))
> (filesets-directory-files dir patt ':files t)))
>
> I don't see why the author's more complex treatment would ever be
> needed, since in order for the :tree clause of the `case' to be
> reached (consp (nth 1 entry)) must be a cons, AFAICT.
>
> At any rate, either the author's code or what I suggest immediately
> above is needed. There is no way that the current GNU Emacs code can
> work with a :tree fileset.
>
>
> In GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
> of 2008-09-03 on LENNART-69DE564
> Windowing system distributor `Microsoft Corp.', version 5.1.2600
> configured using `configure --with-gcc (3.4) --no-opt
> --cflags -Ic:/g/include
> -fno-crossjumping'
>
>
>
>
>
>
>
[-- Attachment #2: throw.el --]
[-- Type: application/octet-stream, Size: 2931 bytes --]
(defun filesets-get-filelist (entry &optional mode event)
"Get all files for fileset ENTRY.
Assume MODE (see `filesets-entry-mode'), if provided."
(let* ((mode (or mode (filesets-entry-mode entry)))
(fl (case mode
((:files) (filesets-entry-get-files entry))
((:file) (list (filesets-entry-get-file entry)))
((:ingroup)
(let ((entry (expand-file-name
(if (stringp entry)
entry
(filesets-entry-get-master entry)))))
(cons entry (filesets-ingroup-cache-get entry))))
((:tree)
(let* ((dirpatt (filesets-entry-get-tree entry))
(dir (nth 0 dirpatt))
(patt (nth 1 dirpatt))
(depth (or (filesets-entry-get-tree-max-level entry)
filesets-tree-max-level)))
(filesets-files-under 0 depth entry dir patt)))
((:pattern)
(let ((dirpatt (filesets-entry-get-pattern entry)))
(if dirpatt
(let ((dir (filesets-entry-get-pattern--dir dirpatt))
(patt (filesets-entry-get-pattern--pattern dirpatt)))
(filesets-directory-files dir patt ':files t))
(filesets-error 'error "Filesets: malformed entry: "
entry)))))))
(filesets-filter-list
fl (lambda (file) (not (filesets-filetype-property file event))))))
(defun filesets-files-under (level depth entry dir patt &optional absolutep)
"Files under DIR that match PATT.
LEVEL is the current level under DIR.
DEPTH is the maximal tree scanning depth for ENTRY.
ENTRY is a fileset.
DIR is a directory.
PATT is a regexp that included file names must match.
ABSOLUTEP non-nil means use absolute file names."
(and (or (= depth 0) (< level depth))
(let* ((dir (file-name-as-directory dir))
(files-here (filesets-directory-files
dir patt nil absolutep
(filesets-entry-get-filter-dirs-flag entry)))
(subdirs (filesets-filter-dir-names files-here))
(files
(filesets-filter-dir-names
(apply #'append
files-here
(mapcar
(lambda (subdir)
(let* ((subdir (file-name-as-directory subdir))
(full-subdir (concat dir subdir)))
(filesets-files-under (+ level 1) depth entry
full-subdir patt)))
subdirs))
t)))
files)))
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#976: 23.0.60; incorrect code for filesets-get-filelist
2008-09-16 20:44 ` Drew Adams
@ 2008-09-16 21:28 ` Drew Adams
0 siblings, 0 replies; 12+ messages in thread
From: Drew Adams @ 2008-09-16 21:28 UTC (permalink / raw)
To: 976, emacs-pretest-bug
[-- Attachment #1: Type: text/plain, Size: 3247 bytes --]
Sorry for the added noise, but it's probably better for the default behavior to
use absolute filenames. The attached code does that.
> From: Drew Adams Sent: Tuesday, September 16, 2008 1:44 PM
> The attached code might help you in fixing this - if not,
> ignore it. I haven't
> tested all of the code that calls `filesets-get-filelist',
> but the version of
> `filesets-get-filelist' attached seems to DTRT, AFAICT.
>
> The problem with the original code, and with the suggestion I
> sent earlier, is
> that `filesets-get-filelist' does not DTRT for type :tree.
> The original code is
> completely broken - does nothing here. In my previously
> suggested code,
> `filesets-get-filelist' does the same thing for type :tree as
> for type :pattern
> - it returns only the files in the directory, not also the
> files in its
> subdirectories.
>
> See attached file for proposed new definition (with new function
> filesets-files-under).
>
> HTH
>
>
> > From: Drew Adams Sent: Saturday, September 13, 2008 9:41 AM
> > The part that treats a :tree of the code defining
> > `filesets-get-filelist' is not correct and never could have been
> > correct. And it does not correspond to the (correct) code from the
> > filesets author. One wonders if the GNU Emacs code was ever tested.
> >
> > This is the `case' clause that treats :tree in the definition
> > of `filesets-get-filelist':
> >
> > ((:tree)
> > (let ((dir (nth 0 entry))
> > (patt (nth 1 entry)))
> > (filesets-directory-files dir patt ':files t)))
> >
> > But `entry' here is a complete fileset, which is of the form
> > ("my-fs" (:tree "/some/directory" "^.+\.suffix$"))
> >
> > The above code thus tries to use "my-fs" as the directory,
> whereas it
> > should use "/some/directory".
> >
> > This is the (correct) code in the latest version from the author
> > (http://members.a1.net/t.link/CompEmacsFilesets.html). (The
> comment is
> > from the author.)
> >
> > ((:tree)
> > ;;well, the way trees are handled is a mess +++
> > (let* ((dirpatt (if (consp (nth 1 entry))
> > (filesets-entry-get-tree entry)
> > entry))
> > (dir (nth 0 dirpatt))
> > (patt (nth 1 dirpatt)))
> > (filesets-list-dir dir patt ':files t)))
> >
> > However, I think the following would be sufficient:
> >
> > ((:tree)
> > (let* ((dirpatt (filesets-entry-get-tree entry))
> > (dir (nth 0 dirpatt))
> > (patt (nth 1 dirpatt)))
> > (filesets-directory-files dir patt ':files t)))
> >
> > I don't see why the author's more complex treatment would ever be
> > needed, since in order for the :tree clause of the `case' to be
> > reached (consp (nth 1 entry)) must be a cons, AFAICT.
> >
> > At any rate, either the author's code or what I suggest immediately
> > above is needed. There is no way that the current GNU Emacs code can
> > work with a :tree fileset.
> >
> >
> > In GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
> > of 2008-09-03 on LENNART-69DE564
> > Windowing system distributor `Microsoft Corp.', version 5.1.2600
> > configured using `configure --with-gcc (3.4) --no-opt
> > --cflags -Ic:/g/include
> > -fno-crossjumping'
> >
> >
> >
> >
> >
> >
> >
>
[-- Attachment #2: throw.el --]
[-- Type: application/octet-stream, Size: 5631 bytes --]
;;; filesets+.el --- Extensions to `filesets.el'.
;;
;; Filename: filesets+.el
;; Description: Extensions to `filesets.el'.
;; Author: Drew Adams
;; Maintainer: Drew Adams
;; Copyright (C) 2008, Drew Adams, all rights reserved.
;; Created: Tue Sep 16 14:11:36 2008 (-0700)
;; Version: 22.0
;; Last-Updated: Tue Sep 16 14:27:01 2008 (-0700)
;; By: dradams
;; Update #: 13
;; URL: http://www.emacswiki.org/cgi-bin/wiki/filesets+.el
;; Keywords:
;; Compatibility: GNU Emacs 22.x
;;
;; Features that might be required by this library:
;;
;; Required feature throw was not provided.
;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;
;;; Commentary:
;;
;; This library provides some fixes to standard library
;; `filesets.el'. The reference version of that library is 1.8.4,
;; but I believe the same fixes are appropriate for other versions
;; (e.g. 2.2, named `filesets2.el', which is the latest version by
;; the original author, at
;; http://members.a1.net/t.link/CompEmacsFilesets.html).
;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;
;;; Change log:
;;
;;
;; 2008/09/16 dadams
;; Created.
;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;
;; This program is free software; you can redistribute it and/or
;; modify it under the terms of the GNU General Public License as
;; published by the Free Software Foundation; either version 3, or
;; (at your option) any later version.
;;
;; This program is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
;; General Public License for more details.
;;
;; You should have received a copy of the GNU General Public License
;; along with this program; see the file COPYING. If not, write to
;; the Free Software Foundation, Inc., 51 Franklin Street, Fifth
;; Floor, Boston, MA 02110-1301, USA.;;; Code:
;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;
;;; Code:
(require 'filesets)
;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(defcustom filesets-menu-path '("file") ; Original was nil.
"The menu under which the filesets menu should be inserted.
See `add-submenu' for documentation."
:set (function filesets-set-default)
:type 'sexp :group 'filesets)
(defcustom filesets-menu-before "Open File..." ; Original was "File".
"The name of a menu before which this menu should be added.
See `add-submenu' for documentation."
:set (function filesets-set-default)
:type 'sexp :group 'filesets)
(defun filesets-get-filelist (entry &optional mode event)
"Get all files for fileset ENTRY.
Assume MODE (see `filesets-entry-mode'), if provided."
(let* ((mode (or mode (filesets-entry-mode entry)))
(fl (case mode
((:files) (filesets-entry-get-files entry))
((:file) (list (filesets-entry-get-file entry)))
((:ingroup)
(let ((entry (expand-file-name
(if (stringp entry)
entry
(filesets-entry-get-master entry)))))
(cons entry (filesets-ingroup-cache-get entry))))
((:tree)
(let* ((dirpatt (filesets-entry-get-tree entry))
(dir (nth 0 dirpatt))
(patt (nth 1 dirpatt))
(depth (or (filesets-entry-get-tree-max-level entry)
filesets-tree-max-level)))
(filesets-files-under 0 depth entry dir patt)))
((:pattern)
(let ((dirpatt (filesets-entry-get-pattern entry)))
(if dirpatt
(let ((dir (filesets-entry-get-pattern--dir dirpatt))
(patt (filesets-entry-get-pattern--pattern dirpatt)))
(filesets-directory-files dir patt ':files t))
(filesets-error 'error "Filesets: malformed entry: "
entry)))))))
(filesets-filter-list
fl (lambda (file) (not (filesets-filetype-property file event))))))
(defun filesets-files-under (level depth entry dir patt &optional relativep)
"Files under DIR that match PATT.
LEVEL is the current level under DIR.
DEPTH is the maximal tree scanning depth for ENTRY.
ENTRY is a fileset.
DIR is a directory.
PATT is a regexp that included file names must match.
RELATIVEP non-nil means use relative file names."
(and (or (= depth 0) (< level depth))
(let* ((dir (file-name-as-directory dir))
(files-here (filesets-directory-files
dir patt nil (not relativep)
(filesets-entry-get-filter-dirs-flag entry)))
(subdirs (filesets-filter-dir-names files-here))
(files
(filesets-filter-dir-names
(apply #'append
files-here
(mapcar
(lambda (subdir)
(let* ((subdir (file-name-as-directory subdir))
(full-subdir (concat dir subdir)))
(filesets-files-under (+ level 1) depth entry
full-subdir patt)))
subdirs))
t)))
files)))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(provide 'filesets+)
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; filesets+.el ends here
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#976: 23.0.60; incorrect code for filesets-get-filelist
2008-09-13 16:40 Drew Adams
2008-09-16 20:44 ` Drew Adams
@ 2015-12-30 1:16 ` Andrew Hyatt
2015-12-30 1:32 ` Drew Adams
2020-11-20 15:39 ` Mauro Aranda
2020-11-24 11:05 ` Mauro Aranda
3 siblings, 1 reply; 12+ messages in thread
From: Andrew Hyatt @ 2015-12-30 1:16 UTC (permalink / raw)
To: Drew Adams; +Cc: 976
"Drew Adams" <drew.adams@oracle.com> writes:
> The part that treats a :tree of the code defining
> `filesets-get-filelist' is not correct and never could have been
> correct. And it does not correspond to the (correct) code from the
> filesets author. One wonders if the GNU Emacs code was ever tested.
>
> This is the `case' clause that treats :tree in the definition
> of `filesets-get-filelist':
>
> ((:tree)
> (let ((dir (nth 0 entry))
> (patt (nth 1 entry)))
> (filesets-directory-files dir patt ':files t)))
>
> But `entry' here is a complete fileset, which is of the form
> ("my-fs" (:tree "/some/directory" "^.+\.suffix$"))
>
> The above code thus tries to use "my-fs" as the directory, whereas it
> should use "/some/directory".
I'm looking into whether this still exists in Emacs 25. The code still
is as you describe. Can you give steps to reproduce a user-visible bug?
>
> This is the (correct) code in the latest version from the author
> (http://members.a1.net/t.link/CompEmacsFilesets.html). (The comment is
> from the author.)
>
> ((:tree)
> ;;well, the way trees are handled is a mess +++
> (let* ((dirpatt (if (consp (nth 1 entry))
> (filesets-entry-get-tree entry)
> entry))
> (dir (nth 0 dirpatt))
> (patt (nth 1 dirpatt)))
> (filesets-list-dir dir patt ':files t)))
>
> However, I think the following would be sufficient:
>
> ((:tree)
> (let* ((dirpatt (filesets-entry-get-tree entry))
> (dir (nth 0 dirpatt))
> (patt (nth 1 dirpatt)))
> (filesets-directory-files dir patt ':files t)))
>
> I don't see why the author's more complex treatment would ever be
> needed, since in order for the :tree clause of the `case' to be
> reached (consp (nth 1 entry)) must be a cons, AFAICT.
>
> At any rate, either the author's code or what I suggest immediately
> above is needed. There is no way that the current GNU Emacs code can
> work with a :tree fileset.
>
>
> In GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
> of 2008-09-03 on LENNART-69DE564
> Windowing system distributor `Microsoft Corp.', version 5.1.2600
> configured using `configure --with-gcc (3.4) --no-opt --cflags -Ic:/g/include
> -fno-crossjumping'
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#976: 23.0.60; incorrect code for filesets-get-filelist
2015-12-30 1:16 ` Andrew Hyatt
@ 2015-12-30 1:32 ` Drew Adams
2015-12-30 2:56 ` Andrew Hyatt
0 siblings, 1 reply; 12+ messages in thread
From: Drew Adams @ 2015-12-30 1:32 UTC (permalink / raw)
To: Andrew Hyatt; +Cc: 976
> I'm looking into whether this still exists in Emacs 25. The code still
> is as you describe. Can you give steps to reproduce a user-visible bug?
My guess is that no one has changed the filesets code, so yes,
I'm pretty sure the problem is still there as reported.
The user-visible bug is what I described. See the original
report, which is quite detailed wrt the problem.
See also the version of `filesets-get-filelist' that I sent (in
attachment throw.el). It should be a fix. If it does not fix
everything 100%, it is at least an improvement over the current
code, which is completely broken for :tree.
You can also contact the original author of filesets.el. As I
said in the bug report, the GNU Emacs version does not respect
what the author wrote, and it cannot possibly work (e.g., for
:tree).
I can't really help more than this now. I'm sure someone on
the Emacs Dev team can fix it, but there is not much interest
in filesets, it seems.
Apparently there was only one person (Chong Yidong) who took a
look at the bug report, and he just asked someone else to look
into it. And that person never followed up (didn't even reply
to Yidong's request, apparently).
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#976: 23.0.60; incorrect code for filesets-get-filelist
2015-12-30 1:32 ` Drew Adams
@ 2015-12-30 2:56 ` Andrew Hyatt
2015-12-30 16:42 ` Drew Adams
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Hyatt @ 2015-12-30 2:56 UTC (permalink / raw)
To: Drew Adams; +Cc: 976
Drew Adams <drew.adams@oracle.com> writes:
>> I'm looking into whether this still exists in Emacs 25. The code still
>> is as you describe. Can you give steps to reproduce a user-visible bug?
>
> My guess is that no one has changed the filesets code, so yes,
> I'm pretty sure the problem is still there as reported.
>
> The user-visible bug is what I described. See the original
> report, which is quite detailed wrt the problem.
>
> See also the version of `filesets-get-filelist' that I sent (in
> attachment throw.el). It should be a fix. If it does not fix
> everything 100%, it is at least an improvement over the current
> code, which is completely broken for :tree.
>
> You can also contact the original author of filesets.el. As I
> said in the bug report, the GNU Emacs version does not respect
> what the author wrote, and it cannot possibly work (e.g., for
> :tree).
>
> I can't really help more than this now. I'm sure someone on
> the Emacs Dev team can fix it, but there is not much interest
> in filesets, it seems.
>
> Apparently there was only one person (Chong Yidong) who took a
> look at the bug report, and he just asked someone else to look
> into it. And that person never followed up (didn't even reply
> to Yidong's request, apparently).
OK, thanks for the reply. The original report seems lost (the bug
report at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=976 starts with
your response, in fact). But given what you've already provided, it
seems that someone familiar with filesets can reproduce and test your
fix. We'll leave the bug open until someone gets to it.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#976: 23.0.60; incorrect code for filesets-get-filelist
2015-12-30 2:56 ` Andrew Hyatt
@ 2015-12-30 16:42 ` Drew Adams
0 siblings, 0 replies; 12+ messages in thread
From: Drew Adams @ 2015-12-30 16:42 UTC (permalink / raw)
To: Andrew Hyatt; +Cc: 976
> >> I'm looking into whether this still exists in Emacs 25. The code still
> >> is as you describe. Can you give steps to reproduce a user-visible bug?
> >
> > My guess is that no one has changed the filesets code, so yes,
> > I'm pretty sure the problem is still there as reported.
> >
> > The user-visible bug is what I described. See the original
> > report, which is quite detailed wrt the problem.
> >
> > See also the version of `filesets-get-filelist' that I sent (in
> > attachment throw.el). It should be a fix. If it does not fix
> > everything 100%, it is at least an improvement over the current
> > code, which is completely broken for :tree.
> >
> > You can also contact the original author of filesets.el. As I
> > said in the bug report, the GNU Emacs version does not respect
> > what the author wrote, and it cannot possibly work (e.g., for
> > :tree).
> >
> > I can't really help more than this now. I'm sure someone on
> > the Emacs Dev team can fix it, but there is not much interest
> > in filesets, it seems.
> >
> > Apparently there was only one person (Chong Yidong) who took a
> > look at the bug report, and he just asked someone else to look
> > into it. And that person never followed up (didn't even reply
> > to Yidong's request, apparently).
>
> OK, thanks for the reply. The original report seems lost (the bug
> report at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=976 starts with
> your response, in fact). But given what you've already provided, it
> seems that someone familiar with filesets can reproduce and test your
> fix. We'll leave the bug open until someone gets to it.
Hi Andrew,
No, AFAICT the thread at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=976
is complete. It starts with my bug report, which begins this way,
continuing from the bug title, which is "incorrect code for
filesets-get-filelist":
The part that treats a :tree of the code defining
`filesets-get-filelist' is not correct and never could have been
correct. And it does not correspond to the (correct) code from the
filesets author. One wonders if the GNU Emacs code was ever tested.
Thx - Drew
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#976: 23.0.60; incorrect code for filesets-get-filelist
2008-09-13 16:40 Drew Adams
2008-09-16 20:44 ` Drew Adams
2015-12-30 1:16 ` Andrew Hyatt
@ 2020-11-20 15:39 ` Mauro Aranda
2020-11-21 22:40 ` Drew Adams
2020-11-24 11:05 ` Mauro Aranda
3 siblings, 1 reply; 12+ messages in thread
From: Mauro Aranda @ 2020-11-20 15:39 UTC (permalink / raw)
To: Drew Adams; +Cc: 976
[-- Attachment #1.1: Type: text/plain, Size: 3835 bytes --]
tags 976 patch
quit
"Drew Adams" <drew.adams@oracle.com> writes:
> Sorry for the added noise, but it's probably better for the default
behavior to
> use absolute filenames. The attached code does that.
>
>
>> From: Drew Adams Sent: Tuesday, September 16, 2008 1:44 PM
>> The attached code might help you in fixing this - if not,
>> ignore it. I haven't
>> tested all of the code that calls `filesets-get-filelist',
>> but the version of
>> `filesets-get-filelist' attached seems to DTRT, AFAICT.
>>
>> The problem with the original code, and with the suggestion I
>> sent earlier, is
>> that `filesets-get-filelist' does not DTRT for type :tree.
>> The original code is
>> completely broken - does nothing here. In my previously
>> suggested code,
>> `filesets-get-filelist' does the same thing for type :tree as
>> for type :pattern
>> - it returns only the files in the directory, not also the
>> files in its
>> subdirectories.
>>
>> See attached file for proposed new definition (with new function
>> filesets-files-under).
>>
>> HTH
>>
>>
>> > From: Drew Adams Sent: Saturday, September 13, 2008 9:41 AM
>> > The part that treats a :tree of the code defining
>> > `filesets-get-filelist' is not correct and never could have been
>> > correct. And it does not correspond to the (correct) code from the
>> > filesets author. One wonders if the GNU Emacs code was ever tested.
>> >
>> > This is the `case' clause that treats :tree in the definition
>> > of `filesets-get-filelist':
>> >
>> > ((:tree)
>> > (let ((dir (nth 0 entry))
>> > (patt (nth 1 entry)))
>> > (filesets-directory-files dir patt ':files t)))
>> >
>> > But `entry' here is a complete fileset, which is of the form
>> > ("my-fs" (:tree "/some/directory" "^.+\.suffix$"))
>> >
>> > The above code thus tries to use "my-fs" as the directory,
>> whereas it
>> > should use "/some/directory".
>> >
>> > This is the (correct) code in the latest version from the author
>> > (http://members.a1.net/t.link/CompEmacsFilesets.html). (The
>> comment is
>> > from the author.)
>> >
>> > ((:tree)
>> > ;;well, the way trees are handled is a mess +++
>> > (let* ((dirpatt (if (consp (nth 1 entry))
>> > (filesets-entry-get-tree entry)
>> > entry))
>> > (dir (nth 0 dirpatt))
>> > (patt (nth 1 dirpatt)))
>> > (filesets-list-dir dir patt ':files t)))
>> >
>> > However, I think the following would be sufficient:
>> >
>> > ((:tree)
>> > (let* ((dirpatt (filesets-entry-get-tree entry))
>> > (dir (nth 0 dirpatt))
>> > (patt (nth 1 dirpatt)))
>> > (filesets-directory-files dir patt ':files t)))
>> >
>> > I don't see why the author's more complex treatment would ever be
>> > needed, since in order for the :tree clause of the `case' to be
>> > reached (consp (nth 1 entry)) must be a cons, AFAICT.
>> >
>> > At any rate, either the author's code or what I suggest immediately
>> > above is needed. There is no way that the current GNU Emacs code can
>> > work with a :tree fileset.
>> >
>> >
>> > In GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
>> > of 2008-09-03 on LENNART-69DE564
>> > Windowing system distributor `Microsoft Corp.', version 5.1.2600
>> > configured using `configure --with-gcc (3.4) --no-opt
>> > --cflags -Ic:/g/include
>> > -fno-crossjumping'
>> >
Hi Drew,
It's been a long time, but your fix looks right to me, and of course
you're right that the current code is broken. I created a :tree fileset
for testing: without your fix I get wrong-type-argument errors, while
with your fix it works just fine.
I've recreated your fix below, added a commit message and formatted it
with `git format-patch'. Could you please take a look and see if it
is OK to push it? Or maybe you would like to provide a different commit
message for your change?
Thanks.
[-- Attachment #1.2: Type: text/html, Size: 5273 bytes --]
[-- Attachment #2: 0001-Fix-finding-filelist-for-tree-fileset-Bug-976.patch --]
[-- Type: text/x-patch, Size: 2973 bytes --]
From a35ea838d4ad512bf9fa391b7009d72db423cc15 Mon Sep 17 00:00:00 2001
From: Drew Adams <drew.adams@oracle.com>
Date: Fri, 20 Nov 2020 11:41:41 -0300
Subject: [PATCH] Fix finding filelist for :tree fileset (Bug#976)
* lisp/filesets.el (filesets-files-under): New function, used to get
all files for a :tree fileset.
(filesets-get-filelist): Use it. Look for the directory and the
pattern in the right place inside entry.
---
lisp/filesets.el | 37 ++++++++++++++++++++++++++++++++++---
1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/lisp/filesets.el b/lisp/filesets.el
index c7ec3f77f4..84196561fc 100644
--- a/lisp/filesets.el
+++ b/lisp/filesets.el
@@ -1718,9 +1718,12 @@ filesets-get-filelist
(filesets-entry-get-master entry)))))
(cons entry (filesets-ingroup-cache-get entry))))
(:tree
- (let ((dir (nth 0 entry))
- (patt (nth 1 entry)))
- (filesets-directory-files dir patt ':files t)))
+ (let* ((dirpatt (filesets-entry-get-tree entry))
+ (dir (nth 0 dirpatt))
+ (patt (nth 1 dirpatt))
+ (depth (or (filesets-entry-get-tree-max-level entry)
+ filesets-tree-max-level)))
+ (filesets-files-under 0 depth entry dir patt)))
(:pattern
(let ((dirpatt (filesets-entry-get-pattern entry)))
(if dirpatt
@@ -1734,6 +1737,34 @@ filesets-get-filelist
(lambda (file)
(not (filesets-filetype-property file event))))))
+(defun filesets-files-under (level depth entry dir patt &optional relativep)
+ "Files under DIR that match PATT.
+LEVEL is the current level under DIR.
+DEPTH is the maximal tree scanning depth for ENTRY.
+ENTRY is a fileset.
+DIR is a directory.
+PATT is a regexp that included file names must match.
+RELATIVEP non-nil means use relative file names."
+ (and (or (= depth 0) (< level depth))
+ (let* ((dir (file-name-as-directory dir))
+ (files-here (filesets-directory-files
+ dir patt nil (not relativep)
+ (filesets-entry-get-filter-dirs-flag entry)))
+ (subdirs (filesets-filter-dir-names files-here))
+ (files
+ (filesets-filter-dir-names
+ (apply #'append
+ files-here
+ (mapcar
+ (lambda (subdir)
+ (let* ((subdir (file-name-as-directory subdir))
+ (full-subdir (concat dir subdir)))
+ (filesets-files-under (+ level 1) depth entry
+ full-subdir patt)))
+ subdirs))
+ t)))
+ files)))
+
(defun filesets-open (&optional mode name lookup-name)
"Open the fileset called NAME.
Use LOOKUP-NAME for searching additional data if provided."
--
2.29.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#976: 23.0.60; incorrect code for filesets-get-filelist
2020-11-20 15:39 ` Mauro Aranda
@ 2020-11-21 22:40 ` Drew Adams
0 siblings, 0 replies; 12+ messages in thread
From: Drew Adams @ 2020-11-21 22:40 UTC (permalink / raw)
To: Mauro Aranda; +Cc: 976
> tags 976 patch
> quit
>
> Hi Drew,
>
> It's been a long time, but your fix looks right to me, and of course
> you're right that the current code is broken. I created a :tree fileset
> for testing: without your fix I get wrong-type-argument errors, while
> with your fix it works just fine.
>
> I've recreated your fix below, added a commit message and formatted it
> with `git format-patch'. Could you please take a look and see if it
> is OK to push it? Or maybe you would like to provide a different commit
> message for your change?
Hi Mauro. Thanks for working on this.
It's been so long since I filed this bug that I'm
not really any help on this. And I really don't
have any time now to help with it or much else wrt
Emacs.
I have complete confidence that whatever you come
up with will be an improvement (and it will likely
be a complete fix). Please do whatever you think
is needed. Thx.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#976: 23.0.60; incorrect code for filesets-get-filelist
2008-09-13 16:40 Drew Adams
` (2 preceding siblings ...)
2020-11-20 15:39 ` Mauro Aranda
@ 2020-11-24 11:05 ` Mauro Aranda
3 siblings, 0 replies; 12+ messages in thread
From: Mauro Aranda @ 2020-11-24 11:05 UTC (permalink / raw)
To: Drew Adams; +Cc: 976
[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]
tags 976 fixed
close 976 28.1
quit
Drew Adams <drew.adams@oracle.com> wrote:
> > tags 976 patch
> > quit
> >
> > Hi Drew,
> >
> > It's been a long time, but your fix looks right to me, and of course
> > you're right that the current code is broken. I created a :tree fileset
> > for testing: without your fix I get wrong-type-argument errors, while
> > with your fix it works just fine.
> >
> > I've recreated your fix below, added a commit message and formatted it
> > with `git format-patch'. Could you please take a look and see if it
> > is OK to push it? Or maybe you would like to provide a different commit
> > message for your change?
>
> Hi Mauro. Thanks for working on this.
>
> It's been so long since I filed this bug that I'm
> not really any help on this. And I really don't
> have any time now to help with it or much else wrt
> Emacs.
OK, no problem. I pushed your fix.
> I have complete confidence that whatever you come
> up with will be an improvement (and it will likely
> be a complete fix). Please do whatever you think
> is needed. Thx.
Thanks to you.
[-- Attachment #2: Type: text/html, Size: 1421 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread