bug#51032: 29.0.50; Choices for blink-matching-paren

bug#51032: 29.0.50; Choices for blink-matching-paren

[Arash Esbati]

Hi all,

`blink-matching-paren' is defined as:

(defcustom blink-matching-paren t
  "Non-nil means show matching open-paren when close-paren is inserted.
If t, highlight the paren.  If `jump', briefly move cursor to its
position.  If `jump-offscreen', move cursor there even if the
position is off screen.  With any other non-nil value, the
off-screen position of the opening paren will be shown in the
echo area."
  :type '(choice
          (const :tag "Disable" nil)
          (const :tag "Highlight" t)
          (const :tag "Move cursor" jump)
          (const :tag "Move cursor, even if off screen" jump-offscreen))
  :group 'paren-blinking)

Is there a specific reason why users cannot choose this feature

    With any other non-nil value, the off-screen position of the opening
    paren will be shown in the echo area.

via customize interface?  Currently, one has to do

   (setq blink-matching-paren 'show)

in the init file to get that behavior.  Or am I missing something?

Best, Arash

bug#51032: 29.0.50; Choices for blink-matching-paren

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Arash Esbati <arash@gnu.org> writes:

>
> Is there a specific reason why users cannot choose this feature
>
>     With any other non-nil value, the off-screen position of the opening
>     paren will be shown in the echo area.
>
> via customize interface?  Currently, one has to do
>
>    (setq blink-matching-paren 'show)
>
> in the init file to get that behavior.  Or am I missing something?
>

I wonder if the documentation is correct.  The off-screen position of an
open paren is shown in the echo area when the variable is non-nil and
not 'jump-offscreen (and of course the open paren is off-screen).

I don't see how 'show is treated differently than t.

bug#51032: 29.0.50; Choices for blink-matching-paren

[Eli Zaretskii]

> From: Arash Esbati <arash@gnu.org>
> Date: Tue, 05 Oct 2021 14:18:35 +0200
> 
> (defcustom blink-matching-paren t
>   "Non-nil means show matching open-paren when close-paren is inserted.
> If t, highlight the paren.  If `jump', briefly move cursor to its
> position.  If `jump-offscreen', move cursor there even if the
> position is off screen.  With any other non-nil value, the
> off-screen position of the opening paren will be shown in the
> echo area."
>   :type '(choice
>           (const :tag "Disable" nil)
>           (const :tag "Highlight" t)
>           (const :tag "Move cursor" jump)
>           (const :tag "Move cursor, even if off screen" jump-offscreen))
>   :group 'paren-blinking)
> 
> Is there a specific reason why users cannot choose this feature
> 
>     With any other non-nil value, the off-screen position of the opening
>     paren will be shown in the echo area.
> 
> via customize interface?

Obviously, because when the variable was made a defcustom, no one
bothered to include that, and no one did since then.

Patches welcome.

bug#51032: 29.0.50; Choices for blink-matching-paren

[Stefan Kangas]

close 51032 28.1
thanks

Daniel Martín <mardani29@yahoo.es> writes:

> Arash Esbati <arash@gnu.org> writes:
>
>>
>> Is there a specific reason why users cannot choose this feature
>>
>>     With any other non-nil value, the off-screen position of the opening
>>     paren will be shown in the echo area.
>>
>> via customize interface?  Currently, one has to do
>>
>>    (setq blink-matching-paren 'show)
>>
>> in the init file to get that behavior.  Or am I missing something?
>>

Thanks for the bug report.

> I wonder if the documentation is correct.  The off-screen position of an
> open paren is shown in the echo area when the variable is non-nil and
> not 'jump-offscreen (and of course the open paren is off-screen).
>
> I don't see how 'show is treated differently than t.

Yup, there is no particular behavior associated with `show'.

Valid choices here include nil, t, `jump', and `jump-offscreen', and
while the last one will move the cursor off screen, t and jump will only
show the position in the echo area.

I have now fixed this on master (commit b2c50d7cf3) with the below
patch.  This change will be a part of the upcoming Emacs 28.1.

