all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
@ 2023-01-04 22:56 lux
  2023-01-06  9:48 ` Robert Pluim
  0 siblings, 1 reply; 16+ messages in thread
From: lux @ 2023-01-04 22:56 UTC (permalink / raw)
  To: 60562

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

If a space in filename, hfy-list-files function error. For example:

$ mkdir /tmp/test 
$ cd /tmp/test
$ touch 'hello world.py'
$ touch hi.py
$ ls
hello world.py  hi.py

In Emacs:

(hfy-list-files "/tmp/test")
("hi.py" "hello" "world.py")

As shown above, "hello world.py" is split into two files.

[-- Attachment #2: 0001-Fix-split-string-error-if-there-is-a-space-in-the-fi.patch --]
[-- Type: text/x-patch, Size: 1172 bytes --]

From 15b9d238ddcca17004d9edce54b4b1fb80556764 Mon Sep 17 00:00:00 2001
From: Xi Lu <lx@shellcodes.org>
Date: Thu, 5 Jan 2023 06:36:19 +0800
Subject: [PATCH] Fix split-string error if there is a space in the filename.

* lisp/htmlfontify.el (hfy-list-files): Specify separator (\n\r).
---
 lisp/htmlfontify.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/htmlfontify.el b/lisp/htmlfontify.el
index c989a12d205..be020b6b1c5 100644
--- a/lisp/htmlfontify.el
+++ b/lisp/htmlfontify.el
@@ -1826,8 +1826,9 @@ hfy-list-files
   ;;(message "hfy-list-files");;DBUG
   ;; FIXME: this changes the dir of the current buffer.  Is that right??
   (cd directory)
-  (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F) (match-string 1 F) F))
-          (split-string (shell-command-to-string hfy-find-cmd))) )
+  (remove-if #'string-empty-p
+             (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F) (match-string 1 F) F))
+                     (split-string (shell-command-to-string hfy-find-cmd) "[\n\r]+")) ))
 
 ;; strip the filename off, return a directory name
 ;; not a particularly thorough implementation, but it will be
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-04 22:56 bug#60562: [PATCH] Fix split-string error if there is a space in the filename lux
@ 2023-01-06  9:48 ` Robert Pluim
  2023-01-07  9:29   ` Eli Zaretskii
  2023-01-07  9:42   ` lux
  0 siblings, 2 replies; 16+ messages in thread
From: Robert Pluim @ 2023-01-06  9:48 UTC (permalink / raw)
  To: lux; +Cc: 60562

>>>>> On Thu, 5 Jan 2023 06:56:05 +0800, lux <lx@shellcodes.org> said:

    lux> * lisp/htmlfontify.el (hfy-list-files): Specify separator (\n\r).
    lux> ---
    lux>  lisp/htmlfontify.el | 5 +++--
    lux>  1 file changed, 3 insertions(+), 2 deletions(-)

    lux> diff --git a/lisp/htmlfontify.el b/lisp/htmlfontify.el
    lux> index c989a12d205..be020b6b1c5 100644
    lux> --- a/lisp/htmlfontify.el
    lux> +++ b/lisp/htmlfontify.el
    lux> @@ -1826,8 +1826,9 @@ hfy-list-files
    lux>    ;;(message "hfy-list-files");;DBUG
    lux>    ;; FIXME: this changes the dir of the current buffer.  Is that right??
    lux>    (cd directory)
    lux> -  (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F) (match-string 1 F) F))
    lux> -          (split-string (shell-command-to-string hfy-find-cmd))) )
    lux> +  (remove-if #'string-empty-p
    lux> +             (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F) (match-string 1 F) F))
    lux> +                     (split-string (shell-command-to-string hfy-find-cmd) "[\n\r]+")) ))

You can avoid the issue (and improve portability) by using
`directory-files-recursively' instead of `find' (which is annoyingly
hard to remember, since the obvious search leads to
`list-directory'. Perhaps we should add `list-directory-recursively'
as an alias?)

