GNU bug report logs - #80116
[PATCH] hideshow: Reword documentation.

Previous Next

To reply to this bug, email your comments to 80116 AT debbugs.gnu.org.
There is no need to reopen the bug first.

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] hideshow: Reword documentation.
Date: Fri, 02 Jan 2026 20:28:16 -0600

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

Tags: patch

This patch is only a (re)wording change, mainly to improve the docstring
of some variables and the commentary header, also i fixed an entry in
the NEWS file.

[0001-hideshow-Reword-documentation.patch (text/x-patch, attachment)]

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

-- 
- E.G via Gnus and Org.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elijah Gabe Pérez <eg642616 <at> gmail.com>
Cc: 80116 <at> debbugs.gnu.org
Subject: Re: bug#80116: [PATCH] hideshow: Reword documentation.
Date: Sat, 03 Jan 2026 10:34:30 +0200

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Date: Fri, 02 Jan 2026 20:28:16 -0600
> 
> This patch is only a (re)wording change, mainly to improve the docstring
> of some variables and the commentary header, also i fixed an entry in
> the NEWS file.

Thanks, please see some comments below.

> +;; *** Non-regexp matching
> +;;
> +;; Hideshow uses regular expressions to match blocks by default,
> +;; however, in most cases this is not useful.  To detect blocks

This begs the question: if using regexps is not useful "in most
cases", then why do we do it in the first place?  Is "in most cases"
really what you wanted to say here?  Perhaps you meant "sometimes" or
"in many cases" instead?