diff --git a/lisp/simple.el b/lisp/simple.el
index 3695415163..459fc67944 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -8419,11 +8419,16 @@ paren-blinking

 (defcustom blink-matching-paren t
   "Non-nil means show matching open-paren when close-paren is inserted.
-If t, highlight the paren.  If `jump', briefly move cursor to its
-position.  If `jump-offscreen', move cursor there even if the
-position is off screen.  With any other non-nil value, the
-off-screen position of the opening paren will be shown in the
-echo area."
+In addition, if the opening paren is not visible on screen, show
+its position in the echo area.
+
+The valid values are:
+
+  nil               Disable.
+  non-nil           Highlight the opening paren.
+  `jump'            Briefly move cursor to its position.
+  `jump-offscreen'  Briefly move cursor to its position,
+                    even if the opening paren is not on screen."
   :type '(choice
           (const :tag "Disable" nil)
           (const :tag "Highlight" t)

bug#51032: 29.0.50; Choices for blink-matching-paren

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 5 Oct 2021 17:23:22 -0400
> Cc: Arash Esbati <arash@gnu.org>, 51032@debbugs.gnu.org
> 
> > I wonder if the documentation is correct.  The off-screen position of an
> > open paren is shown in the echo area when the variable is non-nil and
> > not 'jump-offscreen (and of course the open paren is off-screen).
> >
> > I don't see how 'show is treated differently than t.
> 
> Yup, there is no particular behavior associated with `show'.
> 
> Valid choices here include nil, t, `jump', and `jump-offscreen', and
> while the last one will move the cursor off screen, t and jump will only
> show the position in the echo area.

And 'show' (or 'foo', or any other non-nil value) does NOT show the
matching paren in the echo area when that position is off-screen?  It
does here, FWIW.

>  (defcustom blink-matching-paren t
>    "Non-nil means show matching open-paren when close-paren is inserted.
> -If t, highlight the paren.  If `jump', briefly move cursor to its
> -position.  If `jump-offscreen', move cursor there even if the
> -position is off screen.  With any other non-nil value, the
> -off-screen position of the opening paren will be shown in the
> -echo area."
> +In addition, if the opening paren is not visible on screen, show
> +its position in the echo area.
> +
> +The valid values are:
> +
> +  nil               Disable.
> +  non-nil           Highlight the opening paren.
> +  `jump'            Briefly move cursor to its position.
                                              ^^^
What does "its" there allude to?

> +  `jump-offscreen'  Briefly move cursor to its position,

Likewise.

> +                    even if the opening paren is not on screen."

This doesn't update the choices, which is what the original bug report
was about.  And I think the new doc string is incomplete, because you
removed the information about showing matches that are far away in the
echo-area.

bug#51032: 29.0.50; Choices for blink-matching-paren

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> And 'show' (or 'foo', or any other non-nil value) does NOT show the
> matching paren in the echo area when that position is off-screen?  It
> does here, FWIW.

The behavior you see should be explained in the new docstring.  Did you
miss this part in the diff?

    +In addition, if the opening paren is not visible on screen, show
    +its position in the echo area.

>>  (defcustom blink-matching-paren t
>>    "Non-nil means show matching open-paren when close-paren is inserted.
>> -If t, highlight the paren.  If `jump', briefly move cursor to its
>> -position.  If `jump-offscreen', move cursor there even if the
>> -position is off screen.  With any other non-nil value, the
>> -off-screen position of the opening paren will be shown in the
>> -echo area."
>> +In addition, if the opening paren is not visible on screen, show
>> +its position in the echo area.
>> +
>> +The valid values are:
>> +
>> +  nil               Disable.
>> +  non-nil           Highlight the opening paren.
>> +  `jump'            Briefly move cursor to its position.
>                                               ^^^
> What does "its" there allude to?
>
>> +  `jump-offscreen'  Briefly move cursor to its position,
>
> Likewise.

The opening paren.  What "it" refers to is on the previous line.
I don't think this is unclear, but I don't object if you want to
improve it.

>> +                    even if the opening paren is not on screen."
>
> This doesn't update the choices, which is what the original bug report
> was about.

Yes it does, by changing t to non-nil.  Or maybe I don't understand what
you are saying.

> And I think the new doc string is incomplete, because you
> removed the information about showing matches that are far away in the
> echo-area.

I think you missed a sentence, see above.

bug#51032: 29.0.50; Choices for blink-matching-paren

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Wed, 6 Oct 2021 14:10:45 -0400
> Cc: mardani29@yahoo.es, arash@gnu.org, 51032@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > And 'show' (or 'foo', or any other non-nil value) does NOT show the
> > matching paren in the echo area when that position is off-screen?  It
> > does here, FWIW.
> 
> The behavior you see should be explained in the new docstring.  Did you
> miss this part in the diff?
> 
>     +In addition, if the opening paren is not visible on screen, show
>     +its position in the echo area.

But that's incorrect, because it doesn't happen when the value is
'jump' or 'jump-offscreen'.  The "in addition" part makes it sound
like it happens with any non-nil value.

> >> +                    even if the opening paren is not on screen."
> >
> > This doesn't update the choices, which is what the original bug report
> > was about.
> 
> Yes it does, by changing t to non-nil.  Or maybe I don't understand what
> you are saying.

How many possible values can this variable have, and how many are
listed in the menu of choices that will be presented to the user?

bug#51032: 29.0.50; Choices for blink-matching-paren

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>>     +In addition, if the opening paren is not visible on screen, show
>>     +its position in the echo area.
>
> But that's incorrect, because it doesn't happen when the value is
> 'jump' or 'jump-offscreen'.  The "in addition" part makes it sound
> like it happens with any non-nil value.

The exception you mention is listed immediately below, so I don't see
how it could be considered incorrect.

> How many possible values can this variable have, and how many are
> listed in the menu of choices that will be presented to the user?

I don't follow.  Do you want to change non-nil to t in the docstring?

bug#51032: 29.0.50; Choices for blink-matching-paren

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Wed, 6 Oct 2021 14:57:09 -0400
> Cc: mardani29@yahoo.es, arash@gnu.org, 51032@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >>     +In addition, if the opening paren is not visible on screen, show
> >>     +its position in the echo area.
> >
> > But that's incorrect, because it doesn't happen when the value is
> > 'jump' or 'jump-offscreen'.  The "in addition" part makes it sound
> > like it happens with any non-nil value.
> 
> The exception you mention is listed immediately below, so I don't see
> how it could be considered incorrect.

Where is it mentioned?

> > How many possible values can this variable have, and how many are
> > listed in the menu of choices that will be presented to the user?
> 
> I don't follow.  Do you want to change non-nil to t in the docstring?

I'm not talking about the doc string here, I'm talking about the
values in the value menu that the defcustom will show the user when
the user customizes this variable.

bug#51032: 29.0.50; Choices for blink-matching-paren

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm not talking about the doc string here, I'm talking about the
> values in the value menu that the defcustom will show the user when
> the user customizes this variable.

It's short enough, so I pasted its current definition below:

(defcustom blink-matching-paren t
  "Non-nil means show matching open-paren when close-paren is inserted.
In addition, if the opening paren is not visible on screen, show
its position in the echo area.

The valid values are:

  nil               Disable.
  non-nil           Highlight the opening paren.
  `jump'            Briefly move cursor to its position.
  `jump-offscreen'  Briefly move cursor to its position,
                    even if the opening paren is not on screen."
  :type '(choice
          (const :tag "Disable" nil)
          (const :tag "Highlight" t)
          (const :tag "Move cursor" jump)
          (const :tag "Move cursor, even if off screen" jump-offscreen))
  :group 'paren-blinking)

bug#51032: 29.0.50; Choices for blink-matching-paren

[Arash Esbati]

Stefan Kangas <stefan@marxist.se> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> I'm not talking about the doc string here, I'm talking about the
>> values in the value menu that the defcustom will show the user when
>> the user customizes this variable.

I think the values in the value menu are Ok, the only misleading part of
docstring was

    With any other non-nil value, the off-screen position of the opening
    paren will be shown in the echo area.

which is now (almost) fixed.

> It's short enough, so I pasted its current definition below:
>
> (defcustom blink-matching-paren t
>   "Non-nil means show matching open-paren when close-paren is inserted.
> In addition, if the opening paren is not visible on screen, show
> its position in the echo area.
>
> The valid values are:
>
>   nil               Disable.
>   non-nil           Highlight the opening paren.
>   `jump'            Briefly move cursor to its position.
>   `jump-offscreen'  Briefly move cursor to its position,
>                     even if the opening paren is not on screen."
>   :type '(choice
>           (const :tag "Disable" nil)
>           (const :tag "Highlight" t)
>           (const :tag "Move cursor" jump)
>           (const :tag "Move cursor, even if off screen" jump-offscreen))
>   :group 'paren-blinking)

Thanks Stefan.  May I suggest the following:

(defcustom blink-matching-paren t
  "Non-nil means show matching open-paren when close-paren is inserted.

Valid values are:

  nil               Disable.
  t                 Highlight the opening paren if on screen,
                    show the position in the echo area if off screen.
  `jump'            Briefly move cursor to the opening paren if on screen,
                    show the position in the echo area if off screen.
  `jump-offscreen'  Briefly move cursor to the opening paren,
                    even if the position is off screen."
  :type '(choice
          (const :tag "Disable" nil)
          (const :tag "Highlight" t)
          (const :tag "Move cursor" jump)
          (const :tag "Move cursor, even if off screen" jump-offscreen))
  :group 'paren-blinking)

Best, Arash

bug#51032: 29.0.50; Choices for blink-matching-paren

[martin rudalics]

 > Valid values are:
 >
 >    nil               Disable.
 >    t                 Highlight the opening paren if on screen,
 >                      show the position in the echo area if off screen.
 >    `jump'            Briefly move cursor to the opening paren if on screen,
 >                      show the position in the echo area if off screen.
 >    `jump-offscreen'  Briefly move cursor to the opening paren,
 >                      even if the position is off screen."
 >    :type '(choice
 >            (const :tag "Disable" nil)
 >            (const :tag "Highlight" t)
 >            (const :tag "Move cursor" jump)
 >            (const :tag "Move cursor, even if off screen" jump-offscreen))
 >    :group 'paren-blinking)

So what about "any other non-nil value"?

martin

bug#51032: 29.0.50; Choices for blink-matching-paren

[Arash Esbati]

martin rudalics <rudalics@gmx.at> writes:

>> Valid values are:
>>
>>    nil               Disable.
>>    t                 Highlight the opening paren if on screen,
>>                      show the position in the echo area if off screen.
>>    `jump'            Briefly move cursor to the opening paren if on screen,
>>                      show the position in the echo area if off screen.
>>    `jump-offscreen'  Briefly move cursor to the opening paren,
>>                      even if the position is off screen."
>>    :type '(choice
>>            (const :tag "Disable" nil)
>>            (const :tag "Highlight" t)
>>            (const :tag "Move cursor" jump)
>>            (const :tag "Move cursor, even if off screen" jump-offscreen))
>>    :group 'paren-blinking)
>
> So what about "any other non-nil value"?

"any other non-nil value" will act like t -- this is at least my
understanding.

Best, Arash

bug#51032: 29.0.50; Choices for blink-matching-paren

[martin rudalics]

 >> So what about "any other non-nil value"?
 >
 > "any other non-nil value" will act like t -- this is at least my
 > understanding.

Then we probably should say so.  But how does 'blink-matching-paren'
interact with 'blink-matching-paren-on-screen' non-nil (in particular
when 'blink-matching-paren' is 'jump')?  I'm completely ignorant here.
'blink-matching-paren-on-screen' is not documented, its doc-string says

"Non-nil means show matching open-paren when it is on screen."

What does "show" mean?  Highlight?  What does "screen" mean?  What
happens when the open paren is shown in another window, frame etc.

In either case your proposal looks good to me.

Thanks, martin

bug#51032: 29.0.50; Choices for blink-matching-paren

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Wed, 6 Oct 2021 16:53:40 -0400
> Cc: mardani29@yahoo.es, arash@gnu.org, 51032@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I'm not talking about the doc string here, I'm talking about the
> > values in the value menu that the defcustom will show the user when
> > the user customizes this variable.
> 
> It's short enough, so I pasted its current definition below:

Thanks.

I sense a certain degree of annoyance in your responses, so instead of
continuing the argument and risking to increase the aggravation, I
went ahead and made the improvements in the wording that I had in
mind.

bug#51032: 29.0.50; Choices for blink-matching-paren

[Eli Zaretskii]

> From: Arash Esbati <arash@gnu.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  mardani29@yahoo.es,  51032@debbugs.gnu.org
> Date: Thu, 07 Oct 2021 11:02:23 +0200
> 
> Stefan Kangas <stefan@marxist.se> writes:
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >> I'm not talking about the doc string here, I'm talking about the
> >> values in the value menu that the defcustom will show the user when
> >> the user customizes this variable.
> 
> I think the values in the value menu are Ok, the only misleading part of
> docstring was
> 
>     With any other non-nil value, the off-screen position of the opening
>     paren will be shown in the echo area.

It is IMO wrong to have that text there, because 'jump-offscreen'
doesn't do that, and 'jump' does it only of the open-paren is
off-screen.

Please take a look at the changes I just installed: are they OK?

Thanks.

bug#51032: 29.0.50; Choices for blink-matching-paren

[Arash Esbati]

martin rudalics <rudalics@gmx.at> writes:

>> "any other non-nil value" will act like t -- this is at least my
>> understanding.
>
> Then we probably should say so.  But how does 'blink-matching-paren'
> interact with 'blink-matching-paren-on-screen' non-nil (in particular
> when 'blink-matching-paren' is 'jump')?  I'm completely ignorant here.

Then we are 2.

> 'blink-matching-paren-on-screen' is not documented, its doc-string says
>
> "Non-nil means show matching open-paren when it is on screen."

The second sentence reads:

    If nil, don’t show it (but the open-paren can still be shown
    in the echo area when it is off screen).

My reading is this variable gives you more control in terms of: Don't
highlight the open-paren if on-screen (either jump or highlight), but do
if off-screen, i.e., show the position in the echo area.  But I think it
breaks for this combination:

    (setq blink-matching-paren 'jump-offscreen)
    (setq blink-matching-paren-on-screen nil)

which does nothing.  But that's another bug report from someone more
knowledgeable.

> In either case your proposal looks good to me.

Thanks, but Eli's patch is even better :)

Best, Arash

bug#51032: 29.0.50; Choices for blink-matching-paren

[Arash Esbati]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arash Esbati <arash@gnu.org>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  mardani29@yahoo.es,  51032@debbugs.gnu.org
>> Date: Thu, 07 Oct 2021 11:02:23 +0200
>> 
>> I think the values in the value menu are Ok, the only misleading part of
>> docstring was
>> 
>>     With any other non-nil value, the off-screen position of the opening
>>     paren will be shown in the echo area.
>
> It is IMO wrong to have that text there, because 'jump-offscreen'
> doesn't do that, and 'jump' does it only of the open-paren is
> off-screen.

Agreed.

> Please take a look at the changes I just installed: are they OK?

LGTM.  Thanks for the clarification.

Best, Arash

bug#51032: 29.0.50; Choices for blink-matching-paren

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> I went ahead and made the improvements in the wording that I had in
> mind.

The changes you installed look very good, thank you.

bug#51032: 29.0.50; Choices for blink-matching-paren

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 7 Oct 2021 09:05:09 -0400
> Cc: mardani29@yahoo.es, arash@gnu.org, 51032@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I went ahead and made the improvements in the wording that I had in
> > mind.
> 
> The changes you installed look very good, thank you.

Thanks for looking.

bug#51032: 29.0.50; Choices for blink-matching-paren

[Eli Zaretskii]

> From: Arash Esbati <arash@gnu.org>
> Date: Thu, 07 Oct 2021 12:17:01 +0200
> Cc: 51032@debbugs.gnu.org, Stefan Kangas <stefan@marxist.se>,
>  mardani29@yahoo.es
> 
> > "Non-nil means show matching open-paren when it is on screen."
> 
> The second sentence reads:
> 
>     If nil, don’t show it (but the open-paren can still be shown
>     in the echo area when it is off screen).
> 
> My reading is this variable gives you more control in terms of: Don't
> highlight the open-paren if on-screen (either jump or highlight), but do
> if off-screen, i.e., show the position in the echo area.  But I think it
> breaks for this combination:
> 
>     (setq blink-matching-paren 'jump-offscreen)
>     (setq blink-matching-paren-on-screen nil)
> 
> which does nothing.

Right.  It should jump off-screen in that case.

I think the effect of that option was never updated when we changed
how blink-matching-paren works in Emacs 24.