Robert
-- 





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-06  9:48 ` Robert Pluim
@ 2023-01-07  9:29   ` Eli Zaretskii
  2023-01-07 11:16     ` Robert Pluim
  2023-01-07 14:52     ` lux
  2023-01-07  9:42   ` lux
  1 sibling, 2 replies; 16+ messages in thread
From: Eli Zaretskii @ 2023-01-07  9:29 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 60562, lx

> Cc: 60562@debbugs.gnu.org
> From: Robert Pluim <rpluim@gmail.com>
> Date: Fri, 06 Jan 2023 10:48:43 +0100
> 
> >>>>> On Thu, 5 Jan 2023 06:56:05 +0800, lux <lx@shellcodes.org> said:
> 
>     lux> * lisp/htmlfontify.el (hfy-list-files): Specify separator (\n\r).
>     lux> ---
>     lux>  lisp/htmlfontify.el | 5 +++--
>     lux>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
>     lux> diff --git a/lisp/htmlfontify.el b/lisp/htmlfontify.el
>     lux> index c989a12d205..be020b6b1c5 100644
>     lux> --- a/lisp/htmlfontify.el
>     lux> +++ b/lisp/htmlfontify.el
>     lux> @@ -1826,8 +1826,9 @@ hfy-list-files
>     lux>    ;;(message "hfy-list-files");;DBUG
>     lux>    ;; FIXME: this changes the dir of the current buffer.  Is that right??
>     lux>    (cd directory)
>     lux> -  (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F) (match-string 1 F) F))
>     lux> -          (split-string (shell-command-to-string hfy-find-cmd))) )
>     lux> +  (remove-if #'string-empty-p
>     lux> +             (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F) (match-string 1 F) F))
>     lux> +                     (split-string (shell-command-to-string hfy-find-cmd) "[\n\r]+")) ))
> 
> You can avoid the issue (and improve portability) by using
> `directory-files-recursively' instead of `find'

Right.  Would you like to rewrite the patch using
directory-files-recursively?

> (which is annoyingly
> hard to remember, since the obvious search leads to
> `list-directory'. Perhaps we should add `list-directory-recursively'
> as an alias?)

How did you search for it?





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-06  9:48 ` Robert Pluim
  2023-01-07  9:29   ` Eli Zaretskii
@ 2023-01-07  9:42   ` lux
  2023-01-07 10:48     ` Eli Zaretskii
  1 sibling, 1 reply; 16+ messages in thread
From: lux @ 2023-01-07  9:42 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 60562

On Fri, 06 Jan 2023 10:48:43 +0100
Robert Pluim <rpluim@gmail.com> wrote:

> >>>>> On Thu, 5 Jan 2023 06:56:05 +0800, lux <lx@shellcodes.org>
> >>>>> said:  
> 
>     lux> * lisp/htmlfontify.el (hfy-list-files): Specify separator
>     lux> (\n\r). ---
>     lux>  lisp/htmlfontify.el | 5 +++--
>     lux>  1 file changed, 3 insertions(+), 2 deletions(-)  
> 
>     lux> diff --git a/lisp/htmlfontify.el b/lisp/htmlfontify.el
>     lux> index c989a12d205..be020b6b1c5 100644
>     lux> --- a/lisp/htmlfontify.el
>     lux> +++ b/lisp/htmlfontify.el
>     lux> @@ -1826,8 +1826,9 @@ hfy-list-files
>     lux>    ;;(message "hfy-list-files");;DBUG
>     lux>    ;; FIXME: this changes the dir of the current buffer.  Is
>     lux> that right?? (cd directory)
>     lux> -  (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F)
>     lux> (match-string 1 F) F))
>     lux> -          (split-string (shell-command-to-string
>     lux> hfy-find-cmd))) )
>     lux> +  (remove-if #'string-empty-p
>     lux> +             (mapcar (lambda (F) (if (string-match
>     lux> "^./\\(.*\\)" F) (match-string 1 F) F))
>     lux> +                     (split-string (shell-command-to-string
>     lux> hfy-find-cmd) "[\n\r]+")) ))  
> 
> You can avoid the issue (and improve portability) by using
> `directory-files-recursively' instead of `find'

