GNU bug report logs - #67161
30.0.50; [PATCH] Add option `dired-filename-display-length'

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 67161 in the body.
You can then email your comments to 67161 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Liu Hui <liuhui1610 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; [PATCH] Add option `dired-filename-display-length'
Date: Tue, 14 Nov 2023 17:52:58 +0800

[Message part 1 (text/plain, inline)]

Hi,

The attached patch implements the feature of controlling the display
length of filename in dired buffers.

When setting this option to `window', the long filename is shortened
by hiding the middle part to ensure that the whole line needs not to
be wrapped. The option can also be set to an integer, meaning the
maximum display length of filename. For users who turn on line
truncation, this feature can display the end part including the file
extension, which I think is convenient.


Best,

--
Liu Hui

[0001-Add-option-dired-filename-display-length.patch (text/x-patch, attachment)]

Message #8 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: 67161 <at> debbugs.gnu.org
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Tue, 14 Nov 2023 15:26:38 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Tue, 14 Nov 2023 17:52:58 +0800
> 
> The attached patch implements the feature of controlling the display
> length of filename in dired buffers.
> 
> When setting this option to `window', the long filename is shortened
> by hiding the middle part to ensure that the whole line needs not to
> be wrapped. The option can also be set to an integer, meaning the
> maximum display length of filename. For users who turn on line
> truncation, this feature can display the end part including the file
> extension, which I think is convenient.

Thanks.

