GNU bug report logs - #79294
[PATCH] Add hideable indicators for hideshow.

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 79294 <at> debbugs.gnu.org
Subject: bug#79294: [PATCH] Add hideable indicators for hideshow.
Date: Sat, 13 Sep 2025 13:30:02 +0300

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: 79294 <at> debbugs.gnu.org
> Date: Thu, 11 Sep 2025 21:54:30 -0600
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Ping!  How can we make some progress in this matter?
> 
> Sorry for the delay, I have been very busy and have not had time to
> respond.
> 
> I tried using `pre-redisplay-functions`, but performance was worse.
> 
> I was able to mitigate the poor performance by removing the indicators
> for comment blocks (that implementation was broken and very slow) and
> adding a limit for the indicators.
> 
> I also tried using the new feature in larger files for a while to test
> its performance (since there were no volunteers), and from what I've
> tested, it is stable.
> 
> At the moment, the implementation can now be installed.

Thanks, I have a few minor comments, and then we can install this.

> +@item hs-show-indicators
> +This variable controls whether Hideshow mode should display indicators
> +of hidden and shown blocks.  The indicators also allow toggling the
> +hide/show state of each block. If the value is non-@code{nil}, enables
> +the indicators in all blocks except comment blocks.  The default is
> +@code{nil}, which disables the indicators.

You don't mention the value include-comments here.  It's okay not to
mention it for brevity, but t6hen we should not say "all blocks except
comment blocks", just that non-nil shows them and nil doesn't.

> +@item hs-indicator-maximum-buffer-size
> +This variable determines if the indicators should be enabled if the
> +current buffer size is not larger than this variable value.  If set to
> +@code{nil}, the indicators will be activated regardless of the buffer
> +size.  The default value is 2000000 bytes (2 megabytes).

I suggest to rephrase as follows:

  This variable limits the display of hideshow indicators to buffers
  that are not too large.  (Larger buffers might adversely affect
  redisplay performance.)  By default, buffers larger than 2MB have
  the indicators disabled; the value of nil will activate the
  indicators regardless of the buffer size.

> +*** New user option 'hs-indicator-maximum-buffer-size'.
> +This variable determines whether the indicators should be enabled if the
> +current buffer size is not larger than this variable value.  If set to
> +nil, the indicators will be activated regardless of the buffer size.
> +The default value is 2000000 bytes (2 megabytes).

Suggest the same rephrase here.

> +*** New user option 'hs-indicators-type'.
> +This user option determine which indicator type should be used for the
> +block indicators.
> +
> +The possible values can be: 'left-fringe', display the indicators in the
> +left fringe (the default); 'right-fringe', display the indicators in the
> +right fringe; 'left-margin', display the indicators in the left margin;
> +'right-margin', display the indicators in the right margin; nil, display
> +a string as indicator after the block beginning.

This needs to be updated, since the values are now different.

> +(defun hs-block-positions (&optional beg)
> +  "Return block positions.
> +If BEG is defined, start from BEG."
   ^^^^^^^^^^^^^^^^^
"If BEG is non-nil" is our style.

And the doc string should also explain:

  . what do you mean by "block positions" -- is it a list of positions?
  . what is the significance and the role of the starting position
  . where does the function start if BEG is nil or omitted

> +(defun hs--make-indicators-overlays (beg end)
> +  "Helper function for make the indicators overlays."
                      ^^^^^^^^
Either "to make" or "for making".

> +  (let* ((o (make-overlay
> +             ;; FIXME: The mouse-1 event doesn't work well for fringes,
> +             ;; a workaround for this is set the overlay at the
> +             ;; beginning of the line only for fringe indicators
> +             (if (eq hs-indicator-type 'fringe) (pos-bol) beg)
> +             (1+ beg)))

What doesn't work well in mouse-1 clicks on the fringes?

Previous Next

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