But `hfy-find-cmd' is a configurable variable:

(defcustom hfy-find-cmd
  "find . -type f \\! -name \\*~ \\! -name \\*.flc \\! -path
\\*/CVS/\\*" "Find command used to harvest a list of files to attempt
to fontify." :tag   "find-command"
  :type  '(string))

I don't know if using `directory-files-recursively' has other effects.









^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-07  9:42   ` lux
@ 2023-01-07 10:48     ` Eli Zaretskii
  2023-01-15 12:33       ` Andy Moreton
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2023-01-07 10:48 UTC (permalink / raw)
  To: lux; +Cc: 60562, rpluim

> Cc: 60562@debbugs.gnu.org
> Date: Sat, 7 Jan 2023 17:42:35 +0800
> From: lux <lx@shellcodes.org>
> 
> On Fri, 06 Jan 2023 10:48:43 +0100
> Robert Pluim <rpluim@gmail.com> wrote:
> 
> > You can avoid the issue (and improve portability) by using
> > `directory-files-recursively' instead of `find'
> 
> But `hfy-find-cmd' is a configurable variable:
> 
> (defcustom hfy-find-cmd
>   "find . -type f \\! -name \\*~ \\! -name \\*.flc \\! -path
> \\*/CVS/\\*" "Find command used to harvest a list of files to attempt
> to fontify." :tag   "find-command"
>   :type  '(string))

We can convert that to a customizable regexp, and maybe to a predicate
function if a simple regexp won't do.

> I don't know if using `directory-files-recursively' has other effects.

Why would that bother us?  This is used to find the files under a
given directory, avoiding some files which we know will not be wanted.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-07  9:29   ` Eli Zaretskii
@ 2023-01-07 11:16     ` Robert Pluim
  2023-01-07 11:37       ` Eli Zaretskii
  2023-01-07 14:52     ` lux
  1 sibling, 1 reply; 16+ messages in thread
From: Robert Pluim @ 2023-01-07 11:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60562, lx

>>>>> On Sat, 07 Jan 2023 11:29:58 +0200, Eli Zaretskii <eliz@gnu.org> said:

    Eli> Right.  Would you like to rewrite the patch using
    Eli> directory-files-recursively?

I donʼt have time for that at the moment, unfortunately

    >> (which is annoyingly
    >> hard to remember, since the obvious search leads to
    >> `list-directory'. Perhaps we should add `list-directory-recursively'
    >> as an alias?)

    Eli> How did you search for it?

I use helm, which does subword searching, so

C-h f list dir

since "Iʼm trying to list the contents of a directory, the verb is
'list', the object is 'directory'"

If Iʼd done 'dir files' instead I would have gotten there
quicker. (the same process with 'i' in the elisp info manual leads to
similar results).

Robert
-- 





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-07 11:16     ` Robert Pluim
@ 2023-01-07 11:37       ` Eli Zaretskii
  2023-01-07 13:56         ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2023-01-07 11:37 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 60562, lx

> From: Robert Pluim <rpluim@gmail.com>
> Cc: lx@shellcodes.org,  60562@debbugs.gnu.org
> Date: Sat, 07 Jan 2023 12:16:55 +0100
> 
> >>>>> On Sat, 07 Jan 2023 11:29:58 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> (which is annoyingly
>     >> hard to remember, since the obvious search leads to
>     >> `list-directory'. Perhaps we should add `list-directory-recursively'
>     >> as an alias?)
> 
>     Eli> How did you search for it?
> 
> I use helm, which does subword searching, so
> 
> C-h f list dir

I was about to suggest "M-x apropos-documentation RET list dir RET",
but I see now that for some strange reason (bug?) it doesn't catch
directory-files-recursively, although its doc string does match.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-07 11:37       ` Eli Zaretskii
@ 2023-01-07 13:56         ` Eli Zaretskii
  0 siblings, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2023-01-07 13:56 UTC (permalink / raw)
  To: rpluim; +Cc: 60562, lx

