bug#63089: [PATCH] Display offscreen matched openparen

[Eli Zaretskii]

> From: Shynur Xie <one.last.kiss@outlook.com>
> Date: Wed, 26 Apr 2023 13:39:17 +0000
> 
> A line containing the matched open parenthesis will be displayed in
> the echo area if that parenthesis is offscreen when the user types a
> close parenthesis.
> 
> However, for example, the matched line may contain so many parentheses
> 
> ```
> (... (... ((((((((
> ...
> ...
> ```
> 
> that user will be confused by the text displayed in the echo area:
> 
> ```
> (... (... (((((
> ```
> 
> This patch changed a Lisp function `blink-matching-open' and rewrote a
> Lisp function `blink-paren-open-paren-line-string' in order to help
> user recognize the matched parenthesis more easily.

Thanks.

First, I think this should be an opt-in behavior, not the default.  We
remove properties from the text we show in the echo-area for a reason.
So please add a user option to enable this behavior.

Also, please don't unnecessarily introduce whitespace differences into
the code, like here:

> -       ((save-excursion (skip-chars-backward " \t") (not (bolp)))
> -        (setq regions (list (cons (line-beginning-position)
> -                                  (1+ pos)))))
> +       ;; Show what precedes the open in its line, if anything.
> +       ((save-excursion
> +          (skip-chars-backward " \t")
> +          (not (bolp)))

> +         #("Matches %s"
> +           ;; Make the following text (i.e., %s) prominent.
> +           0 7 (face (:weight light)))
> +         (blink-paren-open-paren-line-string blinkpos)))))))

This assume the font used in the echo-area must have a light variant
installed; if it doesn't, Emacs might select a different font and/or
the same "normal" weight.  So I'm not sure this is a good idea, in
general.  Why not use the 'shadow' face instead?

I'm not sure I understand why you use backticks in some parts of the
patch.  For example:

> -       ((save-excursion (skip-chars-backward " \t") (not (bolp)))
> -        (setq regions (list (cons (line-beginning-position)
> -                                  (1+ pos)))))
> +       ;; Show what precedes the open in its line, if anything.
> +       ((save-excursion
> +          (skip-chars-backward " \t")
> +          (not (bolp)))
> +        (let ((bol (line-beginning-position)))
> +          (setq regions `((,bol . ,(1+ pos)))
> +                openparen-idx (- pos bol))))

The original code didn't use backticks, so why do you need it in the
new version?

> +            (1+openparen-idx (1+ openparen-idx)))
                ^^^^^^^^^^^^^^^
This is a strange and confusing notation, please find a better name
for this variable.

Finally, this patch is too large to accept without copyright
assignment.  What is the status of your legal paperwork?