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

[Liu Hui]

Eli Zaretskii <eliz@gnu.org> 于2023年11月16日周四 20:11写道：
>
> > From: Liu Hui <liuhui1610@gmail.com>
> > Date: Thu, 16 Nov 2023 18:07:04 +0800
> > Cc: Stefan Kangas <stefankangas@gmail.com>, Stefan Monnier 
> > <monnier@iro.umontreal.ca>,
> >       67161@debbugs.gnu.org
> >
> > From 991ea588df6799331a7feea9e83493ef0d724096 Mon Sep 17 00:00:00 2001
> > From: Liu Hui <liuhui1610@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
Description: Text Data