> Cc: 60562@debbugs.gnu.org, lx@shellcodes.org
> Date: Sat, 07 Jan 2023 13:37:54 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> I was about to suggest "M-x apropos-documentation RET list dir RET",
> but I see now that for some strange reason (bug?) it doesn't catch
> directory-files-recursively, although its doc string does match.

The issue with "C-h d" is now bug#60628.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-07  9:29   ` Eli Zaretskii
  2023-01-07 11:16     ` Robert Pluim
@ 2023-01-07 14:52     ` lux
  2023-01-07 23:00       ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 16+ messages in thread
From: lux @ 2023-01-07 14:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60562, Robert Pluim

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

On Sat, 07 Jan 2023 11:29:58 +0200
Eli Zaretskii <eliz@gnu.org> wrote:

> > Cc: 60562@debbugs.gnu.org
> > From: Robert Pluim <rpluim@gmail.com>
> > Date: Fri, 06 Jan 2023 10:48:43 +0100
> >   
> > >>>>> On Thu, 5 Jan 2023 06:56:05 +0800, lux <lx@shellcodes.org>
> > >>>>> said:  
> >   
> >     lux> * lisp/htmlfontify.el (hfy-list-files): Specify separator
> >     lux> (\n\r). ---
> >     lux>  lisp/htmlfontify.el | 5 +++--
> >     lux>  1 file changed, 3 insertions(+), 2 deletions(-)  
> >   
> >     lux> diff --git a/lisp/htmlfontify.el b/lisp/htmlfontify.el
> >     lux> index c989a12d205..be020b6b1c5 100644
> >     lux> --- a/lisp/htmlfontify.el
> >     lux> +++ b/lisp/htmlfontify.el
> >     lux> @@ -1826,8 +1826,9 @@ hfy-list-files
> >     lux>    ;;(message "hfy-list-files");;DBUG
> >     lux>    ;; FIXME: this changes the dir of the current buffer.
> >     lux> Is that right?? (cd directory)
> >     lux> -  (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F)
> >     lux> (match-string 1 F) F))
> >     lux> -          (split-string (shell-command-to-string
> >     lux> hfy-find-cmd))) )
> >     lux> +  (remove-if #'string-empty-p
> >     lux> +             (mapcar (lambda (F) (if (string-match
> >     lux> "^./\\(.*\\)" F) (match-string 1 F) F))
> >     lux> +                     (split-string
> >     lux> (shell-command-to-string hfy-find-cmd) "[\n\r]+")) ))  
> > 
> > You can avoid the issue (and improve portability) by using
> > `directory-files-recursively' instead of `find'  
> 
> Right.  Would you like to rewrite the patch using
> directory-files-recursively?
>

I try rewrite the patch using  directory-files-recursively.


