GNU bug report logs - #78234
[PATCH] Prettify and improve hideshow. (bug#)

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 78234 in the body.
You can then email your comments to 78234 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: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Sat, 03 May 2025 21:39:52 -0600

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

Tags: patch

This patch allows to customize the ellipsis displayed when a block is
hidden, by default it displays "…" (if possible) and fallback to "...".

The ellipsis is now a button, If pressed it will unhide/unfold the block
that was already hidden.

Also it can optionally display the total lines folded/hidden instead
displaying the ellipsis.
Thus, Instead display this:
  (defun fun (args) ...)

It will display:

  (defun fun (args) 2 lines...)

[0001-Prettify-and-improve-hideshow.-bug.patch (text/patch, attachment)]

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

-- 
                                          - E.G via GNU Emacs and Org.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Sun, 04 May 2025 08:44:26 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Date: Sat, 03 May 2025 21:39:52 -0600
> 
> This patch allows to customize the ellipsis displayed when a block is
> hidden, by default it displays "…" (if possible) and fallback to "...".

Thanks.

I understand the default and its fallback, but why do we need to make
this customizable?

Also, shouldn't the test whether the ellipsis can be displayed be done
for a specific frame?  The same Emacs session could have several
frames, some text-only, other GUI, and only some of them could be
capable of displaying the ellipsis.  Testing this once, when the
package is loaded, might be suboptimal.

> +This option doesn't take effect if (@code{hs-indicate-lines-hidden})
> +is non-@code{nil}.

Double negation is usually not a good idea, as it makes the text
confusing and easy to misinterpret.

> +@item hs-indicate-lines-hidden
> +If non-@code{nil}, display the total lines hidden.
> +This overrides (@code{hs-ellipsis}).

It does?  Doesn't this show the ellipsis after the number of lines?

> +** Hideshow mode changes
> +
> +*** New user option 'hs-ellipsis'.
> +This option customize the ellipsis displayed in hidden blocks, this
               ^^^^^^^^^
"customizes"

> +doesn't take effect if `hs-indicate-lines-hidden' is non-nil.
> +
> +*** New user option 'hs-indicate-lines-hidden'.
> +This option, if non-nil, indicate total lines hidden and this is used
                            ^^^^^^^^
"indicates"

> +instead 'hs-ellipsis'.  By default this is set to nil.
   ^^^^^^^
"instead of"

> +(defcustom hs-ellipsis
> +  (if (char-displayable-p ?…)
> +      "…"
> +    "...")
> +  "String to be used as ellipsis in hidden region.
> +This option doesn't take effect if `hs-indicate-lines-hidden' is non-nil."
> +  :type 'string)

Each new defcustom should have a :version tag.

> +(defcustom hs-indicate-lines-hidden nil
> +  "If non-nil, display the total lines hidden.

This is too terse.  Display where?  And it doesn't even mention
hideshow, so could be interpreted as something very general.

> +    (overlay-put ov 'display
> +                 (propertize (hs-insert-ellipsis b e)
> +                             'face 'custom-button
> +                             'mouse-face 'custom-button-mouse
> +                             'help-echo "mouse-1: unhide this"

This is again too terse.  I suggest "mouse-1: show hidden lines"
instead.

> +(defun hs-insert-ellipsis (b e)
> +  "Insert ellipsis on hidden region.
> +If `hs-detailed-ellipsis' is non-nil, indicate the number of characters
> +folded from B to E.

This is inconsistent with the code and the variable names

> +Otherwise fallback to `hs-ellipsis' value."
             ^^^^^^^^
"fall back"

Also, a function that inserts something cannot "fall back to some
value", so rewording is in order here.

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

From: Juri Linkov <juri <at> linkov.net>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Sun, 04 May 2025 09:58:35 +0300

> This patch allows to customize the ellipsis displayed when a block is
> hidden, by default it displays "…" (if possible) and fallback to "...".
>
> The ellipsis is now a button, If pressed it will unhide/unfold the block
> that was already hidden.
>
> Also it can optionally display the total lines folded/hidden instead
> displaying the ellipsis.
> Thus, Instead display this:
>   (defun fun (args) ...)
>
> It will display:
>
>   (defun fun (args) 2 lines...)

Thanks, it's nice to add more features to hideshow.  I tested your patch,
and the only visual problem is that the button looks too heavy.
Would it be possible to make its appearance more subtle?

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow. (bug#)
Date: Sun, 4 May 2025 12:29:55 -0600

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

Juri Linkov <juri <at> linkov.net> escribió:

> > This patch allows to customize the ellipsis displayed when a block is
> > hidden, by default it displays "…" (if possible) and fallback to "...".
> >
> > The ellipsis is now a button, If pressed it will unhide/unfold the block
> > that was already hidden.
> >
> > Also it can optionally display the total lines folded/hidden instead
> > displaying the ellipsis.
> > Thus, Instead display this:
> >   (defun fun (args) ...)
> >
> > It will display:
> >
> >   (defun fun (args) 2 lines...)
>
> Thanks, it's nice to add more features to hideshow.  I tested your patch,
> and the only visual problem is that the button looks too heavy.
> Would it be possible to make its appearance more subtle?
>

It looks normal for me (maybe for the theme that I'm using), but what would
you suggest?,
creating new face for this? Or using `shadow` face instead?

>

[Message part 2 (text/html, inline)]

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

From: Juri Linkov <juri <at> linkov.net>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow. (bug#)
Date: Sun, 04 May 2025 21:47:00 +0300

>> and the only visual problem is that the button looks too heavy.
>> Would it be possible to make its appearance more subtle?
>
> It looks normal for me (maybe for the theme that I'm using), but what would
> you suggest?,

I suggest to use the same face attributes as in this face:

  (defface flymake-end-of-line-diagnostics-face
    '((t :height 0.85 :box (:line-width -1)))
    "Face used for end-of-line diagnostics."
    :package-version '(Flymake . "1.3.5"))

It looks like a button, but still quite moderate, and looks like in other IDEs.

> creating new face for this? Or using `shadow` face instead?

A new customizable face is definitely needed since appearance
is a sensitive matter for users.

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Sun, 04 May 2025 15:30:12 -0600

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
>> Date: Sat, 03 May 2025 21:39:52 -0600
>> 
>> This patch allows to customize the ellipsis displayed when a block is
>> hidden, by default it displays "…" (if possible) and fallback to "...".
>
> Thanks.
>
> I understand the default and its fallback, but why do we need to make
> this customizable?

I've seen some comments about changing the ellipsis displayed,
of course this can be customizable using `display-table-slot', but it's
conflictive, some minor modes can overwrite the display table (such as
whitespace-mode).

Also I don't see any issue making hideshow ellipsis customizable,
org-mode does the same.

> Also, shouldn't the test whether the ellipsis can be displayed be done
> for a specific frame?  The same Emacs session could have several
> frames, some text-only, other GUI, and only some of them could be
> capable of displaying the ellipsis.  Testing this once, when the
> package is loaded, might be suboptimal.

I've updated it to use by default "...".

>> +This option doesn't take effect if (@code{hs-indicate-lines-hidden})
>> +is non-@code{nil}.
>
> Double negation is usually not a good idea, as it makes the text
> confusing and easy to misinterpret.

Fixed.

>> +(defcustom hs-ellipsis
>> +  (if (char-displayable-p ?…)
>> +      "…"
>> +    "...")
>> +  "String to be used as ellipsis in hidden region.
>> +This option doesn't take effect if `hs-indicate-lines-hidden' is non-nil."
>> +  :type 'string)
>
> Each new defcustom should have a :version tag.

I forgot, I was in a hurry to submit this patch.

>> +(defun hs-insert-ellipsis (b e)
>> +  "Insert ellipsis on hidden region.
>> +If `hs-detailed-ellipsis' is non-nil, indicate the number of characters
>> +folded from B to E.
>
> This is inconsistent with the code and the variable names

I've simplified the docstring.

[0001-Prettify-and-improve-hideshow.-bug-78234.patch (text/x-patch, attachment)]

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

-- 
                                          - E.G via GNU Emacs and Org.

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

From: Juri Linkov <juri <at> linkov.net>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Mon, 05 May 2025 09:33:41 +0300

> Also I don't see any issue making hideshow ellipsis customizable,
> org-mode does the same.

I wonder how this is related to this existing comment in hideshow.el:

  ;; See also variable `hs-set-up-overlay' for per-block customization of
  ;; appearance or other effects associated with overlays.  For example:
  ;;
  ;; (setq hs-set-up-overlay
  ;;       (defun my-display-code-line-counts (ov)
  ;;         (when (eq 'code (overlay-get ov 'hs))
  ;;           (overlay-put ov 'display
  ;;                        (propertize
  ;;                         (format " ... <%d>"
  ;;                                 (count-lines (overlay-start ov)
  ;;                                              (overlay-end ov)))
  ;;                         'face 'font-lock-type-face)))))

And in the docstring of the option 'hs-set-up-overlay':

    "Function called with one arg, OV, a newly initialized overlay.
  Hideshow puts a unique overlay on each range of text to be hidden
  in the buffer.  Here is a simple example of how to use this variable:

    (defun display-code-line-counts (ov)
      (when (eq \\='code (overlay-get ov \\='hs))
        (overlay-put ov \\='display
                     (format \"... / %d\"
                             (count-lines (overlay-start ov)
                                          (overlay-end ov))))))

    (setq hs-set-up-overlay #\\='display-code-line-counts)

  This example shows how to get information from the overlay as well
  as how to set its `display' property.  See `hs-make-overlay' and
  info node `(elisp)Overlays'."

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Mon, 05 May 2025 11:54:37 -0600

Juri Linkov <juri <at> linkov.net> writes:

>> Also I don't see any issue making hideshow ellipsis customizable,
>> org-mode does the same.
>
> I wonder how this is related to this existing comment in hideshow.el:
>
>   ;; See also variable `hs-set-up-overlay' for per-block customization of
>   ;; appearance or other effects associated with overlays.  For example:
>   ;;
>   ;; (setq hs-set-up-overlay
>   ;;       (defun my-display-code-line-counts (ov)
>   ;;         (when (eq 'code (overlay-get ov 'hs))
>   ;;           (overlay-put ov 'display
>   ;;                        (propertize
>   ;;                         (format " ... <%d>"
>   ;;                                 (count-lines (overlay-start ov)
>   ;;                                              (overlay-end ov)))
>   ;;                         'face 'font-lock-type-face)))))
>
> And in the docstring of the option 'hs-set-up-overlay':
>
>     "Function called with one arg, OV, a newly initialized overlay.
>   Hideshow puts a unique overlay on each range of text to be hidden
>   in the buffer.  Here is a simple example of how to use this variable:
>
>     (defun display-code-line-counts (ov)
>       (when (eq \\='code (overlay-get ov \\='hs))
>         (overlay-put ov \\='display
>                      (format \"... / %d\"
>                              (count-lines (overlay-start ov)
>                                           (overlay-end ov))))))
>
>     (setq hs-set-up-overlay #\\='display-code-line-counts)
>
>   This example shows how to get information from the overlay as well
>   as how to set its `display' property.  See `hs-make-overlay' and
>   info node `(elisp)Overlays'."

But those are not intuitive, it's almost hidden from the user (unless
they read the source code).  Would not it be better have it as an option
instead of telling the user to create a function for it (like what this
patch does)?

-- 
                                          - E.G via GNU Emacs and Org.

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

From: Juri Linkov <juri <at> linkov.net>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Mon, 05 May 2025 22:00:09 +0300

>>     (defun display-code-line-counts (ov)
>>       (when (eq \\='code (overlay-get ov \\='hs))
>>         (overlay-put ov \\='display
>>                      (format \"... / %d\"
>>                              (count-lines (overlay-start ov)
>>                                           (overlay-end ov))))))
>>
>>     (setq hs-set-up-overlay #\\='display-code-line-counts)
>
> But those are not intuitive, it's almost hidden from the user (unless
> they read the source code).  Would not it be better have it as an option
> instead of telling the user to create a function for it (like what this
> patch does)?

Indeed, an option is much better.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Thu, 08 May 2025 13:45:18 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: 78234 <at> debbugs.gnu.org
> Date: Sun, 04 May 2025 15:30:12 -0600
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> >> Date: Sat, 03 May 2025 21:39:52 -0600
> >> 
> >> This patch allows to customize the ellipsis displayed when a block is
> >> hidden, by default it displays "…" (if possible) and fallback to "...".
> >
> > Thanks.
> >
> > I understand the default and its fallback, but why do we need to make
> > this customizable?
> 
> I've seen some comments about changing the ellipsis displayed,
> of course this can be customizable using `display-table-slot', but it's
> conflictive, some minor modes can overwrite the display table (such as
> whitespace-mode).
> 
> Also I don't see any issue making hideshow ellipsis customizable,
> org-mode does the same.

Assuming we get the value right automatically for each frame type, and
avoid showing a character that cannot be displayed, what would be the
reason for people to customize this?

> > Also, shouldn't the test whether the ellipsis can be displayed be done
> > for a specific frame?  The same Emacs session could have several
> > frames, some text-only, other GUI, and only some of them could be
> > capable of displaying the ellipsis.  Testing this once, when the
> > package is loaded, might be suboptimal.
> 
> I've updated it to use by default "...".

That's not what I meant, sorry for being unclear.  What I meant was to
compute the value for each frame based on whether the frame can or
cannot display the ellipsis character '…', by using
char-displayable-on-frame-p.

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -689,6 +689,15 @@ Now 'treesit-explore-mode' (or 'treesit-explore') prompts for a parser
>  rather than a language, and it is now possible to select a local parser
>  at point to explore.
>  
> +** Hideshow mode changes

Just "Hideshow" is enough.

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Thu, 08 May 2025 15:42:34 -0600

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
>> Cc: 78234 <at> debbugs.gnu.org
>> Date: Sun, 04 May 2025 15:30:12 -0600
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
>> >> Date: Sat, 03 May 2025 21:39:52 -0600
>> >> 
>> >> This patch allows to customize the ellipsis displayed when a block is
>> >> hidden, by default it displays "…" (if possible) and fallback to "...".
>> >
>> > Thanks.
>> >
>> > I understand the default and its fallback, but why do we need to make
>> > this customizable?
>> 
>> I've seen some comments about changing the ellipsis displayed,
>> of course this can be customizable using `display-table-slot', but it's
>> conflictive, some minor modes can overwrite the display table (such as
>> whitespace-mode).
>> 
>> Also I don't see any issue making hideshow ellipsis customizable,
>> org-mode does the same.
>
> Assuming we get the value right automatically for each frame type, and
> avoid showing a character that cannot be displayed, what would be the
> reason for people to customize this?

I'm not sure, most Emacs users tend to customize everything.

I made it customizable because it was not possible (for me) get the
ellipsis string that can be displayed in outline-mode and org-mode.

And because the "..." takes up space as opposed to just using the
ellipsis char.

But maybe it would be better to hardcore this using "…" if displayable
and use "..." otherwise.

>> > Also, shouldn't the test whether the ellipsis can be displayed be done
>> > for a specific frame?  The same Emacs session could have several
>> > frames, some text-only, other GUI, and only some of them could be
>> > capable of displaying the ellipsis.  Testing this once, when the
>> > package is loaded, might be suboptimal.
>> 
>> I've updated it to use by default "...".
>
> That's not what I meant, sorry for being unclear.  What I meant was to
> compute the value for each frame based on whether the frame can or
> cannot display the ellipsis character '…', by using
> char-displayable-on-frame-p.

Fine, it should work well now.

[0001-Prettify-and-improve-hideshow.-bug-78234.patch (text/x-patch, attachment)]

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

-- 
                                          - E.G via GNU Emacs and Org.

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Thu, 08 May 2025 15:54:24 -0600

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

Elijah Gabe Pérez <eg642616 <at> gmail.com> writes:

> I made it customizable because it was not possible (for me) get the
> ellipsis string that can be displayed in outline-mode and org-mode.

I found the solution, now it should use `selective-display' string from
display table (if set), otherwise use "…" or "...".

[0001-Prettify-and-improve-hideshow.-bug-78234.patch (text/x-patch, attachment)]

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

-- 
                                          - E.G via GNU Emacs and Org.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Sat, 10 May 2025 14:13:23 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: 78234 <at> debbugs.gnu.org
> Date: Thu, 08 May 2025 15:54:24 -0600
> 
> +@item hs-indicate-lines-hidden
> +If non-@code{nil}, display the total lines hidden in the hideshow
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"display the number of hidden lines"

> +** Hideshow
> +
> +*** New user option 'hs-indicate-lines-hidden'.
> +This option, if non-nil, indicates total lines hidden in the ellipsis.

"Indicates" is not accurate, because the feature actually shows the
number.  So "show" or "display".  And see a similar comment regarding
the wording above.

> +(defface hs-ellipsis
> +  '((t :height 0.80 :box (:line-width -1)))
> +  "Face used for hideshow ellipsis."
> +  :version "31.1")

Hmm, what happens if the string you get from the display-table slot
already has a face?  In general, if we take the ellipsis from the
display table, shouldn't we take its face as well?  If so, I think you
want to move the face and help-echo stuff into the function
hs-insert-ellipsis.

> +(defcustom hs-indicate-lines-hidden nil
> +  "If non-nil, display the total lines hidden in the hideshow ellipsis."

See the comments about wording above.

Thanks.

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Sat, 10 May 2025 18:11:27 -0600

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

Eli Zaretskii <eliz <at> gnu.org> writes:

>> +(defface hs-ellipsis
>> +  '((t :height 0.80 :box (:line-width -1)))
>> +  "Face used for hideshow ellipsis."
>> +  :version "31.1")
>
> Hmm, what happens if the string you get from the display-table slot
> already has a face?  In general, if we take the ellipsis from the
> display table, shouldn't we take its face as well?

Now it should use display-table slot face (if there is).

> If so, I think you want to move the face and help-echo stuff into the
> function hs-insert-ellipsis.

Why help-echo too? It is very rare that someone would use help-echo for
the ellipsis (and I haven't seen anything like this).
I think just the face would be more than enough.

[0001-Prettify-and-improve-hideshow.-bug-78234.patch (text/x-patch, attachment)]

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

-- 
                                          - E.G via GNU Emacs and Org.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Sun, 11 May 2025 08:49:39 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: 78234 <at> debbugs.gnu.org
> Date: Sat, 10 May 2025 18:11:27 -0600
> 
> >> +(defface hs-ellipsis
> >> +  '((t :height 0.80 :box (:line-width -1)))
> >> +  "Face used for hideshow ellipsis."
> >> +  :version "31.1")
> >
> > Hmm, what happens if the string you get from the display-table slot
> > already has a face?  In general, if we take the ellipsis from the
> > display table, shouldn't we take its face as well?
> 
> Now it should use display-table slot face (if there is).

This is trickier than it sounds, see below.

> +** Hideshow
> +
> +*** New user option 'hs-display-lines-hidden'.
> +This option, if non-nil, display total lines hidden in the ellipsis.

 "If this option is non-nil, Hideshow displays the number of hidden
 lines next to the ellipsis."

> +(defun hs--get-ellipsis (b e)
> +  "Helper function for `hs-make-overlay'.
> +This return a list containing the ellipsis string to use and its
> +face."
> +  (cond*
> +   ((bind*
> +     (d-t-ellipsis (display-table-slot standard-display-table 'selective-display))
> +     (d-t-property (if d-t-ellipsis (get-text-property 0 'face d-t-ellipsis)))
> +     (ellipsis
> +      (cond
> +       ((and d-t-ellipsis (not (length= d-t-ellipsis 0)))
> +        (concat d-t-ellipsis)) ; convert ellipsis vector to string
> +       ((char-displayable-on-frame-p ?…)
> +        "…")
> +       (t "...")))))
> +   (hs-display-lines-hidden
> +    (let ((lines (1- (count-lines b e))))
> +      (list
> +       (concat (number-to-string lines)
> +               (if (= lines 1) " line" " lines")
> +               ellipsis)
> +       'face (or d-t-property 'hs-ellipsis))))
> +   (t (list ellipsis 'face (or d-t-property 'hs-ellipsis)))))

I think this is based on the incorrect assumption that the value of
'selective-display' slot is necessarily a string, with or without a
face.  But that's not true.  The ELisp manual says:

     The ordinary elements of the display table are indexed by character
  codes; the element at index C says how to display the character code C.
  The value should be ‘nil’ (which means to display the character C
  according to the usual display conventions; *note Usual Display::), or a
  vector of glyph codes (which means to display the character C as those
  glyphs; *note Glyphs::).
  [...]
     The display table also has six “extra slots” which serve special
  purposes.  Here is a table of their meanings; ‘nil’ in any slot means to
  use the default for that slot, as stated below.
  [...]
  4
       A vector of glyphs for indicating the presence of invisible lines
       (the default is ‘...’).  *Note Selective Display::.

A glyph is a combination of a character and a face, made by
make-glyph.  You cannot use get-text-property and concat on such
vectors, because they will signal an error.  Instead, you need to
obtain the glyph faces by using glyph-face; see the node "Glyphs" in
the ELisp manual.  The node "Display Tables" shows how to construct
elements of a display-table; here's an example that I suggest to use
in your testing of this feature:

  (setq standard-display-table (make-display-table))
  (set-display-table-slot standard-display-table
			  'selective-display
			  (vector (make-glyph-code #x2026 'warning)))

This sets up the 'selective-display' slot to a glyph whose character
is U+2026 HORIZONTAL ELLIPSIS and whose face is 'warning'.

In the most general case that the 'selective-display' slot is a vector
of more than one glyph, converting such a vector to a string needs to
extract the characters one by one using glyph-char and the
corresponding faces using glyph-face, and then apply to each character
in the string the corresponding face.

Let me know if you need more help with dealing with glyphs.

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78234 <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Sun, 11 May 2025 11:31:50 -0600

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

Eli Zaretskii <eliz <at> gnu.org> writes:
>> +(defun hs--get-ellipsis (b e)
>> +  "Helper function for `hs-make-overlay'.
>> +This return a list containing the ellipsis string to use and its
>> +face."
>> +  (cond*
>> +   ((bind*
>> +     (d-t-ellipsis (display-table-slot standard-display-table 'selective-display))
>> +     (d-t-property (if d-t-ellipsis (get-text-property 0 'face d-t-ellipsis)))
>> +     (ellipsis
>> +      (cond
>> +       ((and d-t-ellipsis (not (length= d-t-ellipsis 0)))
>> +        (concat d-t-ellipsis)) ; convert ellipsis vector to string
>> +       ((char-displayable-on-frame-p ?…)
>> +        "…")
>> +       (t "...")))))
>> +   (hs-display-lines-hidden
>> +    (let ((lines (1- (count-lines b e))))
>> +      (list
>> +       (concat (number-to-string lines)
>> +               (if (= lines 1) " line" " lines")
>> +               ellipsis)
>> +       'face (or d-t-property 'hs-ellipsis))))
>> +   (t (list ellipsis 'face (or d-t-property 'hs-ellipsis)))))
>
> I think this is based on the incorrect assumption that the value of
> 'selective-display' slot is necessarily a string, with or without a
> face.  But that's not true.  The ELisp manual says:
>
>      The ordinary elements of the display table are indexed by character
>   codes; the element at index C says how to display the character code C.
>   The value should be ‘nil’ (which means to display the character C
[...]
> In the most general case that the 'selective-display' slot is a vector
> of more than one glyph, converting such a vector to a string needs to
> extract the characters one by one using glyph-char and the
> corresponding faces using glyph-face, and then apply to each character
> in the string the corresponding face.

Thanks for the clarification, i thought that strings were also
supported due my tests, I found that was a bug in my code so this should
now be fixed.

Now it convert the vector to a string (with its faces).

[0001-Prettify-and-improve-hideshow.-bug-78234.patch (text/x-patch, attachment)]

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

-- 
                                          - E.G via GNU Emacs and Org.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 78234-done <at> debbugs.gnu.org
Subject: Re: bug#78234: [PATCH] Prettify and improve hideshow.  (bug#)
Date: Sat, 17 May 2025 12:12:47 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: 78234 <at> debbugs.gnu.org
> Date: Sun, 11 May 2025 11:31:50 -0600
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> >> +(defun hs--get-ellipsis (b e)
> >> +  "Helper function for `hs-make-overlay'.
> >> +This return a list containing the ellipsis string to use and its
> >> +face."
> >> +  (cond*
> >> +   ((bind*
> >> +     (d-t-ellipsis (display-table-slot standard-display-table 'selective-display))
> >> +     (d-t-property (if d-t-ellipsis (get-text-property 0 'face d-t-ellipsis)))
> >> +     (ellipsis
> >> +      (cond
> >> +       ((and d-t-ellipsis (not (length= d-t-ellipsis 0)))
> >> +        (concat d-t-ellipsis)) ; convert ellipsis vector to string
> >> +       ((char-displayable-on-frame-p ?…)
> >> +        "…")
> >> +       (t "...")))))
> >> +   (hs-display-lines-hidden
> >> +    (let ((lines (1- (count-lines b e))))
> >> +      (list
> >> +       (concat (number-to-string lines)
> >> +               (if (= lines 1) " line" " lines")
> >> +               ellipsis)
> >> +       'face (or d-t-property 'hs-ellipsis))))
> >> +   (t (list ellipsis 'face (or d-t-property 'hs-ellipsis)))))
> >
> > I think this is based on the incorrect assumption that the value of
> > 'selective-display' slot is necessarily a string, with or without a
> > face.  But that's not true.  The ELisp manual says:
> >
> >      The ordinary elements of the display table are indexed by character
> >   codes; the element at index C says how to display the character code C.
> >   The value should be ‘nil’ (which means to display the character C
> [...]
> > In the most general case that the 'selective-display' slot is a vector
> > of more than one glyph, converting such a vector to a string needs to
> > extract the characters one by one using glyph-char and the
> > corresponding faces using glyph-face, and then apply to each character
> > in the string the corresponding face.
> 
> Thanks for the clarification, i thought that strings were also
> supported due my tests, I found that was a bug in my code so this should
> now be fixed.
> 
> Now it convert the vector to a string (with its faces).

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

Previous Next

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