> +This is used by `hs-block-positions' to move the point to the beginning

Our convention is not to use "the" when we talk about point.  Thus, we
say "to move point", not "to move the point".

> +As a special case, it can be nil (to use the position done by
> +`hs-forward-sexp-function'), or a function without arguments to check
> +this condition and return non-nil and set `match-data' to that block end
> +positions.")

The description of the function here is confusing.  Suggest to break
it into two or more sentences.  Something like

  As a special case, it can be nil (to use the position from
  `hs-forward-sexp-function'), or a function without arguments.  If
  it's a function, it should return non-nil if point is at end of a
  block, and set `match-data' to the position of the block.

>  (defvar-local hs-forward-sexp-function #'forward-sexp
> -  "Function used to do a `forward-sexp'.
> -It is called with 1 argument (like `forward-sexp').
> +  "Function used to reposition point to the end of the region to hide.
> +For backward compatibility, the function must take one argument, which
> +can be ignored.

I would replace "must take" with "is called with".

> +The function is called in front of the beginning of the block (usually the
> +current value of `hs-block-start-regexp' in the buffer) and should
> +reposition the point to the end of the block.

"the point" again.

> +Despite its name, the function does not necessarily have to act like
> +`forward-sexp'.")

What does this mean, exactly?  Which aspects of forward-sexp prompted
you to write this sentence?

>  (defvar-local hs-adjust-block-beginning-function nil
>    "Function used to tweak the block beginning.
> -It should return the position from where we should start hiding, as
> -opposed to hiding it from the position returned when searching for
> -`hs-block-start-regexp'.
> +It is called at the beginning of the block (usually the current value of
> +`hs-block-start-regexp' in the buffer) and return the position from
                                          ^^^^^^^^^^^^^^^^^^^^^^^
"...and should return the position..."

> +where we should start hiding.

Please don't use "we" in doc strings, because it's unclear who or what
is meant by that.

>  (defvar-local hs-adjust-block-end-function nil
>    "Function used to tweak the block end.
> +
> +It is called at the end of the block and must take one argument, which
> +is the start position where the overlay will be created, and should
> +return either the last position to hide or nil.  If it returns nil,
> +hideshow will guess the end position.

Again, it is best to break long and complex sentences into several
shorter ones.  Like this, for example:

  It is called at the end of the block with one argument, the start
  position where the overlay will be created.  It should return either
  the last position to hide or nil.  If it returns nil, Hideshow will
  guess the end position.

And I have a question: what is this "overlay" this doc string
mentions?  It is "out of the blue" here.  Perhaps instead of talking
about the overlay (which is an implementation detail), you should talk
about its effect(s), like hiding or something?

> +  "Function used to reposition point at the beginning of current block.
> +It should reposition point at the beginning of the current block and
> +return non-nil.  Otherwise, it should return nil if no block was found.")

This is backwards.  Suggest to reword:

  Function used to reposition point at the beginning of current block.
  If it finds the block beginning, it should reposition point there
  and return non-nil, otherwise it should return nil.

>  (defvar-local hs-find-next-block-function
>    #'hs-find-next-block-fn--default
> -  "Function used to do `hs-find-next-block'.
> +  "Function used to find a block forward.

I would suggest "Function to find the start of the next block."
instead.

> +It is called with three arguments REGEXP, BOUND, and COMMENTS.
> +REGEXP is a regexp representing block start.  When block start is found,
> +`match-data' should be set using REGEXP.
                              ^^^^^^^^^^^^
What do you mean by "using REGEXP" here?

> +BOUND is a buffer position that limits the search.
> +When COMMENTS is non-nil, REGEXP matches not only beginning of a block
> +but also beginning of a comment.  In this case, the function should find
> +a nearest block or comment and return non-nil.")
   ^^^^^^^^^^^^^^^^^^^^^^^^^^
"the nearest block or comment"

>  (defvar-local hs-looking-at-block-start-predicate
>    #'hs-looking-at-block-start-p--default
> -  "Function used to do `hs-looking-at-block-start-p'.
> +  "Function used to check if point is at the block start.
>  It should return non-nil if the point is at the block start and set
> -match data with the beginning and end of that position.
> -
> -Specifying this function is necessary for languages such as
> -Python, where `looking-at' and `syntax-ppss' check is not enough
> -to check if the point is at the block start.")
> +match data with the beginning and end of that position.")

"set match data with the beginning and end" is not detailed enough.
I'm guessing that you mean (match-beginning 0) and (match-end 0)
should return, respectively, the beginning and the end, but of what,
exactly?  A position has no meaningful beginning and end, it only has
a single value, the position itself.  You are probably talking about a
match of some regular expression, but there's no regexp involved in
this doc string, so it's unclear how to obtain those beginning and end
positions.  Please describe this in enough detail for users to write a
suitable function that abides by this protocol.

> +This is for only get code block positions, for comments use
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"This is for code block positions only"

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

From: "Elijah G." <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 80116 <at> debbugs.gnu.org
Subject: Re: bug#80116: [PATCH] hideshow: Reword documentation.
Date: Sat, 03 Jan 2026 13:03:18 -0600

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

>> +;; *** Non-regexp matching
>> +;;
>> +;; Hideshow uses regular expressions to match blocks by default,
>> +;; however, in most cases this is not useful.  To detect blocks
>
> This begs the question: if using regexps is not useful "in most
> cases", then why do we do it in the first place?  Is "in most cases"
> really what you wanted to say here?  Perhaps you meant "sometimes" or
> "in many cases" instead?

Actually, this meant: "for something more advanced than regexp".  Using
regexp depends on what one want to do, such as the tree-sitter support
which doesn't need regexp, or a (hypothetical) folding support via eglot
which wouldn't require regexp.

>> +This is used by `hs-block-positions' to move the point to the
>> beginning
>
> Our convention is not to use "the" when we talk about point.  Thus, we
> say "to move point", not "to move the point".
>

[...]

Thanks, i will fix it.

>> +Despite its name, the function does not necessarily have to act
>> like
>> +`forward-sexp'.")
>
> What does this mean, exactly?  Which aspects of forward-sexp prompted
> you to write this sentence?

This is because the name of this variable is somewhat confusing, and
also because the function must take one argument since this argument
was intended for commands similar to forward-sexp/list.  But i
agree this part is confusing, so I will remove it.


[...]

>>  (defvar-local hs-adjust-block-end-function nil
>>    "Function used to tweak the block end.
>> +
>> +It is called at the end of the block and must take one argument, which
>> +is the start position where the overlay will be created, and should
>> +return either the last position to hide or nil.  If it returns nil,
>> +hideshow will guess the end position.
>
> Again, it is best to break long and complex sentences into several
> shorter ones.  Like this, for example:
>
>   It is called at the end of the block with one argument, the start
>   position where the overlay will be created.  It should return either
>   the last position to hide or nil.  If it returns nil, Hideshow will
>   guess the end position.
>
> And I have a question: what is this "overlay" this doc string
> mentions?  It is "out of the blue" here.  Perhaps instead of talking
> about the overlay (which is an implementation detail), you should talk
> about its effect(s), like hiding or something?

The overlay here meant "the region in the buffer that will be hidden".
I will remove the "overlay" part here for something more clearer.

[...]

>> +It is called with three arguments REGEXP, BOUND, and COMMENTS.
>> +REGEXP is a regexp representing block start.  When block start is
>> found,
>> +`match-data' should be set using REGEXP.
>                               ^^^^^^^^^^^^
> What do you mean by "using REGEXP" here?

That "should set the match data according to the beginning position of
the matched REGEXP or block start position".

[...]

>>  (defvar-local hs-looking-at-block-start-predicate
>>    #'hs-looking-at-block-start-p--default
>> -  "Function used to do `hs-looking-at-block-start-p'.
>> +  "Function used to check if point is at the block start.
>>  It should return non-nil if the point is at the block start and set
>> -match data with the beginning and end of that position.
>> -
>> -Specifying this function is necessary for languages such as
>> -Python, where `looking-at' and `syntax-ppss' check is not enough
>> -to check if the point is at the block start.")
>> +match data with the beginning and end of that position.")
>
> "set match data with the beginning and end" is not detailed enough.
> I'm guessing that you mean (match-beginning 0) and (match-end 0)
> should return, respectively, the beginning and the end, but of what,
> exactly?

The block beginning matched.

> A position has no meaningful beginning and end, it only has a single
> value, the position itself.  You are probably talking about a match of
> some regular expression, but there's no regexp involved in this doc
> string, so it's unclear how to obtain those beginning and end
> positions.  Please describe this in enough detail for users to write a
> suitable function that abides by this protocol.

Actually, this is already confusing, so I'm going to have to modify the
behavior of this variable, so users will not have to modify match-data.

-- 
- E.G via GNU Emacs Android port.

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 80116 <at> debbugs.gnu.org
Subject: Re: bug#80116: [PATCH] hideshow: Reword documentation.
Date: Sun, 04 Jan 2026 14:04:26 -0600

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

Sending fixed patch:

[0001-hideshow-Reword-documentation.-Bug-80116.patch (text/x-patch, attachment)]

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

-- 
- E.G via Gnus and Org.

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

From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 80116 <at> debbugs.gnu.org
Subject: Re: bug#80116: [PATCH] hideshow: Reword documentation.
Date: Sun, 04 Jan 2026 17:15:07 -0600

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

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

> Sending fixed patch:

I realized that this breaks one test, so I had to revert one change from
the previous patch about the match data.

Since hideshow can use regexp and non-regexp matching, it's not easy
know which of the two is being used, that's why the docstrings for some
variables indicate it is needed to modify the match data, to make this
"difference" clearer and less prone to bugs.

[0001-hideshow-Reword-documentation.-Bug-80116.patch (text/x-patch, attachment)]

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

-- 
- E.G via Gnus and Org.

Message #22 received at 80116-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: 80116-done <at> debbugs.gnu.org
Subject: Re: bug#80116: [PATCH] hideshow: Reword documentation.
Date: Sat, 10 Jan 2026 14:57:00 +0200

> From: Elijah Gabe Pérez <eg642616 <at> gmail.com>
> Cc: 80116 <at> debbugs.gnu.org
> Date: Sun, 04 Jan 2026 17:15:07 -0600
> 
> Elijah Gabe Pérez <eg642616 <at> gmail.com> writes:
> 
> > Sending fixed patch:
> 
> I realized that this breaks one test, so I had to revert one change from
> the previous patch about the match data.
> 
> Since hideshow can use regexp and non-regexp matching, it's not easy
> know which of the two is being used, that's why the docstrings for some
> variables indicate it is needed to modify the match data, to make this
> "difference" clearer and less prone to bugs.

Thanks, installed on master, 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.