[-- Attachment #2: 0001-Replace-hfy-find-cmd-with-directory-files-recursivel.patch --]
[-- Type: text/x-patch, Size: 1880 bytes --]

From 1e00b856ca423bb7e4491b9999cd3de80223d0bd Mon Sep 17 00:00:00 2001
From: Xi Lu <lx@shellcodes.org>
Date: Sat, 7 Jan 2023 22:46:40 +0800
Subject: [PATCH] Replace `hfy-find-cmd' with `directory-files-recursively'.

---
 lisp/htmlfontify.el | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/lisp/htmlfontify.el b/lisp/htmlfontify.el
index c989a12d205..0746b5e3a36 100644
--- a/lisp/htmlfontify.el
+++ b/lisp/htmlfontify.el
@@ -372,11 +372,14 @@ hfy-istext-command
   :tag   "istext-command"
   :type  '(string))
 
-(defcustom hfy-find-cmd
-  "find . -type f \\! -name \\*~ \\! -name \\*.flc \\! -path \\*/CVS/\\*"
-  "Find command used to harvest a list of files to attempt to fontify."
-  :tag   "find-command"
-  :type  '(string))
+(defcustom hfy-exclude-file-rules
+  '("\\.flc$"
+    "/CVS/.*"
+    ".*~"
+    "\\.git/.*")
+  "Define some regular expressions to exclude files"
+  :tag "exclude-rules"
+  :type '(list string))
 
 (defcustom hfy-display-class nil
   "Display class to use to determine which display class to use when
@@ -1823,11 +1826,12 @@ htmlfontify-buffer
 (defun hfy-list-files (directory)
   "Return a list of files under DIRECTORY.
 Strips any leading \"./\" from each filename."
-  ;;(message "hfy-list-files");;DBUG
+  ;;(message "hfy-list-files");;DEBUG
   ;; FIXME: this changes the dir of the current buffer.  Is that right??
   (cd directory)
-  (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F) (match-string 1 F) F))
-          (split-string (shell-command-to-string hfy-find-cmd))) )
+  (remove-if (lambda (f) (seq-some (lambda (r)
+                                     (string-match r f)) hfy-exclude-file-rules))
+             (directory-files-recursively "." ".*")))
 
 ;; strip the filename off, return a directory name
 ;; not a particularly thorough implementation, but it will be
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-07 14:52     ` lux
@ 2023-01-07 23:00       ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-01-08  7:23         ` lux
  0 siblings, 1 reply; 16+ messages in thread
From: Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-01-07 23:00 UTC (permalink / raw)
  To: lux; +Cc: 60562, eliz, rpluim

Hi,

>-(defcustom hfy-find-cmd
>-  "find . -type f \\! -name \\*~ \\! -name \\*.flc \\! -path \\*/CVS/\\*"
>-  "Find command used to harvest a list of files to attempt to fontify."
>-  :tag   "find-command"
>-  :type  '(string))
>+(defcustom hfy-exclude-file-rules
>+  '("\\.flc$"
>+    "/CVS/.*"
>+    ".*~"
>+    "\\.git/.*")
>+  "Define some regular expressions to exclude files"
>+  :tag "exclude-rules"
>+  :type '(list string))

For the third entry, shouldn't it be ".*~$" instead, to indicate that
"~" is the last character?

For the fourth entry, currently it would match against the file name
"ROOT/hello.git/foo".  In addition, for git submodules, ".git" is a
regular file instead of a directory.  Maybe something like this is what
you want:

    (rx "/.git" (opt "/" (0+ any)) line-end)

or in raw regexp: "/\\.git\\(?:/.*\\)?$"

Also, in this change, we are dropping the requirement that the found
file are actually files, whereas we used to say "-type f".  Is this
change fine?

> (defun hfy-list-files (directory)
>   "Return a list of files under DIRECTORY.
> Strips any leading \"./\" from each filename."
>-  ;;(message "hfy-list-files");;DBUG
>+  ;;(message "hfy-list-files");;DEBUG
>   ;; FIXME: this changes the dir of the current buffer.  Is that right??
>   (cd directory)
>-  (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F) (match-string 1 F) F))
>-          (split-string (shell-command-to-string hfy-find-cmd))) )
>+  (remove-if (lambda (f) (seq-some (lambda (r)
>+                                     (string-match r f)) hfy-exclude-file-rules))
>+             (directory-files-recursively "." ".*")))

We should change `remove-if' into `cl-remove-if' because both "cl.el"
and the alias `remove-if' are deprecated.

Best,


RY





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-07 23:00       ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-01-08  7:23         ` lux
  2023-01-09 14:28           ` Robert Pluim
  2023-01-14  9:12           ` Eli Zaretskii
  0 siblings, 2 replies; 16+ messages in thread
From: lux @ 2023-01-08  7:23 UTC (permalink / raw)
  To: Ruijie Yu; +Cc: 60562, eliz, rpluim

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

On Sat, 07 Jan 2023 17:00:02 -0600
Ruijie Yu <ruijie@netyu.xyz> wrote:

