bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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


Hi,

I have found a file on my disk named "31 octobre 2022 Paris.zip" that is
currently mismatch by dired.  A 'n' from the previous line sets the
cursor on the P of "Paris" and i'm not able to open or even edit the
name of this file from dired.

I propose the attached patch that fixes this issue.

Best regards,

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-dired-mismatch-on-some-filenames.patch --]
[-- Type: text/x-patch, Size: 1546 bytes --]

From 6bb77e5c364e4627c7b12519aa95d097fa80ee96 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sat, 22 Jul 2023 14:40:56 +0200
Subject: [PATCH] Fix dired mismatch on some filenames

* lisp/files.el (directory-listing-before-filename-regexp): Look for
the first five columns of a 'ls -l' entry instead of searching for a
size pattern.
---
 lisp/files.el | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index f8867432000..6090327def0 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -30,6 +30,7 @@
 
 (eval-when-compile
   (require 'pcase)
+  (require 'rx)
   (require 'easy-mmode)) ; For `define-minor-mode'.
 
 (defvar font-lock-keywords)
@@ -7765,9 +7766,12 @@ directory-listing-before-filename-regexp
          ;; parentheses:
          ;; -rw-r--r-- (modified) 2005-10-22 21:25 files.el
          ;; This is not supported yet.
-    (purecopy (concat "\\([0-9][BkKMGTPEZYRQ]? " iso
-		      "\\|.*[0-9][BkKMGTPEZYRQ]? "
-	              "\\(" western "\\|" western-comma
+    (purecopy (concat "\\("
+                      ;; Five columns before modification time: mode,
+                      ;; links, owner, group and size
+                      (rx (repeat 5 (and (one-or-more graph)
+				         (one-or-more space))))
+	              "\\(" iso "\\|" western "\\|" western-comma
                       "\\|" DD-MMM-YYYY "\\|" east-asian "\\)"
 		      "\\) +")))
   "Regular expression to match up to the file name in a directory listing.
-- 
2.40.0


[-- Attachment #3: Type: text/plain, Size: 7905 bytes --]



In GNU Emacs 30.0.50 (build 2, x86_64-unknown-openbsd7.3, cairo version
 1.17.8) of 2023-07-22 built on computer
Repository revision: 7298e66604d2119c72c4f7abd5160b29340737af
Repository branch: mgi/dired-regex
Windowing system distributor 'The X.Org Foundation', version 11.0.12101006
System Description: OpenBSD computer 7.3 GENERIC.MP#1125 amd64

Configured using:
 'configure --prefix=/home/manuel/emacs --bindir=/home/manuel/bin
 --with-x-toolkit=no --with-toolkit-scroll-bars=no --without-sound
 --without-compress-install CPPFLAGS=-I/usr/local/include
 LDFLAGS=-L/usr/local/lib'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBOTF LIBXML2 MODULES NOTIFY KQUEUE OLDXMENU PDUMPER PNG RSVG
SQLITE3 THREADS TIFF TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Magit

Minor modes in effect:
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  display-time-mode: t
  display-battery-mode: t
  server-mode: t
  hyperbole-mode: t
  override-global-mode: t
  repeat-mode: t
  desktop-save-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/manuel/.emacs.d/elpa/ef-themes-1.2.0/theme-loaddefs hides /home/manuel/emacs/share/emacs/30.0.50/lisp/theme-loaddefs

Features:
(shadow mail-extr emacsbug whitespace magit-patch magit-extras
face-remap magit-bookmark magit-submodule magit-obsolete magit-blame
magit-stash magit-reflog magit-bisect magit-push magit-pull magit-fetch
magit-clone magit-remote magit-commit magit-sequence magit-notes
magit-worktree magit-tag magit-merge magit-branch magit-reset
magit-files magit-refs magit-status magit magit-repos magit-apply
magit-wip magit-log which-func magit-diff smerge-mode diff git-commit
log-edit add-log magit-core magit-autorevert magit-margin
magit-transient magit-process with-editor magit-mode transient magit-git
magit-section magit-utils dash cus-edit cus-start kotl-autoloads
css-mode sgml-mode facemenu imenu vc-cvs vc-rcs log-view pcvs-util
make-mode sh-script smie treesit executable vc pascal vc-hg conf-mode
reveal gnus-dired mule-util vc-git diff-mode bug-reference paredit time
battery org-indent vc-dispatcher vc-svn oc-basic ol-eww eww url-queue
mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect ol-docview
doc-view jka-compr image-mode exif ol-bibtex bibtex ol-bbdb ol-w3m
ol-doi org-link-doi org-agenda cus-load exwm-randr xcb-randr exwm-config
ido exwm exwm-input xcb-keysyms xcb-xkb exwm-manage exwm-floating
xcb-cursor xcb-render exwm-layout exwm-workspace exwm-core xcb-ewmh
xcb-icccm xcb xcb-xproto xcb-types xcb-debug server modus-operandi-theme
modus-themes zone speed-type url-http url-auth url-gw nsm compat ytdious
mingus libmpdee reporter edebug detached-init detached autorevert
filenotify transmission calc-bin calc-ext calc calc-loaddefs rect
calc-macs supercite regi ebdb-message ebdb-gnus ebdb-mua ebdb-com crm
ebdb-format ebdb eieio-opt speedbar ezimage dframe eieio-base pcase
timezone gnus-icalendar org-capture icalendar visual-basic-mode cl
web-mode disp-table erlang-start smart-tabs-mode skeleton cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs slime-asdf grep slime-tramp slime-fancy slime-indentation
slime-cl-indent cl-indent slime-trace-dialog slime-fontifying-fu
slime-package-fu slime-references slime-compiler-notes-tree advice
slime-scratch slime-presentations bridge slime-macrostep macrostep
slime-mdot-fu slime-enclosing-context slime-fuzzy slime-fancy-trace
slime-fancy-inspector slime-c-p-c slime-editing-commands slime-autodoc
slime-repl slime-parse slime apropos arc-mode archive-mode hyperspec
thingatpt slime-autoloads hyperbole hinit hibtypes hib-doc-id klink
hib-kbd hib-debbugs hib-social hsys-www hypb-ert ert ewoc debug
backtrace hactypes bookmark pp hui-mini hui hui-mouse hmouse-key hargs
etags fileloop xref project hsys-org org-element org-persist org-id
org-refile avl-tree generator org ob ob-tangle ob-ref ob-lob ob-table
ob-exp org-macro org-src ob-comint org-pcomplete org-list org-footnote
org-faces org-entities ob-emacs-lisp ob-core ob-eval org-cycle org-table
ol org-fold org-fold-core org-keys oc org-loaddefs find-func org-version
org-compat org-macs hui-menu hyrolo-menu hyrolo sort hui-jmenu hpath
tramp rx tramp-loaddefs trampver tramp-integration files-x tramp-compat
xdg shell pcomplete format-spec noutline outline icons hmouse-sh
hmouse-drv hui-window pulse color hycontrol hui-select hsettings
hui-em-but hbut view hbdata hgnus gnus-msg gnus-art mm-uu mml2015
mm-view mml-smime smime gnutls dig gnus-sum shr pixel-fill kinsoku
url-file svg dom gnus-group gnus-undo gnus-start gnus-dbus gnus-cloud
nnimap nnmail mail-source utf7 nnoo parse-time iso8601 gnus-spec
gnus-int gnus-range gnus-win gnus nnheader range wid-edit hsmail message
sendmail yank-media puny rfc822 mml mml-sec epa derived epg rfc6068
epg-config gnus-util time-date mm-decode mm-bodies mm-encode mail-parse
rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr mailabbrev
mail-utils gmm-utils mailheader hmail htz cal-julian hbmap hmoccur hvar
hypb locate hact hhist compile text-property-search comint ansi-osc
ansi-color ring set hversion hload-path edmacro kmacro
use-package-bind-key bind-key appt diary-lib diary-loaddefs cal-menu
calendar cal-loaddefs dired-x dired-aux dired dired-loaddefs
notifications dbus xml cl-extra help-mode use-package-core repeat
easy-mmode desktop frameset debbugs-autoloads detached-autoloads
ebdb-autoloads ef-themes-autoloads exwm-autoloads hyperbole-autoloads
magit-autoloads git-commit-autoloads finder-inf magit-section-autoloads
dash-autoloads osm-autoloads paredit-autoloads rust-mode-autoloads
speed-type-autoloads transmission-autoloads visual-fill-column-autoloads
with-editor-autoloads info compat-autoloads ytdious-autoloads package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs password-cache
json subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs
cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind kqueue lcms2 dynamic-setting system-font-setting
font-render-setting cairo xinput2 x multi-tty move-toolbar
make-network-process emacs)

Memory information:
((conses 16 763799 139807) (symbols 48 62414 8)
 (strings 32 202382 14370) (string-bytes 1 6422275)
 (vectors 16 121262) (vector-slots 8 2342839 99930) (floats 8 641 507)
 (intervals 56 19677 523) (buffers 984 91))

-- 
Manuel Giraud

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Eli Zaretskii]

> Date: Sat, 22 Jul 2023 15:22:54 +0200
> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> I have found a file on my disk named "31 octobre 2022 Paris.zip" that is
> currently mismatch by dired.  A 'n' from the previous line sets the
> cursor on the P of "Paris" and i'm not able to open or even edit the
> name of this file from dired.
> 
> I propose the attached patch that fixes this issue.

I'm not sure there can be the end to such fixes.  The real fix is to
use GNU 'ls' which has special support for Dired (or use ls-lisp.el).
I guess you don't have GNU 'ls', since you are on OpenBSD?

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm not sure there can be the end to such fixes.  The real fix is to
> use GNU 'ls' which has special support for Dired (or use ls-lisp.el).
> I guess you don't have GNU 'ls', since you are on OpenBSD?

You're right I'm using OpenBSD's ls.  But I thought it was a reasonable
fix: it seems more obvious then looking for something that looked like a
file size.
-- 
Manuel Giraud

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 64791@debbugs.gnu.org
> Date: Sat, 22 Jul 2023 16:46:27 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I'm not sure there can be the end to such fixes.  The real fix is to
> > use GNU 'ls' which has special support for Dired (or use ls-lisp.el).
> > I guess you don't have GNU 'ls', since you are on OpenBSD?
> 
> You're right I'm using OpenBSD's ls.  But I thought it was a reasonable
> fix: it seems more obvious then looking for something that looked like a
> file size.

Maybe.  Let's hear what others think about this.

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: 64791@debbugs.gnu.org
>> Date: Sat, 22 Jul 2023 16:46:27 +0200
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> > I'm not sure there can be the end to such fixes.  The real fix is to
>> > use GNU 'ls' which has special support for Dired (or use ls-lisp.el).
>> > I guess you don't have GNU 'ls', since you are on OpenBSD?

Yup.  Perhaps we should consider checking for the existence of "gls" and
using that.  E.g., I use the following, here:

  (if (and (memq system-type '(berkeley-unix darwin))
           (executable-find "gls"))
      (setq insert-directory-program "gls"))

But I've only tested this on macOS so far, so I'm not sure how well this
would work on *BSD.  From memory, I think it should be fine.  Perhaps
Manuel can say what he thinks.

>> You're right I'm using OpenBSD's ls.  But I thought it was a reasonable
>> fix: it seems more obvious then looking for something that looked like a
>> file size.
>
> Maybe.  Let's hear what others think about this.

FWIW, I don't think I mind such portability fixes.  It wouldn't be a bad
thing if Dired played a bit more nicely on BSD machines.

The point that this could be a slippery slope is valid, but we can also
choose to reject further tweaks on a case-by-case basis.  The format of
ls is also inherently stable: many things depend on it.

On the other hand, the fix I propose above might be all that is needed
in many cases.

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sun, 3 Sep 2023 04:34:38 -0700
> Cc: Manuel Giraud <manuel@ledu-giraud.fr>, 64791@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Manuel Giraud <manuel@ledu-giraud.fr>
> >> Cc: 64791@debbugs.gnu.org
> >> Date: Sat, 22 Jul 2023 16:46:27 +0200
> >>
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >>
> >> > I'm not sure there can be the end to such fixes.  The real fix is to
> >> > use GNU 'ls' which has special support for Dired (or use ls-lisp.el).
> >> > I guess you don't have GNU 'ls', since you are on OpenBSD?
> 
> Yup.  Perhaps we should consider checking for the existence of "gls" and
> using that.  E.g., I use the following, here:
> 
>   (if (and (memq system-type '(berkeley-unix darwin))
>            (executable-find "gls"))
>       (setq insert-directory-program "gls"))
> 
> But I've only tested this on macOS so far, so I'm not sure how well this
> would work on *BSD.  From memory, I think it should be fine.  Perhaps
> Manuel can say what he thinks.
> 
> >> You're right I'm using OpenBSD's ls.  But I thought it was a reasonable
> >> fix: it seems more obvious then looking for something that looked like a
> >> file size.
> >
> > Maybe.  Let's hear what others think about this.
> 
> FWIW, I don't think I mind such portability fixes.  It wouldn't be a bad
> thing if Dired played a bit more nicely on BSD machines.
> 
> The point that this could be a slippery slope is valid, but we can also
> choose to reject further tweaks on a case-by-case basis.  The format of
> ls is also inherently stable: many things depend on it.
> 
> On the other hand, the fix I propose above might be all that is needed
> in many cases.

I'm still undecided.  And it doesn't help that none of *BSD users
except you spoke up.

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>>> Cc: 64791@debbugs.gnu.org
>>> Date: Sat, 22 Jul 2023 16:46:27 +0200
>>>
>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>
>>> > I'm not sure there can be the end to such fixes.  The real fix is to
>>> > use GNU 'ls' which has special support for Dired (or use ls-lisp.el).
>>> > I guess you don't have GNU 'ls', since you are on OpenBSD?
>
> Yup.  Perhaps we should consider checking for the existence of "gls" and
> using that.  E.g., I use the following, here:
>
>   (if (and (memq system-type '(berkeley-unix darwin))
>            (executable-find "gls"))
>       (setq insert-directory-program "gls"))
>
> But I've only tested this on macOS so far, so I'm not sure how well this
> would work on *BSD.  From memory, I think it should be fine.  Perhaps
> Manuel can say what he thinks.

It would work also on OpenBSD because GNU ls is also called "gls" in the
coreutils package here.  BTW, Could you reproduce this bug with default
ls on macOS?

>>> You're right I'm using OpenBSD's ls.  But I thought it was a reasonable
>>> fix: it seems more obvious then looking for something that looked like a
>>> file size.
>>
>> Maybe.  Let's hear what others think about this.
>
> FWIW, I don't think I mind such portability fixes.  It wouldn't be a bad
> thing if Dired played a bit more nicely on BSD machines.

Yes and I still think that this patch is reasonable even though someone
is using GNU ls.  It went "look for something that looks like a file
size" to "go directly after column 5 where the modification is
starting".  I find the former more error prone than the latter.

But maybe, I'm missing something about all the architectures (Unix and
non-Unix) that Emacs supports.
-- 
Manuel Giraud

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Stefan Kangas]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> BTW, Could you reproduce this bug with default ls on macOS?

Yes, I can reproduce it in "emacs -Q" on macOS.

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sun, 3 Sep 2023 11:18:01 -0700
> Cc: Eli Zaretskii <eliz@gnu.org>, 64791@debbugs.gnu.org
> 
> Manuel Giraud <manuel@ledu-giraud.fr> writes:
> 
> > BTW, Could you reproduce this bug with default ls on macOS?
> 
> Yes, I can reproduce it in "emacs -Q" on macOS.

What about making insert-directory-program a defcustom, so that people
who have GNU ls under a name other than "ls" could customize that?

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> What about making insert-directory-program a defcustom, so that people
> who have GNU ls under a name other than "ls" could customize that?

We could do that.  It wouldn't help my case unfortunately, as I'm using
the same config on my GNU/Linux machines as well, so I have to do the
whole `executable-find' thing anyway.

What do you think of something along these lines, which seems to work at
least on macOS and OpenBSD?  If you agree, we could investigate if it
works on FreeBSD and NetBSD too.  I *think* it will, but it's worth
checking to make sure.

diff --git a/lisp/files.el b/lisp/files.el
index 4188615e490..88c4ecadc9a 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -7723,7 +7723,11 @@ shell-quote-wildcard-pattern
       pattern))))


-(defvar insert-directory-program (purecopy "ls")
+(defvar insert-directory-program
+  (if (and (memq system-type '(berkeley-unix darwin))
+           (executable-find "gls"))
+      (purecopy "gls")
+    (purecopy "ls"))
   "Absolute or relative name of the `ls'-like program.
 This is used by `insert-directory' and `dired-insert-directory'
 \(thus, also by `dired').")

BTW, I see that doom-emacs has something much like this here:
https://github.com/doomemacs/doomemacs/blob/7a7503045850ea83f205de6e71e6d886187f4a22/modules/emacs/dired/config.el#L33

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sun, 3 Sep 2023 11:54:07 -0700
> Cc: manuel@ledu-giraud.fr, 64791@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > What about making insert-directory-program a defcustom, so that people
> > who have GNU ls under a name other than "ls" could customize that?
> 
> We could do that.  It wouldn't help my case unfortunately, as I'm using
> the same config on my GNU/Linux machines as well, so I have to do the
> whole `executable-find' thing anyway.
> 
> What do you think of something along these lines, which seems to work at
> least on macOS and OpenBSD?  If you agree, we could investigate if it
> works on FreeBSD and NetBSD too.  I *think* it will, but it's worth
> checking to make sure.
> 
> diff --git a/lisp/files.el b/lisp/files.el
> index 4188615e490..88c4ecadc9a 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -7723,7 +7723,11 @@ shell-quote-wildcard-pattern
>        pattern))))
> 
> 
> -(defvar insert-directory-program (purecopy "ls")
> +(defvar insert-directory-program
> +  (if (and (memq system-type '(berkeley-unix darwin))
> +           (executable-find "gls"))
> +      (purecopy "gls")
> +    (purecopy "ls"))
>    "Absolute or relative name of the `ls'-like program.
>  This is used by `insert-directory' and `dired-insert-directory'
>  \(thus, also by `dired').")

If we believe "gls" is the only popular name, I guess that's good
enough.

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> If we believe "gls" is the only popular name, I guess that's good
> enough.

I checked, and this seems to be the case.  According to the below links,
GNU ls is installed as "gls" on FreeBSD and NetBSD as well:

https://cgit.freebsd.org/ports/plain/sysutils/coreutils/pkg-plist
http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/sysutils/coreutils/PLIST?rev=1.22&content-type=text/x-cvsweb-markup

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Kangas <stefankangas@gmail.com>
>> Date: Sun, 3 Sep 2023 11:54:07 -0700
>> Cc: manuel@ledu-giraud.fr, 64791@debbugs.gnu.org
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > What about making insert-directory-program a defcustom, so that people
>> > who have GNU ls under a name other than "ls" could customize that?
>> 
>> We could do that.  It wouldn't help my case unfortunately, as I'm using
>> the same config on my GNU/Linux machines as well, so I have to do the
>> whole `executable-find' thing anyway.
>> 
>> What do you think of something along these lines, which seems to work at
>> least on macOS and OpenBSD?  If you agree, we could investigate if it
>> works on FreeBSD and NetBSD too.  I *think* it will, but it's worth
>> checking to make sure.
>> 
>> diff --git a/lisp/files.el b/lisp/files.el
>> index 4188615e490..88c4ecadc9a 100644
>> --- a/lisp/files.el
>> +++ b/lisp/files.el
>> @@ -7723,7 +7723,11 @@ shell-quote-wildcard-pattern
>>        pattern))))
>> 
>> 
>> -(defvar insert-directory-program (purecopy "ls")
>> +(defvar insert-directory-program
>> +  (if (and (memq system-type '(berkeley-unix darwin))
>> +           (executable-find "gls"))
>> +      (purecopy "gls")
>> +    (purecopy "ls"))
>>    "Absolute or relative name of the `ls'-like program.
>>  This is used by `insert-directory' and `dired-insert-directory'
>>  \(thus, also by `dired').")
>
> If we believe "gls" is the only popular name, I guess that's good
> enough.

Hi,

I don't know if this count as a test but it seems that both FreeBSD and
NetBSD also called it "gls" in their respective packages:

https://cgit.freebsd.org/ports/tree/sysutils/coreutils/pkg-plist
http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/sysutils/coreutils/PLIST?rev=1.22&content-type=text/x-cvsweb-markup

But, I have to say that I still don't understand why you would not also
consider applying my patch.
-- 
Manuel Giraud

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> I don't know if this count as a test but it seems that both FreeBSD and
> NetBSD also called it "gls" in their respective packages:
>
> https://cgit.freebsd.org/ports/tree/sysutils/coreutils/pkg-plist
> http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/sysutils/coreutils/PLIST?rev=1.22&content-type=text/x-cvsweb-markup
>
> But, I have to say that I still don't understand why you would not also
> consider applying my patch.

The popular name for GNU ls is actually `ls', given that you install
Coreutils from the release tarball.  Favoring `gls' over `ls' will lead
to Emacs overlooking a potentially newer Coreutils installation as long
as one from the package manager is already present.

Have the BSDs abjured the nasty habit of placing site software in a
single /usr/local directory?  If so, we could perhaps search in the
directory where they install GNU software, i.e. /usr/sfw (?)

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Po Lu <luangruo@yahoo.com> writes:

> Manuel Giraud <manuel@ledu-giraud.fr> writes:
>
>> I don't know if this count as a test but it seems that both FreeBSD and
>> NetBSD also called it "gls" in their respective packages:
>>
>> https://cgit.freebsd.org/ports/tree/sysutils/coreutils/pkg-plist
>> http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/sysutils/coreutils/PLIST?rev=1.22&content-type=text/x-cvsweb-markup
>>
>> But, I have to say that I still don't understand why you would not also
>> consider applying my patch.
>
> The popular name for GNU ls is actually `ls', given that you install
> Coreutils from the release tarball.  Favoring `gls' over `ls' will lead
> to Emacs overlooking a potentially newer Coreutils installation as long
> as one from the package manager is already present.

Yes there is this consideration too.  Maybe we should have something
more elaborate then.  AFAIR, there is some test afterward to see if the
found "ls" support "--dired" so maybe if the found "ls" does not support
"--dired" and we are on a BSD then we should look for "gls".

> Have the BSDs abjured the nasty habit of placing site software in a
> single /usr/local directory?

I don't know what you are calling "site software" but yes (OpenBSD at
least) is using /usr/local for its package software.

> If so, we could perhaps search in the directory where they install GNU
> software, i.e. /usr/sfw (?)

There is no such directory by default on OpenBSD.
-- 
Manuel Giraud

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Eli Zaretskii]

> Cc: 64791@debbugs.gnu.org, manuel@ledu-giraud.fr
> Date: Sun, 03 Sep 2023 22:25:30 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > -(defvar insert-directory-program (purecopy "ls")
> > +(defvar insert-directory-program
> > +  (if (and (memq system-type '(berkeley-unix darwin))
> > +           (executable-find "gls"))
> > +      (purecopy "gls")
> > +    (purecopy "ls"))
> >    "Absolute or relative name of the `ls'-like program.
> >  This is used by `insert-directory' and `dired-insert-directory'
> >  \(thus, also by `dired').")
> 
> If we believe "gls" is the only popular name, I guess that's good
> enough.

Actually, I take this back: this defvar will be computed when files.el
is loaded, which is at dump time.  So this is not TRT, since the
conditions and the environment at dump time could be very different
from those at run time.  It could be even on another machine.

So I once again suggest to make this a defcustom.  This has two
advantages:

 . it will be evaluated at Emacs startup, not at dump time
 . it will allow users to customize the value and thus avoid costly
   search  for the program and prevent finding the wrong version of
   'ls'

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> The popular name for GNU ls is actually `ls', given that you install
> Coreutils from the release tarball.

I doubt anyone would make install GNU coreutils on top of a BSD
userland, because there would be no end to the breakage that would
create.  That's the exact reason why the 'popular name' on BSD is "gls".

> Favoring `gls' over `ls' will lead to Emacs overlooking a potentially
> newer Coreutils installation as long as one from the package manager
> is already present.

I don't think I understand what situation you have in mind here.

> Have the BSDs abjured the nasty habit of placing site software in a
> single /usr/local directory?

They have not.

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

> I doubt anyone would make install GNU coreutils on top of a BSD
> userland, because there would be no end to the breakage that would
> create.  That's the exact reason why the 'popular name' on BSD is
> "gls".

???

GNU ls is a compatible replacement for Unix ls, the installation of
which shouldn't produce problems on Unix systems.  I've been installing
it from the coreutils tarball since it was still named fileutils,
without any ill effect.

>> Favoring `gls' over `ls' will lead to Emacs overlooking a
>> potentially
>> newer Coreutils installation as long as one from the package manager
>> is already present.
>
> I don't think I understand what situation you have in mind here.

Where /usr/local/bin/gls is older than another installation of
coreutils.

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> GNU ls is a compatible replacement for Unix ls, the installation of
> which shouldn't produce problems on Unix systems.  I've been installing
> it from the coreutils tarball since it was still named fileutils,
> without any ill effect.

I assume you do this on your BSD machines, so I'll take your word for
it.  I wouldn't be brave enough, myself.

In any case, I'm confident that most BSD users will use ports rather
than replacing their BSD userland with the GNU one, and we have
established that ports names it "gls".

Let's worry about the 99.99 % of use cases here.

>>> Favoring `gls' over `ls' will lead to Emacs overlooking a
>>> potentially
>>> newer Coreutils installation as long as one from the package manager
>>> is already present.
>>
>> I don't think I understand what situation you have in mind here.
>
> Where /usr/local/bin/gls is older than another installation of
> coreutils.

Why would that be an issue?

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Stefan Kangas]

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

Eli Zaretskii <eliz@gnu.org> writes:

> Actually, I take this back: this defvar will be computed when files.el
> is loaded, which is at dump time.  So this is not TRT, since the
> conditions and the environment at dump time could be very different
> from those at run time.  It could be even on another machine.

Yes, right.  That's no good.

> So I once again suggest to make this a defcustom.  This has two
> advantages:
>
>  . it will be evaluated at Emacs startup, not at dump time

Agreed.  Let's change it into a defcustom.

>  . it will allow users to customize the value and thus avoid costly
>    search  for the program and prevent finding the wrong version of
>    'ls'

Perhaps this part is okay though, given that the search should only
affect users on macOS and *BSD (who presumably will want this anyways)?

Proposed patch attached.

[-- Attachment #2: 0001-Make-insert-directory-program-a-defcustom-and-use-gl.patch --]
[-- Type: text/x-patch, Size: 1981 bytes --]

From 43ff9396fdc6b5643a72fdb76eca10067762b334 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Wed, 6 Sep 2023 00:16:03 +0200
Subject: [PATCH] Make insert-directory-program a defcustom and use "gls" on
 *BSD

* lisp/files.el (insert-directory-program): Change into defcustom.
Default to using "gls" on *BSD and macOS.  (Bug#64791)
---
 etc/NEWS      |  8 ++++++++
 lisp/files.el | 11 +++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index a68e67e0694..52648400261 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -128,6 +128,14 @@ can use this to distinguish between buffers visiting files with the
 same base name that belong to different projects by using the provided
 transform function 'project-uniquify-dirname-transform'.
 
+** 'insert-directory-program' is now a defcustom.
+
+** 'insert-directory-program' prefers "gls" on *BSD and macOS.
+On *BSD and macOS systems, it now defaults to using the "gls"
+executable, if it exists.  This should remove the need to change its
+value when installing GNU coreutils using something like ports or
+Homebrew.
+
 ** cl-print
 
 *** You can expand the "..." truncation everywhere.
diff --git a/lisp/files.el b/lisp/files.el
index 4188615e490..ef5c6a96b55 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -7723,10 +7723,17 @@ shell-quote-wildcard-pattern
       pattern))))
 
 
-(defvar insert-directory-program (purecopy "ls")
+(defcustom insert-directory-program
+  (if (and (memq system-type '(berkeley-unix darwin))
+           (executable-find "gls"))
+      (purecopy "gls")
+    (purecopy "ls"))
   "Absolute or relative name of the `ls'-like program.
 This is used by `insert-directory' and `dired-insert-directory'
-\(thus, also by `dired').")
+(thus, also by `dired')."
+  :group 'dired
+  :type 'string
+  :version "30.1")
 
 (defcustom directory-free-space-program (purecopy "df")
   "Program to get the amount of free space on a file system.
-- 
2.42.0

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Stefan Kangas]

Stefan Kangas <stefankangas@gmail.com> writes:

>> So I once again suggest to make this a defcustom.  This has two
>> advantages:
>>
>>  . it will be evaluated at Emacs startup, not at dump time
>
> Agreed.  Let's change it into a defcustom.
>
>>  . it will allow users to customize the value and thus avoid costly
>>    search  for the program and prevent finding the wrong version of
>>    'ls'
>
> Perhaps this part is okay though, given that the search should only
> affect users on macOS and *BSD (who presumably will want this anyways)?
>
> Proposed patch attached.

This doesn't work either, as it's also evaluated at dump time.
I guess we'd have to set it in something like `normal-top-level'.

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Tue, 5 Sep 2023 15:43:21 -0700
> Cc: 64791@debbugs.gnu.org, manuel@ledu-giraud.fr
> 
> Stefan Kangas <stefankangas@gmail.com> writes:
> 
> >> So I once again suggest to make this a defcustom.  This has two
> >> advantages:
> >>
> >>  . it will be evaluated at Emacs startup, not at dump time
> >
> > Agreed.  Let's change it into a defcustom.
> >
> >>  . it will allow users to customize the value and thus avoid costly
> >>    search  for the program and prevent finding the wrong version of
> >>    'ls'
> >
> > Perhaps this part is okay though, given that the search should only
> > affect users on macOS and *BSD (who presumably will want this anyways)?
> >
> > Proposed patch attached.
> 
> This doesn't work either, as it's also evaluated at dump time.

I thought it should be evaluated at dump time _and_ at startup time.
Was I misremembering?

> I guess we'd have to set it in something like `normal-top-level'.

We have a machinery for delaying the initialization to startup time:
custom-initialize-delay.

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Stefan Kangas]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> I guess we'd have to set it in something like `normal-top-level'.
>
> We have a machinery for delaying the initialization to startup time:
> custom-initialize-delay.

TIL, thanks.  How does the attached patch look?

It bootstraps and seems to work on macOS, at least.

[-- Attachment #2: 0001-Make-insert-directory-program-a-defcustom-and-use-gl.patch --]
[-- Type: text/x-patch, Size: 2368 bytes --]

From 7a183ab0a6908030cbeee9301429920406465470 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sun, 24 Sep 2023 09:52:27 +0200
Subject: [PATCH] Make insert-directory-program a defcustom and use "gls" on
 *BSD

* lisp/files.el (insert-directory-program): Change into defcustom.
Default to using "gls" on *BSD and macOS.  (Bug#64791)
---
 etc/NEWS      |  8 ++++++++
 lisp/files.el | 19 +++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 53c8451dc19..6400fb59255 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -144,6 +144,14 @@ can use this to distinguish between buffers visiting files with the
 same base name that belong to different projects by using the provided
 transform function 'project-uniquify-dirname-transform'.
 
+** 'insert-directory-program' is now a defcustom.
+
+** 'insert-directory-program' prefers "gls" on *BSD and macOS.
+On *BSD and macOS systems, it now defaults to using the "gls"
+executable, if it exists.  This should remove the need to change its
+value when installing GNU coreutils using something like ports or
+Homebrew.
+
 ** cl-print
 
 *** You can expand the "..." truncation everywhere.
diff --git a/lisp/files.el b/lisp/files.el
index 9d766683d26..d305c51748c 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -7727,10 +7727,25 @@ shell-quote-wildcard-pattern
       pattern))))
 
 
-(defvar insert-directory-program (purecopy "ls")
+(defcustom insert-directory-program
+  (if (and (memq system-type '(berkeley-unix darwin))
+           (executable-find "gls"))
+      (purecopy "gls")
+    (purecopy "ls"))
   "Absolute or relative name of the `ls'-like program.
 This is used by `insert-directory' and `dired-insert-directory'
-\(thus, also by `dired').")
+(thus, also by `dired').  This should point to GNU ls, or another
+version of ls that supports the \"--dired\" flag.
+
+On GNU/Linux and other capable systems, the default is \"ls\".
+
+On *BSD and macOS systems, the default \"ls\" does not support
+the \"--dired\" flag.  Therefore, the default is to use the
+\"gls\" executable on such machines, if it exists."
+  :group 'dired
+  :type 'string
+  :initialize #'custom-initialize-delay
+  :version "30.1")
 
 (defcustom directory-free-space-program (purecopy "df")
   "Program to get the amount of free space on a file system.
-- 
2.42.0

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sun, 24 Sep 2023 05:34:20 -0700
> Cc: 64791@debbugs.gnu.org, manuel@ledu-giraud.fr
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> I guess we'd have to set it in something like `normal-top-level'.
> >
> > We have a machinery for delaying the initialization to startup time:
> > custom-initialize-delay.
> 
> TIL, thanks.  How does the attached patch look?
> 
> It bootstraps and seems to work on macOS, at least.

LGTM, thanks.

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> I guess we'd have to set it in something like `normal-top-level'.
>>
>> We have a machinery for delaying the initialization to startup time:
>> custom-initialize-delay.
>
> TIL, thanks.  How does the attached patch look?

Hi Stefan,

LGTM and I think it is better to have a custom for
`insert-directory-program'.  I'm not so sure about the docstring because
"--dired" compatibility is not really mandatory.
-- 
Manuel Giraud

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Stefan Kangas]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> LGTM and I think it is better to have a custom for
> `insert-directory-program'.

Thanks for revieiwing.

> I'm not so sure about the docstring because "--dired" compatibility is
> not really mandatory.

It's not, but some things won't work without it (see
`dired-use-ls-dired').

I'll see if I can find a way to make the situation a bit more clear
before installing.

bug#64791: 30.0.50; [PATCH] Fix dired mismatch on some filenames

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> LGTM, thanks.

Thanks, installed on master [1: 9f91a1c3ebe].

[1: 9f91a1c3ebe]: 2023-09-25 11:36:14 +0200
  Make insert-directory-program a defcustom; use "gls" on *BSD
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9f91a1c3ebef4034d5e96741023abc4f3d02d730