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

[Eli Zaretskii]

> 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.

> 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.