> Hi,
> 
> >-(defcustom hfy-find-cmd
> >-  "find . -type f \\! -name \\*~ \\! -name \\*.flc \\! -path
> >\\*/CVS/\\*"
> >-  "Find command used to harvest a list of files to attempt to
> >fontify."
> >-  :tag   "find-command"
> >-  :type  '(string))
> >+(defcustom hfy-exclude-file-rules
> >+  '("\\.flc$"
> >+    "/CVS/.*"
> >+    ".*~"
> >+    "\\.git/.*")
> >+  "Define some regular expressions to exclude files"
> >+  :tag "exclude-rules"
> >+  :type '(list string))  
> 
> For the third entry, shouldn't it be ".*~$" instead, to indicate that
> "~" is the last character?
> 
> For the fourth entry, currently it would match against the file name
> "ROOT/hello.git/foo".  In addition, for git submodules, ".git" is a
> regular file instead of a directory.  Maybe something like this is
> what you want:
> 
>     (rx "/.git" (opt "/" (0+ any)) line-end)
> 
> or in raw regexp: "/\\.git\\(?:/.*\\)?$"
> 
> Also, in this change, we are dropping the requirement that the found
> file are actually files, whereas we used to say "-type f".  Is this
> change fine?
> 
> > (defun hfy-list-files (directory)
> >   "Return a list of files under DIRECTORY.
> > Strips any leading \"./\" from each filename."
> >-  ;;(message "hfy-list-files");;DBUG
> >+  ;;(message "hfy-list-files");;DEBUG
> >   ;; FIXME: this changes the dir of the current buffer.  Is that
> > right?? (cd directory)
> >-  (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F)
> >(match-string 1 F) F))
> >-          (split-string (shell-command-to-string hfy-find-cmd))) )
> >+  (remove-if (lambda (f) (seq-some (lambda (r)
> >+                                     (string-match r f))
> >hfy-exclude-file-rules))
> >+             (directory-files-recursively "." ".*")))  
> 
> We should change `remove-if' into `cl-remove-if' because both "cl.el"
> and the alias `remove-if' are deprecated.
> 

Thank you, I updated the patch file.

[-- Attachment #2: 0001-Replace-hfy-find-cmd-with-directory-files-recursivel.patch --]
[-- Type: text/x-patch, Size: 1837 bytes --]

From 58fcb50eae760a6565e1703dfb8cea46a4ee5843 Mon Sep 17 00:00:00 2001
From: Xi Lu <lx@shellcodes.org>
Date: Sat, 7 Jan 2023 22:46:40 +0800
Subject: [PATCH] Replace `hfy-find-cmd' with `directory-files-recursively'.

---
 lisp/htmlfontify.el | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/lisp/htmlfontify.el b/lisp/htmlfontify.el
index c989a12d205..f05bc4e1e35 100644
--- a/lisp/htmlfontify.el
+++ b/lisp/htmlfontify.el
@@ -372,11 +372,14 @@ hfy-istext-command
   :tag   "istext-command"
   :type  '(string))
 
-(defcustom hfy-find-cmd
-  "find . -type f \\! -name \\*~ \\! -name \\*.flc \\! -path \\*/CVS/\\*"
-  "Find command used to harvest a list of files to attempt to fontify."
-  :tag   "find-command"
-  :type  '(string))
+(defcustom hfy-exclude-file-rules
+  '("\\.flc$"
+    "/CVS/.*"
+    ".*~$"
+    "/\\.git\\(?:/.*\\)?$")
+  "Define some regular expressions to exclude files"
+  :tag "exclude-rules"
+  :type '(list string))
 
 (defcustom hfy-display-class nil
   "Display class to use to determine which display class to use when
@@ -1826,8 +1829,12 @@ hfy-list-files
   ;;(message "hfy-list-files");;DBUG
   ;; FIXME: this changes the dir of the current buffer.  Is that right??
   (cd directory)
-  (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F) (match-string 1 F) F))
-          (split-string (shell-command-to-string hfy-find-cmd))) )
+  (cl-remove-if (lambda (f)
+                  (or (null (file-regular-p f))
+                      (seq-some (lambda (r)
+                                  (string-match r f))
+                                hfy-exclude-file-rules)))
+                (directory-files-recursively "." ".*" nil t)))
 
 ;; strip the filename off, return a directory name
 ;; not a particularly thorough implementation, but it will be
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-08  7:23         ` lux
@ 2023-01-09 14:28           ` Robert Pluim
  2023-01-09 14:55             ` lux
  2023-01-14  9:12           ` Eli Zaretskii
  1 sibling, 1 reply; 16+ messages in thread