> +(defcustom dired-filename-hiding-ellipsis "…"

We shouldn't use non-ASCII characters by default, without checking
they can be displayed.

> +    (let ((ell-len (string-width dired-filename-hiding-ellipsis))
> +          maxlen filename-col)
> +      (while (< (point) end)
> +        (ignore-errors
> +	  (if (not (dired-move-to-filename))

Did you consider using truncate-string-to-width here?

Message #11 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67161 <at> debbugs.gnu.org
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Wed, 15 Nov 2023 18:04:13 +0800

Eli Zaretskii <eliz <at> gnu.org> 于2023年11月14日周二 21:26写道：

> > +(defcustom dired-filename-hiding-ellipsis "…"
>
> We shouldn't use non-ASCII characters by default, without checking
> they can be displayed.

Thanks for pointing this out. I think it can be changed to "...".

> > +    (let ((ell-len (string-width dired-filename-hiding-ellipsis))
> > +          maxlen filename-col)
> > +      (while (< (point) end)
> > +        (ignore-errors
> > +       (if (not (dired-move-to-filename))
>
> Did you consider using truncate-string-to-width here?

I'm not sure what you mean. Using truncate-string-to-width to shorten
string or using truncate-string-ellipsis to obtain the ellipsis? The
problem of truncate-string-to-width is that it produces a new string.
I want to avoid modifying the buffer text since it would break other
dired functionalities. Therefore this patch only changes the display
instead of the underlying text.

Message #14 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: 67161 <at> debbugs.gnu.org
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Wed, 15 Nov 2023 10:54:36 -0500

> +(defcustom dired-filename-hiding-ellipsis "…"
[...]
> +(defun dired-filename-update-invisibility-spec ()
> +  (funcall (if (derived-mode-p 'dired-mode)
> +               'add-to-invisibility-spec
> +             'remove-from-invisibility-spec)
> +           `(dired-filename-hide . ,dired-filename-hiding-ellipsis)))

AFAIK this does not do what the code suggests: the ELLIPSIS part
of the element passed to `add-to-invisibility-spec` is treated as
a boolean.

How the ellipses are displayed is controlled by the `selective-display`
extra slot of the display table.

I think it's good that you use `invisible`, tho, specifically since it
then uses whichever ellipsis the user has chosen via the display-table.
The downside is that most users don't know how to change it and there's
no convenient facility to access it.  You probably need something like

    (defun length-of-standard-ellipsis ()
      (let ((glyphs
             (and standard-display-table
                  (display-table-slot standard-display-table
                                      'selective-display))))
        (if glyphs (length glyphs) (eval-when-compile (length "...")))))


-- Stefan

Message #17 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>, Stefan Kangas <stefankangas <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67161 <at> debbugs.gnu.org
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Wed, 15 Nov 2023 14:32:30 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Wed, 15 Nov 2023 18:04:13 +0800
> Cc: 67161 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> 于2023年11月14日周二 21:26写道：
> 
> > > +(defcustom dired-filename-hiding-ellipsis "…"
> >
> > We shouldn't use non-ASCII characters by default, without checking
> > they can be displayed.
> 
> Thanks for pointing this out. I think it can be changed to "...".

I actually wonder why we need it as a defcustom.  If you use
truncate-string-ellipsis, you will get both a customizable setting and
the correct string for the terminal at hand.  Why do we need an extra
option?

> > > +    (let ((ell-len (string-width dired-filename-hiding-ellipsis))
> > > +          maxlen filename-col)
> > > +      (while (< (point) end)
> > > +        (ignore-errors
> > > +       (if (not (dired-move-to-filename))
> >
> > Did you consider using truncate-string-to-width here?
> 
> I'm not sure what you mean. Using truncate-string-to-width to shorten
> string or using truncate-string-ellipsis to obtain the ellipsis? The
> problem of truncate-string-to-width is that it produces a new string.
> I want to avoid modifying the buffer text since it would break other
> dired functionalities. Therefore this patch only changes the display
> instead of the underlying text.

So you consider using the 'invisible' property a good feature?  I
actually don't like using it for such purposes, as it gets in the way.
But maybe as an opt-in behavior that could be OK?

Stefan & Stefan, WDYT?

Btw, using string-width might not be the best possibility here.  I
would recommend string-pixel-width instead (with subsequent division
by what default-char-width returns), as that will produce a better
approximation, especially on GUI frames.

Message #20 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Liu Hui <liuhui1610 <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: "67161 <at> debbugs.gnu.org" <67161 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Wed, 15 Nov 2023 18:06:42 +0000

> > We shouldn't use non-ASCII characters by default, without checking
> > they can be displayed.
> 
> Thanks for pointing this out. I think it can be changed to "...".

+1.

Would someone please do likewise for `dired-sort-set-mode-line'?
My version (in Dired+) uses "...", not an ellipsis char.

FWIW, IMO an ellipsis char is _bad_ when the font used is fixed
width.  It's not very legible, in particular.
___


FWIW2: The other difference my version has is
that it respects a user option:
___

diredp-switches-in-mode-line is a variable defined in `dired+.el'.

Its value is nil

How to indicate `dired-actual-switches' in mode-line.
Possible values:
 * `nil':    Indicate name-or-date sort order, if possible.
             Else show full switches.
 * `as-is':  Show full switches.
 * Integer:  Show only the first N chars of full switches.
 * Function: Pass `dired-actual-switches' as arg and show result.
___

If vanilla Emacs did likewise or something similar then I
wouldn't need my version of `dired-sort-set-mode-line'
(for new Emacs versions, at least).

Message #23 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67161 <at> debbugs.gnu.org
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Thu, 16 Nov 2023 11:44:06 +0800

Stefan Monnier <monnier <at> iro.umontreal.ca> 于2023年11月15日周三 23:57写道：

> I think it's good that you use `invisible`, tho, specifically since it
> then uses whichever ellipsis the user has chosen via the display-table.
> The downside is that most users don't know how to change it and there's
> no convenient facility to access it.  You probably need something like
>
>     (defun length-of-standard-ellipsis ()
>       (let ((glyphs
>              (and standard-display-table
>                   (display-table-slot standard-display-table
>                                       'selective-display))))
>         (if glyphs (length glyphs) (eval-when-compile (length "...")))))

Thanks very much for pointing out the problem! Your suggestion is
valuable, and I will update the patch later.

Message #26 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67161 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Thu, 16 Nov 2023 18:07:04 +0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> 于2023年11月15日周三 20:32写道：

> I actually wonder why we need it as a defcustom.  If you use
> truncate-string-ellipsis, you will get both a customizable setting and
> the correct string for the terminal at hand.  Why do we need an extra
> option?

As Stefan promptly pointed out, the ellipsis defcustom is actually
useless because the display of ellipsis is determined by the display
table. So I have deleted it in the attached patch.

> So you consider using the 'invisible' property a good feature?  I
> actually don't like using it for such purposes, as it gets in the way.

I don't know the drawback of the 'invisible' property. This patch is
inspired by dired-hide-details-mode, which uses the property to hide
file information.

> Btw, using string-width might not be the best possibility here.  I
> would recommend string-pixel-width instead (with subsequent division
> by what default-char-width returns), as that will produce a better
> approximation, especially on GUI frames.

I agree using pixel width is more accurate, though the approximation
based on character width is good enough in most cases. In fact, in
addition to string-width, move-to-column also needs to be replaced by
a function that moves the point to the character at a specific pixel
position. It seems there isn't such a built-in function.

[0001-Add-option-dired-filename-display-length.patch (text/x-patch, attachment)]

Message #29 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: 67161 <at> debbugs.gnu.org, stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Thu, 16 Nov 2023 14:11:35 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Thu, 16 Nov 2023 18:07:04 +0800
> Cc: Stefan Kangas <stefankangas <at> gmail.com>, Stefan Monnier <monnier <at> iro.umontreal.ca>, 
> 	67161 <at> debbugs.gnu.org
> 
> From 991ea588df6799331a7feea9e83493ef0d724096 Mon Sep 17 00:00:00 2001
> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Tue, 14 Nov 2023 16:14:12 +0800
> Subject: [PATCH] Add option `dired-filename-display-length'
> 
> * lisp/dired.el (dired-filename-display-length): New option.
> (dired-insert-set-properties): Set invisibility spec for long
> filenames.
> (dired--get-ellipsis-length)
> (dired--get-filename-display-length)
> (dired-filename-update-invisibility-spec): New functions.
> (dired-mode): Add filename invisibility spec.
> (dired-make-directory-clickable):
> (dired-kill-when-opening-new-dired-buffer):
> (dired-hide-details-preserved-columns): Add missing group.
> * lisp/wdired.el (wdired-change-to-wdired-mode)
> (wdired-change-to-dired-mode): Update filename invisibility spec.

These changes need a NEWS entry to describe the new option and its
effect in short.

> diff --git a/lisp/dired.el b/lisp/dired.el
> index 8919d2c223f..aad77a3dfc0 100644
> --- a/lisp/dired.el
> +++ b/lisp/dired.el
> @@ -350,6 +350,7 @@ dired-after-readin-hook
>  (defcustom dired-make-directory-clickable t
>    "When non-nil, make the directory at the start of the dired buffer clickable."
>    :version "29.1"
> +  :group 'dired
>    :type 'boolean)
>  
>  (defcustom dired-initial-position-hook nil
> @@ -429,6 +430,7 @@ dired-mark-region
>  (defcustom dired-kill-when-opening-new-dired-buffer nil
>    "If non-nil, kill the current buffer when selecting a new directory."
>    :type 'boolean
> +  :group 'dired
>    :version "28.1")
>  
>  (defcustom dired-guess-shell-case-fold-search t
> @@ -515,6 +517,18 @@ dired-movement-style
>  (defcustom dired-hide-details-preserved-columns nil
>    "List of columns which are not hidden in `dired-hide-details-mode'."
>    :type '(repeat integer)
> +  :group 'dired
> +  :version "30.1")

Why do we need an explicit :group tag here?  Options whose group is
not 'dired' indeed need it, but those whose group is 'dired' get it
implicitly, AFAIU.

> +(defcustom dired-filename-display-length nil
> +  "If non-nil, hide middle part of long filenames in Dired buffers.
> +If the value is the symbol `window', then filenames are shortened
> +to not exceed the right edge of current window.  Otherwise, it
> +should be an integer representing the maximum filename length."

This should tell more about the effect: that portions of longer file
names are hidden by using the 'invisible' property and that the
ellipsis is displayed in their stead.

> +(defun dired--get-ellipsis-length ()
> +  "Return length of ellipsis."
> +  (let* ((dt (or (window-display-table)
> +                 buffer-display-table
> +                 standard-display-table))
> +         (glyphs (and dt (display-table-slot dt 'selective-display))))
> +    (if glyphs (length glyphs) (eval-when-compile (length "...")))))

Why do you use 'length' here and not 'string-width' or similar?  You
seem to assume that each character takes just one column on display?

> +(defun dired--get-filename-display-length ()
> +  "Return maximum display length of filename."

This doc string is inaccurate.  The function actually returns the
number of columns available for displaying the file names in a Dired
buffer, and it should be called with point at the first character of
the file name.

> +(defun dired-filename-update-invisibility-spec ()

This function should have a doc string.

> diff --git a/lisp/wdired.el b/lisp/wdired.el
> index 079d93d6011..5d50a574290 100644
> --- a/lisp/wdired.el
> +++ b/lisp/wdired.el
> @@ -261,6 +261,7 @@ wdired-change-to-wdired-mode
>    (add-function :override (local 'revert-buffer-function) #'wdired-revert)
>    (set-buffer-modified-p nil)
>    (setq buffer-undo-list nil)
> +  (dired-filename-update-invisibility-spec)
>    (run-mode-hooks 'wdired-mode-hook)
>    (message "%s" (substitute-command-keys
>  		 "Press \\[wdired-finish-edit] when finished \
> @@ -456,6 +457,7 @@ wdired-change-to-dired-mode
>    (dired-sort-set-mode-line)
>    (dired-advertise)
>    (dired-hide-details-update-invisibility-spec)
> +  (dired-filename-update-invisibility-spec)

Please add comments in these two places explaining why you call
dired-filename-update-invisibility-spec here, so that the reader won't
need to look at that function to understand the reason.

Thanks.

Message #32 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67161 <at> debbugs.gnu.org, stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Sat, 18 Nov 2023 17:23:26 +0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> 于2023年11月16日周四 20:11写道：
>
> > From: Liu Hui <liuhui1610 <at> gmail.com>
> > Date: Thu, 16 Nov 2023 18:07:04 +0800
> > Cc: Stefan Kangas <stefankangas <at> gmail.com>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
> >       67161 <at> debbugs.gnu.org
> >
> > From 991ea588df6799331a7feea9e83493ef0d724096 Mon Sep 17 00:00:00 2001
> > From: Liu Hui <liuhui1610 <at> gmail.com>
> > Date: Tue, 14 Nov 2023 16:14:12 +0800
> > Subject: [PATCH] Add option `dired-filename-display-length'
> >
> > * lisp/dired.el (dired-filename-display-length): New option.
> > (dired-insert-set-properties): Set invisibility spec for long
> > filenames.
> > (dired--get-ellipsis-length)
> > (dired--get-filename-display-length)
> > (dired-filename-update-invisibility-spec): New functions.
> > (dired-mode): Add filename invisibility spec.
> > (dired-make-directory-clickable):
> > (dired-kill-when-opening-new-dired-buffer):
> > (dired-hide-details-preserved-columns): Add missing group.
> > * lisp/wdired.el (wdired-change-to-wdired-mode)
> > (wdired-change-to-dired-mode): Update filename invisibility spec.
>
> These changes need a NEWS entry to describe the new option and its
> effect in short.

Added.

> > diff --git a/lisp/dired.el b/lisp/dired.el
> > index 8919d2c223f..aad77a3dfc0 100644
> > --- a/lisp/dired.el
> > +++ b/lisp/dired.el
> > @@ -350,6 +350,7 @@ dired-after-readin-hook
> >  (defcustom dired-make-directory-clickable t
> >    "When non-nil, make the directory at the start of the dired buffer clickable."
> >    :version "29.1"
> > +  :group 'dired
> >    :type 'boolean)
> >
> >  (defcustom dired-initial-position-hook nil
> > @@ -429,6 +430,7 @@ dired-mark-region
> >  (defcustom dired-kill-when-opening-new-dired-buffer nil
> >    "If non-nil, kill the current buffer when selecting a new directory."
> >    :type 'boolean
> > +  :group 'dired
> >    :version "28.1")
> >
> >  (defcustom dired-guess-shell-case-fold-search t
> > @@ -515,6 +517,18 @@ dired-movement-style
> >  (defcustom dired-hide-details-preserved-columns nil
> >    "List of columns which are not hidden in `dired-hide-details-mode'."
> >    :type '(repeat integer)
> > +  :group 'dired
> > +  :version "30.1")
>
> Why do we need an explicit :group tag here?  Options whose group is
> not 'dired' indeed need it, but those whose group is 'dired' get it
> implicitly, AFAIU.

I find these options are implicitly assigned to the dired-guess group
rather than dired group, since dired-guess is the last group.

> > +(defcustom dired-filename-display-length nil
> > +  "If non-nil, hide middle part of long filenames in Dired buffers.
> > +If the value is the symbol `window', then filenames are shortened
> > +to not exceed the right edge of current window.  Otherwise, it
> > +should be an integer representing the maximum filename length."
>
> This should tell more about the effect: that portions of longer file
> names are hidden by using the 'invisible' property and that the
> ellipsis is displayed in their stead.

Done.

> > +(defun dired--get-ellipsis-length ()
> > +  "Return length of ellipsis."
> > +  (let* ((dt (or (window-display-table)
> > +                 buffer-display-table
> > +                 standard-display-table))
> > +         (glyphs (and dt (display-table-slot dt 'selective-display))))
> > +    (if glyphs (length glyphs) (eval-when-compile (length "...")))))
>
> Why do you use 'length' here and not 'string-width' or similar?  You
> seem to assume that each character takes just one column on display?

Fixed.

> > +(defun dired--get-filename-display-length ()
> > +  "Return maximum display length of filename."
>
> This doc string is inaccurate.  The function actually returns the
> number of columns available for displaying the file names in a Dired
> buffer, and it should be called with point at the first character of
> the file name.

Why is it inaccurate? When `dired-filename-display-length' is window,
the function does return the number of columns available, and it is
used as the maximum display length. Moreover, it could be an integer,
which also means the maximum display length.

> > +(defun dired-filename-update-invisibility-spec ()
>
> This function should have a doc string.

Added.

> > diff --git a/lisp/wdired.el b/lisp/wdired.el
> > index 079d93d6011..5d50a574290 100644
> > --- a/lisp/wdired.el
> > +++ b/lisp/wdired.el
> > @@ -261,6 +261,7 @@ wdired-change-to-wdired-mode
> >    (add-function :override (local 'revert-buffer-function) #'wdired-revert)
> >    (set-buffer-modified-p nil)
> >    (setq buffer-undo-list nil)
> > +  (dired-filename-update-invisibility-spec)
> >    (run-mode-hooks 'wdired-mode-hook)
> >    (message "%s" (substitute-command-keys
> >                "Press \\[wdired-finish-edit] when finished \
> > @@ -456,6 +457,7 @@ wdired-change-to-dired-mode
> >    (dired-sort-set-mode-line)
> >    (dired-advertise)
> >    (dired-hide-details-update-invisibility-spec)
> > +  (dired-filename-update-invisibility-spec)
>
> Please add comments in these two places explaining why you call
> dired-filename-update-invisibility-spec here, so that the reader won't
> need to look at that function to understand the reason.

Added.

> Thanks.

BTW, I just find isearch doesn't handle hidden filenames based on
'invisible' text property, which needs to be fixed. So maybe an
overlay-based approach is better? though I don't know if there is any
possible problem with using overlays for this feature. WDYT?

[0001-Add-option-dired-filename-display-length.patch (text/x-patch, attachment)]

Message #35 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: 67161 <at> debbugs.gnu.org, stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Sat, 18 Nov 2023 12:55:16 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Sat, 18 Nov 2023 17:23:26 +0800
> Cc: stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca, 67161 <at> debbugs.gnu.org
> 
> > > +(defun dired--get-filename-display-length ()
> > > +  "Return maximum display length of filename."
> >
> > This doc string is inaccurate.  The function actually returns the
> > number of columns available for displaying the file names in a Dired
> > buffer, and it should be called with point at the first character of
> > the file name.
> 
> Why is it inaccurate? When `dired-filename-display-length' is window,
> the function does return the number of columns available, and it is
> used as the maximum display length. Moreover, it could be an integer,
> which also means the maximum display length.

If there are two different modes of using this function, the doc
string could describe them both.  Currently, it only describes the use
case where dired-filename-display-length is an integer, in which case
it is a simple accessor.  The other use case is more subtle, and the
doc string is completely silent about that, in particular it doesn't
mention that point should be at the first character of the file name
in that case.

> BTW, I just find isearch doesn't handle hidden filenames based on
> 'invisible' text property, which needs to be fixed. So maybe an
> overlay-based approach is better? though I don't know if there is any
> possible problem with using overlays for this feature. WDYT?

Doesn't "M-x i" during isearch handle this?

> --- a/lisp/wdired.el
> +++ b/lisp/wdired.el
> @@ -261,6 +261,9 @@ wdired-change-to-wdired-mode
>    (add-function :override (local 'revert-buffer-function) #'wdired-revert)
>    (set-buffer-modified-p nil)
>    (setq buffer-undo-list nil)
> +  ;; remove filename invisibility spec to ensure filenames are visible
> +  ;; for editing

Comments should be complete sentences: begin with a capital letter and
end with a period.  Also, please mention here the new defcustom you
add to Dired, so that the comment could better explain itself.

Thanks.

Message #38 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Liu Hui <liuhui1610 <at> gmail.com>
Cc: "67161 <at> debbugs.gnu.org" <67161 <at> debbugs.gnu.org>,
 "stefankangas <at> gmail.com" <stefankangas <at> gmail.com>,
 "monnier <at> iro.umontreal.ca" <monnier <at> iro.umontreal.ca>
Subject: RE: [External] : bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Sat, 18 Nov 2023 16:12:34 +0000

> Doesn't "M-x i" during isearch handle this?

Typo, I think.  `M-s i'.

Message #41 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67161 <at> debbugs.gnu.org, stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Mon, 20 Nov 2023 12:34:13 +0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> 于2023年11月18日周六 18:55写道：

> If there are two different modes of using this function, the doc
> string could describe them both.  Currently, it only describes the use
> case where dired-filename-display-length is an integer, in which case
> it is a simple accessor.  The other use case is more subtle, and the
> doc string is completely silent about that, in particular it doesn't
> mention that point should be at the first character of the file name
> in that case.

Thanks for the explanation. I have updated the doc string as suggested.

> > BTW, I just find isearch doesn't handle hidden filenames based on
> > 'invisible' text property, which needs to be fixed. So maybe an
> > overlay-based approach is better? though I don't know if there is any
> > possible problem with using overlays for this feature. WDYT?
>
> Doesn't "M-x i" during isearch handle this?

No. I want to search text in hidden part rather than only in the
visible part, which is necessary for users to find relevant files even
if they are partly hidden.

Here is an example illustrating the problem:
1. emacs -Q
2. type M-: (insert "visible " (propertize "hidden" 'invisible t))
3. C-s hidden

isearch fails to match the invisible text by default, unless
search-invisible is changed to t. However, the default value of
search-invisible is open and it cannot be changed to t during isearch
by M-s i.

Therefore, I would like to change

  (put-text-property ell-beg (point) 'invisible 'dired-filename-hide)

to

  (let ((ov (make-overlay ell-beg (point))))
    (overlay-put ov 'invisible 'dired-filename-hide)
    (overlay-put ov 'isearch-open-invisible t)
    (overlay-put ov 'evaporate t))

in the attached patch, then text in hidden part can be matched
regardless of search-invisible being open or t. It also has a bonus
that hidden text can be revealed during isearch.

> > --- a/lisp/wdired.el
> > +++ b/lisp/wdired.el
> > @@ -261,6 +261,9 @@ wdired-change-to-wdired-mode
> >    (add-function :override (local 'revert-buffer-function) #'wdired-revert)
> >    (set-buffer-modified-p nil)
> >    (setq buffer-undo-list nil)
> > +  ;; remove filename invisibility spec to ensure filenames are visible
> > +  ;; for editing
>
> Comments should be complete sentences: begin with a capital letter and
> end with a period.  Also, please mention here the new defcustom you
> add to Dired, so that the comment could better explain itself.
>
> Thanks.

Fixed.

[0001-Add-option-dired-filename-display-length.patch (text/x-patch, attachment)]

Message #44 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>, Juri Linkov <juri <at> linkov.net>
Cc: 67161 <at> debbugs.gnu.org, stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Mon, 20 Nov 2023 14:10:23 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Mon, 20 Nov 2023 12:34:13 +0800
> Cc: stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca, 67161 <at> debbugs.gnu.org
> 
> > > BTW, I just find isearch doesn't handle hidden filenames based on
> > > 'invisible' text property, which needs to be fixed. So maybe an
> > > overlay-based approach is better? though I don't know if there is any
> > > possible problem with using overlays for this feature. WDYT?
> >
> > Doesn't "M-s i" during isearch handle this?
> 
> No. I want to search text in hidden part rather than only in the
> visible part, which is necessary for users to find relevant files even
> if they are partly hidden.
> 
> Here is an example illustrating the problem:
> 1. emacs -Q
> 2. type M-: (insert "visible " (propertize "hidden" 'invisible t))
> 3. C-s hidden
> 
> isearch fails to match the invisible text by default, unless
> search-invisible is changed to t. However, the default value of
> search-invisible is open and it cannot be changed to t during isearch
> by M-s i.
> 
> Therefore, I would like to change
> 
>   (put-text-property ell-beg (point) 'invisible 'dired-filename-hide)
> 
> to
> 
>   (let ((ov (make-overlay ell-beg (point))))
>     (overlay-put ov 'invisible 'dired-filename-hide)
>     (overlay-put ov 'isearch-open-invisible t)
>     (overlay-put ov 'evaporate t))
> 
> in the attached patch, then text in hidden part can be matched
> regardless of search-invisible being open or t. It also has a bonus
> that hidden text can be revealed during isearch.

Juri, this seems to be a usability bug?  Why does Isearch only look
into invisible overlay properties, but not text properties?  IOW, why
doesn't "M-s i" set the value of isearch-invisible to t, not 'open'?
Or maybe 'open' should also search inside text properties, not just
overlays?  In any case, the differences between properties and
overlays is unusual and IMO a misfeature.  Can this be fixed?

Message #47 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Liu Hui <liuhui1610 <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: "67161 <at> debbugs.gnu.org" <67161 <at> debbugs.gnu.org>,
 "stefankangas <at> gmail.com" <stefankangas <at> gmail.com>,
 "monnier <at> iro.umontreal.ca" <monnier <at> iro.umontreal.ca>
Subject: RE: [External] : bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Mon, 20 Nov 2023 17:20:58 +0000

> 1. emacs -Q
> 2. type M-: (insert "visible " (propertize "hidden" 'invisible t))
> 3. C-s hidden

Just set or bind `isearch-filter-predicate' to
`isearch-range-invisible', to search only invisible text.

`C-h f isearch-range-invisible':

  isearch-range-invisible is a compiled Lisp function in ‘isearch.el’.

  (isearch-range-invisible BEG END)

  Return t if all the text from BEG to END is invisible.

Message #50 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Liu Hui <liuhui1610 <at> gmail.com>, 67161 <at> debbugs.gnu.org,
 stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Mon, 20 Nov 2023 19:54:27 +0200

>> Therefore, I would like to change
>>
>>   (put-text-property ell-beg (point) 'invisible 'dired-filename-hide)
>>
>> to
>>
>>   (let ((ov (make-overlay ell-beg (point))))
>>     (overlay-put ov 'invisible 'dired-filename-hide)
>>     (overlay-put ov 'isearch-open-invisible t)
>>     (overlay-put ov 'evaporate t))
>
> Juri, this seems to be a usability bug?  Why does Isearch only look
> into invisible overlay properties, but not text properties?  IOW, why
> doesn't "M-s i" set the value of isearch-invisible to t, not 'open'?
> Or maybe 'open' should also search inside text properties, not just
> overlays?  In any case, the differences between properties and
> overlays is unusual and IMO a misfeature.  Can this be fixed?

I recall Ihor implemented the revealing of invisible text properties
for Org-mode when overlays were very slow.  But now overlays are
optimized, so Liu is absolutely right that better to use overlays.

Message #53 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: liuhui1610 <at> gmail.com, 67161 <at> debbugs.gnu.org, stefankangas <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Mon, 20 Nov 2023 20:42:30 +0200

> From: Juri Linkov <juri <at> linkov.net>
> Cc: Liu Hui <liuhui1610 <at> gmail.com>,  stefankangas <at> gmail.com,
>   monnier <at> iro.umontreal.ca,  67161 <at> debbugs.gnu.org
> Date: Mon, 20 Nov 2023 19:54:27 +0200
> 
> >> Therefore, I would like to change
> >>
> >>   (put-text-property ell-beg (point) 'invisible 'dired-filename-hide)
> >>
> >> to
> >>
> >>   (let ((ov (make-overlay ell-beg (point))))
> >>     (overlay-put ov 'invisible 'dired-filename-hide)
> >>     (overlay-put ov 'isearch-open-invisible t)
> >>     (overlay-put ov 'evaporate t))
> >
> > Juri, this seems to be a usability bug?  Why does Isearch only look
> > into invisible overlay properties, but not text properties?  IOW, why
> > doesn't "M-s i" set the value of isearch-invisible to t, not 'open'?
> > Or maybe 'open' should also search inside text properties, not just
> > overlays?  In any case, the differences between properties and
> > overlays is unusual and IMO a misfeature.  Can this be fixed?
> 
> I recall Ihor implemented the revealing of invisible text properties
> for Org-mode when overlays were very slow.  But now overlays are
> optimized, so Liu is absolutely right that better to use overlays.

That might be so, but I was asking about Isearch being able to find
matches inside invisible text, not about the Dired feature.  Why
cannot a user tell Isearch to find text that is invisible due to text
properties?

Message #56 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Liu Hui <liuhui1610 <at> gmail.com>, 67161 <at> debbugs.gnu.org,
 stefankangas <at> gmail.com, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Mon, 20 Nov 2023 13:55:58 -0500

>> in the attached patch, then text in hidden part can be matched
>> regardless of search-invisible being open or t. It also has a bonus
>> that hidden text can be revealed during isearch.

Yes, using an overlay sounds like a better choice here, thanks.

> Juri, this seems to be a usability bug?  Why does Isearch only look
> into invisible overlay properties, but not text properties?  IOW, why
> doesn't "M-s i" set the value of isearch-invisible to t, not 'open'?

I think the idea is that matching invisible text is somewhat
inconvenient, so it's better to temporarily reveal/open that text.
But I think it points to an bug in Isearch: when `isearch-invisible` is
`open` we should match text inside invisible text, regardless if we can
reveal/open that invisible text or not.  IOW `open` should find the
same matches as `t`, the only difference being that it additionally
(tries to) reveal/open the text.

> In any case, the differences between properties and
> overlays is unusual and IMO a misfeature.  Can this be fixed?

Indeed, it would be good to implement some way to reveal/open invisible
text made invisible via text-properties rather than overlays.
That also applies to `reveal-mode`, of course.


        Stefan

Message #59 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: liuhui1610 <at> gmail.com, 67161 <at> debbugs.gnu.org, stefankangas <at> gmail.com,
 juri <at> linkov.net
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Mon, 20 Nov 2023 21:17:35 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Liu Hui <liuhui1610 <at> gmail.com>,  Juri Linkov <juri <at> linkov.net>,
>   stefankangas <at> gmail.com,  67161 <at> debbugs.gnu.org
> Date: Mon, 20 Nov 2023 13:55:58 -0500
> 
> > Juri, this seems to be a usability bug?  Why does Isearch only look
> > into invisible overlay properties, but not text properties?  IOW, why
> > doesn't "M-s i" set the value of isearch-invisible to t, not 'open'?
> 
> I think the idea is that matching invisible text is somewhat
> inconvenient, so it's better to temporarily reveal/open that text.
> But I think it points to an bug in Isearch: when `isearch-invisible` is
> `open` we should match text inside invisible text, regardless if we can
> reveal/open that invisible text or not.  IOW `open` should find the
> same matches as `t`, the only difference being that it additionally
> (tries to) reveal/open the text.

Agreed.

Message #62 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Liu Hui <liuhui1610 <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
 67161 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Tue, 21 Nov 2023 09:52:02 +0200

>>> in the attached patch, then text in hidden part can be matched
>>> regardless of search-invisible being open or t. It also has a bonus
>>> that hidden text can be revealed during isearch.
>
> Yes, using an overlay sounds like a better choice here, thanks.
>
>> Juri, this seems to be a usability bug?  Why does Isearch only look
>> into invisible overlay properties, but not text properties?  IOW, why
>> doesn't "M-s i" set the value of isearch-invisible to t, not 'open'?
>
> I think the idea is that matching invisible text is somewhat
> inconvenient, so it's better to temporarily reveal/open that text.
> But I think it points to an bug in Isearch: when `isearch-invisible` is
> `open` we should match text inside invisible text, regardless if we can
> reveal/open that invisible text or not.  IOW `open` should find the
> same matches as `t`, the only difference being that it additionally
> (tries to) reveal/open the text.

This could be a new value like `open-or-match` with a trivial change:

@@ -3899,6 +3899,9 @@ isearch-filter-visible
 of what `isearch-range-invisible' says."
   (and (not (text-property-not-all beg end 'inhibit-isearch nil))
        (or (eq search-invisible t)
+           (when (eq search-invisible 'open-or-match)
+             (isearch-range-invisible beg end)
+             t)
            (not (isearch-range-invisible beg end)))))

But I doubt that it could be enabled by default because in many cases
it's too confusing for users to stop at a match that is not visible.

OTOH, there was a request about informing the user about hidden matches.
So now the Isearch prompt shows the number of invisible matches.
Having this information the user can reveal in some way and visually
inspect these matches.

>> In any case, the differences between properties and
>> overlays is unusual and IMO a misfeature.  Can this be fixed?
>
> Indeed, it would be good to implement some way to reveal/open invisible
> text made invisible via text-properties rather than overlays.
> That also applies to `reveal-mode`, of course.

Would be nice to have such useful feature.

Message #65 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: liuhui1610 <at> gmail.com, 67161 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 stefankangas <at> gmail.com
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Tue, 21 Nov 2023 13:55:08 +0200

> From: Juri Linkov <juri <at> linkov.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  Liu Hui <liuhui1610 <at> gmail.com>,
>   stefankangas <at> gmail.com,  67161 <at> debbugs.gnu.org
> Date: Tue, 21 Nov 2023 09:52:02 +0200
> 
> > I think the idea is that matching invisible text is somewhat
> > inconvenient, so it's better to temporarily reveal/open that text.
> > But I think it points to an bug in Isearch: when `isearch-invisible` is
> > `open` we should match text inside invisible text, regardless if we can
> > reveal/open that invisible text or not.  IOW `open` should find the
> > same matches as `t`, the only difference being that it additionally
> > (tries to) reveal/open the text.
> 
> This could be a new value like `open-or-match` with a trivial change:
> 
> @@ -3899,6 +3899,9 @@ isearch-filter-visible
>  of what `isearch-range-invisible' says."
>    (and (not (text-property-not-all beg end 'inhibit-isearch nil))
>         (or (eq search-invisible t)
> +           (when (eq search-invisible 'open-or-match)
> +             (isearch-range-invisible beg end)
> +             t)
>             (not (isearch-range-invisible beg end)))))
> 
> But I doubt that it could be enabled by default because in many cases
> it's too confusing for users to stop at a match that is not visible.

Isn't it possible to unhide the invisible text by temporarily
manipulating buffer-invisibility-spec?

Message #68 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: liuhui1610 <at> gmail.com, 67161 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 stefankangas <at> gmail.com
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Tue, 21 Nov 2023 19:12:10 +0200

>> > I think the idea is that matching invisible text is somewhat
>> > inconvenient, so it's better to temporarily reveal/open that text.
>> > But I think it points to an bug in Isearch: when `isearch-invisible` is
>> > `open` we should match text inside invisible text, regardless if we can
>> > reveal/open that invisible text or not.  IOW `open` should find the
>> > same matches as `t`, the only difference being that it additionally
>> > (tries to) reveal/open the text.
>>
>> This could be a new value like `open-or-match` with a trivial change:
>>
>> @@ -3899,6 +3899,9 @@ isearch-filter-visible
>>  of what `isearch-range-invisible' says."
>>    (and (not (text-property-not-all beg end 'inhibit-isearch nil))
>>         (or (eq search-invisible t)
>> +           (when (eq search-invisible 'open-or-match)
>> +             (isearch-range-invisible beg end)
>> +             t)
>>             (not (isearch-range-invisible beg end)))))
>>
>> But I doubt that it could be enabled by default because in many cases
>> it's too confusing for users to stop at a match that is not visible.
>
> Isn't it possible to unhide the invisible text by temporarily
> manipulating buffer-invisibility-spec?

Do you mean unhiding text properties?

Message #71 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67161 <at> debbugs.gnu.org, stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Wed, 22 Nov 2023 13:41:18 +0800

[Message part 1 (text/plain, inline)]

> Therefore, I would like to change
>
>   (put-text-property ell-beg (point) 'invisible 'dired-filename-hide)
>
> to
>
>   (let ((ov (make-overlay ell-beg (point))))
>     (overlay-put ov 'invisible 'dired-filename-hide)
>     (overlay-put ov 'isearch-open-invisible t)
>     (overlay-put ov 'evaporate t))
>
> in the attached patch, then text in hidden part can be matched
> regardless of search-invisible being open or t. It also has a bonus
> that hidden text can be revealed during isearch.

Since there is no objection to using overlay, I have updated the patch
accordingly with additional explanation. Thanks.

[0001-Add-option-dired-filename-display-length.patch (text/x-patch, attachment)]

Message #76 received at 67161-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: 67161-done <at> debbugs.gnu.org, stefankangas <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Sat, 25 Nov 2023 12:52:34 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Wed, 22 Nov 2023 13:41:18 +0800
> Cc: stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca, 67161 <at> debbugs.gnu.org
> 
> > Therefore, I would like to change
> >
> >   (put-text-property ell-beg (point) 'invisible 'dired-filename-hide)
> >
> > to
> >
> >   (let ((ov (make-overlay ell-beg (point))))
> >     (overlay-put ov 'invisible 'dired-filename-hide)
> >     (overlay-put ov 'isearch-open-invisible t)
> >     (overlay-put ov 'evaporate t))
> >
> > in the attached patch, then text in hidden part can be matched
> > regardless of search-invisible being open or t. It also has a bonus
> > that hidden text can be revealed during isearch.
> 
> Since there is no objection to using overlay, I have updated the patch
> accordingly with additional explanation. Thanks.

Thanks, installed on the master branch, and closing the bug.

Please in the future always mention the bug number as part of the
commit log message, when the bug number is known.

Message #79 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: 67161 <at> debbugs.gnu.org
Cc: liuhui1610 <at> gmail.com, eliz <at> gnu.org
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Sat, 25 Nov 2023 19:51:55 +0200

> Thanks, installed on the master branch, and closing the bug.

There is a problem with this specific configuration
when ellipsis is a vector:

  (progn
    (or standard-display-table (setq standard-display-table (make-display-table)))
    (let* ((face (lsh (face-id 'escape-glyph) 22))
           (dots (+ face ?…)))
      (set-char-table-extra-slot standard-display-table 4 (vector dots))))

that fails with

  Debugger entered--Lisp error: (wrong-type-argument characterp 88088614)
    concat([88088614])
    (if glyphs (concat glyphs) "...")
    (string-width (if glyphs (concat glyphs) "..."))
    dired--get-ellipsis-length()
    ...

Message #82 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: liuhui1610 <at> gmail.com, 67161 <at> debbugs.gnu.org
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Sat, 25 Nov 2023 22:02:38 +0200

> From: Juri Linkov <juri <at> linkov.net>
> Cc: eliz <at> gnu.org,  liuhui1610 <at> gmail.com
> Date: Sat, 25 Nov 2023 19:51:55 +0200
> 
> > Thanks, installed on the master branch, and closing the bug.
> 
> There is a problem with this specific configuration
> when ellipsis is a vector:
> 
>   (progn
>     (or standard-display-table (setq standard-display-table (make-display-table)))
>     (let* ((face (lsh (face-id 'escape-glyph) 22))
>            (dots (+ face ?…)))
>       (set-char-table-extra-slot standard-display-table 4 (vector dots))))
> 
> that fails with
> 
>   Debugger entered--Lisp error: (wrong-type-argument characterp 88088614)
>     concat([88088614])
>     (if glyphs (concat glyphs) "...")
>     (string-width (if glyphs (concat glyphs) "..."))
>     dired--get-ellipsis-length()
>     ...
> 

Does the below fix the problem, and is the Dired display after the
change correct when file names are truncated?

diff --git a/lisp/dired.el b/lisp/dired.el
index 23a6fc0..096d6a8 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -2021,8 +2021,12 @@ dired--get-ellipsis-length
   (let* ((dt (or (window-display-table)
                  buffer-display-table
                  standard-display-table))
-         (glyphs (and dt (display-table-slot dt 'selective-display))))
-    (string-width (if glyphs (concat glyphs) "..."))))
+         (glyphs (and dt (display-table-slot dt 'selective-display)))
+         (vlen (length glyphs))
+         (char-glyphs (make-vector vlen nil)))
+    (dotimes (i vlen)
+      (aset char-glyphs i (glyph-char (aref glyphs i))))
+    (string-width (if glyphs (concat char-glyphs) "..."))))
 
 (defun dired--get-filename-display-length ()
   "Return maximum display length of filename.

Message #85 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67161 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Sun, 26 Nov 2023 10:56:52 +0800

Eli Zaretskii <eliz <at> gnu.org> 于2023年11月26日周日 04:02写道：

> Does the below fix the problem, and is the Dired display after the
> change correct when file names are truncated?
>
> diff --git a/lisp/dired.el b/lisp/dired.el
> index 23a6fc0..096d6a8 100644
> --- a/lisp/dired.el
> +++ b/lisp/dired.el
> @@ -2021,8 +2021,12 @@ dired--get-ellipsis-length
>    (let* ((dt (or (window-display-table)
>                   buffer-display-table
>                   standard-display-table))
> -         (glyphs (and dt (display-table-slot dt 'selective-display))))
> -    (string-width (if glyphs (concat glyphs) "..."))))
> +         (glyphs (and dt (display-table-slot dt 'selective-display)))
> +         (vlen (length glyphs))
> +         (char-glyphs (make-vector vlen nil)))
> +    (dotimes (i vlen)
> +      (aset char-glyphs i (glyph-char (aref glyphs i))))
> +    (string-width (if glyphs (concat char-glyphs) "..."))))
>
>  (defun dired--get-filename-display-length ()
>    "Return maximum display length of filename.

Thanks Eli! I wasn't aware of the face of glyph and I confirm the
dired display is correct with your fix.

Message #88 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67161 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Sun, 26 Nov 2023 07:59:29 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Sun, 26 Nov 2023 10:56:52 +0800
> Cc: Juri Linkov <juri <at> linkov.net>, 67161 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> 于2023年11月26日周日 04:02写道：
> 
> > Does the below fix the problem, and is the Dired display after the
> > change correct when file names are truncated?
> >
> > diff --git a/lisp/dired.el b/lisp/dired.el
> > index 23a6fc0..096d6a8 100644
> > --- a/lisp/dired.el
> > +++ b/lisp/dired.el
> > @@ -2021,8 +2021,12 @@ dired--get-ellipsis-length
> >    (let* ((dt (or (window-display-table)
> >                   buffer-display-table
> >                   standard-display-table))
> > -         (glyphs (and dt (display-table-slot dt 'selective-display))))
> > -    (string-width (if glyphs (concat glyphs) "..."))))
> > +         (glyphs (and dt (display-table-slot dt 'selective-display)))
> > +         (vlen (length glyphs))
> > +         (char-glyphs (make-vector vlen nil)))
> > +    (dotimes (i vlen)
> > +      (aset char-glyphs i (glyph-char (aref glyphs i))))
> > +    (string-width (if glyphs (concat char-glyphs) "..."))))
> >
> >  (defun dired--get-filename-display-length ()
> >    "Return maximum display length of filename.
> 
> Thanks Eli! I wasn't aware of the face of glyph and I confirm the
> dired display is correct with your fix.

Thanks for testing.

However, I'm beginning to think that this is a subtle misfeature in
'concat': it should support vector of glyphs with faces, and should
preserve the face information (my patch simply discards faces, which
could perhaps affect string-width result).

So I think we should perhaps extend 'concat' to support vectors of
glyphs with faces.  Stefan, WDYT?

Message #91 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: juri <at> linkov.net
Cc: liuhui1610 <at> gmail.com, 67161 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Sun, 26 Nov 2023 12:49:22 +0200

> Cc: 67161 <at> debbugs.gnu.org, juri <at> linkov.net
> Date: Sun, 26 Nov 2023 07:59:29 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> > From: Liu Hui <liuhui1610 <at> gmail.com>
> > Date: Sun, 26 Nov 2023 10:56:52 +0800
> > Cc: Juri Linkov <juri <at> linkov.net>, 67161 <at> debbugs.gnu.org
> > 
> > Thanks Eli! I wasn't aware of the face of glyph and I confirm the
> > dired display is correct with your fix.
> 
> Thanks for testing.
> 
> However, I'm beginning to think that this is a subtle misfeature in
> 'concat': it should support vector of glyphs with faces, and should
> preserve the face information (my patch simply discards faces, which
> could perhaps affect string-width result).
> 
> So I think we should perhaps extend 'concat' to support vectors of
> glyphs with faces.  Stefan, WDYT?

For now, I installed my fix, so that at least the code will work with
Juri's customizations.  If and when 'concat' supports glyphs with
faces in display-tables, we could revert the change in dired.el

Message #94 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Liu Hui <liuhui1610 <at> gmail.com>, 67161 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Sun, 26 Nov 2023 09:03:39 -0500

> However, I'm beginning to think that this is a subtle misfeature in
> 'concat': it should support vector of glyphs with faces, and should
> preserve the face information (my patch simply discards faces, which
> could perhaps affect string-width result).

Hmm...  It's hard for me to judge because I don't really understand why
we have "vectors of glyphs".  If vectors of glyphs can express things we
can't express in a string, then the question is what should `concat` do
in that case, and if we can then ... why do we even have vectors of
glyphs?


        Stefan

Message #97 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: liuhui1610 <at> gmail.com, 67161 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Sun, 26 Nov 2023 16:53:42 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Liu Hui <liuhui1610 <at> gmail.com>,  juri <at> linkov.net,  67161 <at> debbugs.gnu.org
> Date: Sun, 26 Nov 2023 09:03:39 -0500
> 
> > However, I'm beginning to think that this is a subtle misfeature in
> > 'concat': it should support vector of glyphs with faces, and should
> > preserve the face information (my patch simply discards faces, which
> > could perhaps affect string-width result).
> 
> Hmm...  It's hard for me to judge because I don't really understand why
> we have "vectors of glyphs".

They are stored in display-tables.

> If vectors of glyphs can express things we can't express in a
> string, then the question is what should `concat` do in that case,
> and if we can then ... why do we even have vectors of glyphs?

See above.  I don't know why display-tables store vectors and not
strings, but it was like that forever.

Message #100 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: liuhui1610 <at> gmail.com, 67161 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Sun, 26 Nov 2023 12:08:04 -0500

>> > However, I'm beginning to think that this is a subtle misfeature in
>> > 'concat': it should support vector of glyphs with faces, and should
>> > preserve the face information (my patch simply discards faces, which
>> > could perhaps affect string-width result).
>> Hmm...  It's hard for me to judge because I don't really understand why
>> we have "vectors of glyphs".
> They are stored in display-tables.

Yes, I know do that much :-)

>> If vectors of glyphs can express things we can't express in a
>> string, then the question is what should `concat` do in that case,
>> and if we can then ... why do we even have vectors of glyphs?
> See above.  I don't know why display-tables store vectors and not
> strings, but it was like that forever.

If everything we can do with vectors of glyphs can be done with strings
(i.e. vectors of glyphs are basically accidents of history), then it
seems it would make sense to auto-convert a vector of glyph to a string
*and* to phase out the use of vectors of glyphs.


        Stefan

Message #103 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: liuhui1610 <at> gmail.com, 67161 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Sun, 26 Nov 2023 19:58:18 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: liuhui1610 <at> gmail.com,  juri <at> linkov.net,  67161 <at> debbugs.gnu.org
> Date: Sun, 26 Nov 2023 12:08:04 -0500
> 
> >> If vectors of glyphs can express things we can't express in a
> >> string, then the question is what should `concat` do in that case,
> >> and if we can then ... why do we even have vectors of glyphs?
> > See above.  I don't know why display-tables store vectors and not
> > strings, but it was like that forever.
> 
> If everything we can do with vectors of glyphs can be done with strings
> (i.e. vectors of glyphs are basically accidents of history), then it
> seems it would make sense to auto-convert a vector of glyph to a string
> *and* to phase out the use of vectors of glyphs.

Yes, but who will have the energy and motivation to go over all the
places that use the display-tables (both in Lisp and in C), and
convert all of them to use strings with faces instead vectors of
glyphs?  To say nothing of the related documentation?  And then we
will probably discover that some subtle aspect of this is the real
reason why we use vectors of glyphs...

Message #106 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: liuhui1610 <at> gmail.com, 67161 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Sun, 26 Nov 2023 13:06:00 -0500

> Yes, but who will have the energy and motivation to go over all the
> places that use the display-tables (both in Lisp and in C), and
> convert all of them to use strings with faces instead vectors of
> glyphs?  To say nothing of the related documentation?  And then we
> will probably discover that some subtle aspect of this is the real
> reason why we use vectors of glyphs...

Sounds exciting!


        Stefan

Message #109 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 67161 <at> debbugs.gnu.org
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Mon, 27 Nov 2023 09:19:36 +0200

>> Does the below fix the problem, and is the Dired display after the
>> change correct when file names are truncated?
>
> Thanks Eli! I wasn't aware of the face of glyph and I confirm the
> dired display is correct with your fix.

Now that it works I was able to find a different problem:

  Debugger entered--Lisp error: (void-function t)
    t(#<overlay from 960 to 1016 in tmp/>)
    isearch-open-necessary-overlays(#<overlay from 960 to 1016 in tmp/>)
    isearch-clean-overlays()
    isearch-exit()
    funcall-interactively(isearch-exit)
    command-execute(isearch-exit)

because 'isearch-open-invisible' should be a function.
I wonder what would be a suitable function.
After searching for other uses I found a good example
in lisp/org/org-fold-core.el:

  (overlay-put o 'isearch-open-invisible #'delete-overlay)

that works nicely leaving the overlay open after exiting isearch in dired.

Message #112 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 67161 <at> debbugs.gnu.org
Subject: Re: bug#67161: 30.0.50;
 [PATCH] Add option `dired-filename-display-length'
Date: Mon, 27 Nov 2023 16:32:45 +0800

Juri Linkov <juri <at> linkov.net> 于2023年11月27日周一 15:31写道：

> Now that it works I was able to find a different problem:
>
>   Debugger entered--Lisp error: (void-function t)
>     t(#<overlay from 960 to 1016 in tmp/>)
>     isearch-open-necessary-overlays(#<overlay from 960 to 1016 in tmp/>)
>     isearch-clean-overlays()
>     isearch-exit()
>     funcall-interactively(isearch-exit)
>     command-execute(isearch-exit)
>
> because 'isearch-open-invisible' should be a function.
> I wonder what would be a suitable function.
> After searching for other uses I found a good example
> in lisp/org/org-fold-core.el:
>
>   (overlay-put o 'isearch-open-invisible #'delete-overlay)
>
> that works nicely leaving the overlay open after exiting isearch in dired.

Sorry for the mistake! I can reproduce the problem and the fix works
well. Thank you, Juri!

Message #115 received at 67161 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 67161 <at> debbugs.gnu.org
Subject: Re: bug#67161: 30.0.50; [PATCH] Add option
 `dired-filename-display-length'
Date: Mon, 27 Nov 2023 19:16:10 +0200

>> Now that it works I was able to find a different problem:
>>
>>   Debugger entered--Lisp error: (void-function t)
>>     t(#<overlay from 960 to 1016 in tmp/>)
>>     isearch-open-necessary-overlays(#<overlay from 960 to 1016 in tmp/>)
>>     isearch-clean-overlays()
>>     isearch-exit()
>>     funcall-interactively(isearch-exit)
>>     command-execute(isearch-exit)
>>
>> because 'isearch-open-invisible' should be a function.
>> I wonder what would be a suitable function.
>> After searching for other uses I found a good example
>> in lisp/org/org-fold-core.el:
>>
>>   (overlay-put o 'isearch-open-invisible #'delete-overlay)
>>
>> that works nicely leaving the overlay open after exiting isearch in dired.
>
> Sorry for the mistake! I can reproduce the problem and the fix works
> well. Thank you, Juri!

Thanks for confirming.  Now pushed and everything works nicely.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.