From: Robert Pluim @ 2023-01-09 14:28 UTC (permalink / raw)
  To: Ruijie Yu; +Cc: 60562, lux, eliz

    >> 
    >> Also, in this change, we are dropping the requirement that the found
    >> file are actually files, whereas we used to say "-type f".  Is this
    >> change fine?
    >>

`directory-files-recursively' by default only returns files (and the
latest patch explicitly passes `nil' for INCLUDE-DIRECTORIES anyway)

Robert
-- 





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-09 14:28           ` Robert Pluim
@ 2023-01-09 14:55             ` lux
  0 siblings, 0 replies; 16+ messages in thread
From: lux @ 2023-01-09 14:55 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Ruijie Yu, 60562, eliz

Robert Pluim <rpluim@gmail.com> writes:

>     >>
>     >> Also, in this change, we are dropping the requirement that the found
>     >> file are actually files, whereas we used to say "-type f".  Is this
>     >> change fine?
>     >>
>
> `directory-files-recursively' by default only returns files (and the
> latest patch explicitly passes `nil' for INCLUDE-DIRECTORIES anyway)
>

"-type f" will return all regular files, `directory-files-recursively'
contains all types of files (socket, block, etc..), so I latest patch
added `file-regular-p' to check the file type.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-08  7:23         ` lux
  2023-01-09 14:28           ` Robert Pluim
@ 2023-01-14  9:12           ` Eli Zaretskii
  1 sibling, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2023-01-14  9:12 UTC (permalink / raw)
  To: lux; +Cc: ruijie, 60562-done, rpluim

> Date: Sun, 8 Jan 2023 15:23:45 +0800
> From: lux <lx@shellcodes.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, 60562@debbugs.gnu.org, Robert Pluim
>  <rpluim@gmail.com>, bug-gnu-emacs@gnu.org
> 
> On Sat, 07 Jan 2023 17:00:02 -0600
> Ruijie Yu <ruijie@netyu.xyz> wrote:
> 
> > Hi,
> > 
> > >-(defcustom hfy-find-cmd
> > >-  "find . -type f \\! -name \\*~ \\! -name \\*.flc \\! -path
> > >\\*/CVS/\\*"
> > >-  "Find command used to harvest a list of files to attempt to
> > >fontify."
> > >-  :tag   "find-command"
> > >-  :type  '(string))
> > >+(defcustom hfy-exclude-file-rules
> > >+  '("\\.flc$"
> > >+    "/CVS/.*"
> > >+    ".*~"
> > >+    "\\.git/.*")
> > >+  "Define some regular expressions to exclude files"
> > >+  :tag "exclude-rules"
> > >+  :type '(list string))  
> > 
> > For the third entry, shouldn't it be ".*~$" instead, to indicate that
> > "~" is the last character?
> > 
> > For the fourth entry, currently it would match against the file name
> > "ROOT/hello.git/foo".  In addition, for git submodules, ".git" is a
> > regular file instead of a directory.  Maybe something like this is
> > what you want:
> > 
> >     (rx "/.git" (opt "/" (0+ any)) line-end)
> > 
> > or in raw regexp: "/\\.git\\(?:/.*\\)?$"
> > 
> > Also, in this change, we are dropping the requirement that the found
> > file are actually files, whereas we used to say "-type f".  Is this
> > change fine?
> > 
> > > (defun hfy-list-files (directory)
> > >   "Return a list of files under DIRECTORY.
> > > Strips any leading \"./\" from each filename."
> > >-  ;;(message "hfy-list-files");;DBUG
> > >+  ;;(message "hfy-list-files");;DEBUG
> > >   ;; FIXME: this changes the dir of the current buffer.  Is that
> > > right?? (cd directory)
> > >-  (mapcar (lambda (F) (if (string-match "^./\\(.*\\)" F)
> > >(match-string 1 F) F))
> > >-          (split-string (shell-command-to-string hfy-find-cmd))) )
> > >+  (remove-if (lambda (f) (seq-some (lambda (r)
> > >+                                     (string-match r f))
> > >hfy-exclude-file-rules))
> > >+             (directory-files-recursively "." ".*")))  
> > 
> > We should change `remove-if' into `cl-remove-if' because both "cl.el"
> > and the alias `remove-if' are deprecated.
> > 
> 
> Thank you, I updated the patch file.

Thanks, installed on the emacs-29 branch, and closing the bug.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-07 10:48     ` Eli Zaretskii
@ 2023-01-15 12:33       ` Andy Moreton
  2023-01-15 14:07         ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Andy Moreton @ 2023-01-15 12:33 UTC (permalink / raw)
  To: 60562

On Sat 07 Jan 2023, Eli Zaretskii wrote:

>> Cc: 60562@debbugs.gnu.org
>> Date: Sat, 7 Jan 2023 17:42:35 +0800
>> From: lux <lx@shellcodes.org>
>> 
>> On Fri, 06 Jan 2023 10:48:43 +0100
>> Robert Pluim <rpluim@gmail.com> wrote:
>> 
>> > You can avoid the issue (and improve portability) by using
>> > `directory-files-recursively' instead of `find'
>> 
>> But `hfy-find-cmd' is a configurable variable:
>> 
>> (defcustom hfy-find-cmd
>>   "find . -type f \\! -name \\*~ \\! -name \\*.flc \\! -path
>> \\*/CVS/\\*" "Find command used to harvest a list of files to attempt
>> to fontify." :tag   "find-command"
>>   :type  '(string))
>
> We can convert that to a customizable regexp, and maybe to a predicate
> function if a simple regexp won't do.
>
>> I don't know if using `directory-files-recursively' has other effects.
>
> Why would that bother us?  This is used to find the files under a
> given directory, avoiding some files which we know will not be wanted.

The fixes for this problem now cause bootstrap to fail, due to the
defcustom for hfy-exclude-file-rules containing a version tag that is
not a string.

    AndyM






^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#60562: [PATCH] Fix split-string error if there is a space in the filename.
  2023-01-15 12:33       ` Andy Moreton
@ 2023-01-15 14:07         ` Eli Zaretskii
  0 siblings, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2023-01-15 14:07 UTC (permalink / raw)
  To: Andy Moreton; +Cc: 60562

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Sun, 15 Jan 2023 12:33:49 +0000
> 
> On Sat 07 Jan 2023, Eli Zaretskii wrote:
> 
> The fixes for this problem now cause bootstrap to fail, due to the
> defcustom for hfy-exclude-file-rules containing a version tag that is
> not a string.

That was fixed yesterday, so please update from Git, and sorry for the
trouble.





^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-01-15 14:07 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-04 22:56 bug#60562: [PATCH] Fix split-string error if there is a space in the filename lux
2023-01-06  9:48 ` Robert Pluim
2023-01-07  9:29   ` Eli Zaretskii
2023-01-07 11:16     ` Robert Pluim
2023-01-07 11:37       ` Eli Zaretskii
2023-01-07 13:56         ` Eli Zaretskii
2023-01-07 14:52     ` lux
2023-01-07 23:00       ` Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-01-08  7:23         ` lux
2023-01-09 14:28           ` Robert Pluim
2023-01-09 14:55             ` lux
2023-01-14  9:12           ` Eli Zaretskii
2023-01-07  9:42   ` lux
2023-01-07 10:48     ` Eli Zaretskii
2023-01-15 12:33       ` Andy Moreton
2023-01-15 14:07         ` Eli Zaretskii

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.