unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#63089: [PATCH] Display offscreen matched openparen
@ 2023-04-26 13:39 Shynur Xie
  2023-04-28  6:28 ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Shynur Xie @ 2023-04-26 13:39 UTC (permalink / raw)
  To: 63089

[-- Attachment #1: Type: text/plain, Size: 584 bytes --]

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.

--
shynur

[-- Attachment #2: 0001-Display-offscreen-matched-openparen.patch --]
[-- Type: application/octet-stream, Size: 5416 bytes --]

From 8c904b9db10cf3f6d3bf53da8818d2fe1a90f23d Mon Sep 17 00:00:00 2001
From: Shynur <one.last.kiss@outlook.com>
Date: Wed, 26 Apr 2023 21:16:20 +0800
Subject: [PATCH] Display offscreen matched openparen

Propertize the matched openparen displayed in the echo area in order to make
it prominent; use light font for non-context characters (i.e., Matches').
* lisp/simple.el (blink-matching-open): Light font for 'Matches'.
* lisp/simple.el (blink-paren-open-paren-line-string): Add a face to the
matched openparen.
---
 lisp/simple.el | 78 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 23 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index b621e1603bd..bb977517ab2 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -9312,47 +9312,79 @@ blink-matching-open
                  (delete-overlay blink-matching--overlay)))))
        ((not show-paren-context-when-offscreen)
         (minibuffer-message
-         "Matches %s"
-         (substring-no-properties
-          (blink-paren-open-paren-line-string blinkpos))))))))
+         #("Matches %s"
+           ;; Make the following text (i.e., %s) prominent.
+           0 7 (face (:weight light)))
+         (blink-paren-open-paren-line-string blinkpos)))))))
 
 (defun blink-paren-open-paren-line-string (pos)
-  "Return the line string that contains the openparen at POS."
+  "Return the line string that contains the openparen at POS.
+Remove the line string's properties; give the openparen a face."
   (save-excursion
     (goto-char pos)
     ;; Capture the regions in terms of (beg . end) conses whose
     ;; buffer-substrings we want to show as a context string.  Ensure
     ;; they are font-locked (bug#59527).
-    (let (regions)
-      ;; Show what precedes the open in its line, if anything.
+    (let (regions
+          openparen-idx)
       (cond
-       ((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))))
        ;; Show what follows the open in its line, if anything.
        ((save-excursion
           (forward-char 1)
           (skip-chars-forward " \t")
           (not (eolp)))
-        (setq regions (list (cons pos (line-end-position)))))
+        (setq regions `((,pos . ,(line-end-position)))
+              openparen-idx 0))
        ;; Otherwise show the previous nonblank line,
        ;; if there is one.
-       ((save-excursion (skip-chars-backward "\n \t") (not (bobp)))
-        (setq regions (list (cons (progn
-                                    (skip-chars-backward "\n \t")
-                                    (line-beginning-position))
-                                  (progn (end-of-line)
-                                         (skip-chars-backward " \t")
-                                         (point)))
-                            (cons pos (1+ pos)))))
+       ((save-excursion
+          (skip-chars-backward "\n \t")
+          (not (bobp)))
+        (setq regions `((,(let (bol)
+                            (skip-chars-backward "\n \t")
+                            (setq bol (line-beginning-position)
+                                  openparen-idx (- bol))
+                            bol)
+                         . ,(let (eol)
+                              (end-of-line)
+                              (skip-chars-backward " \t")
+                              (setq eol (point)
+                                    openparen-idx (+ openparen-idx
+                                                     eol
+                                                     ;; (length "...")
+                                                     3))
+                              eol))
+                        (,pos . ,(1+ pos)))))
        ;; There is nothing to show except the char itself.
-       (t (setq regions (list (cons pos (1+ pos))))))
+       (t
+        (setq regions `((,pos . ,(1+ pos)))
+              openparen-idx 0)))
       ;; Ensure we've font-locked the context region.
       (font-lock-ensure (caar regions) (cdar (last regions)))
-      (mapconcat (lambda (region)
-                   (buffer-substring (car region) (cdr region)))
-                 regions
-                 "..."))))
+      (let ((line-string
+             (mapconcat
+              (lambda (region)
+                (buffer-substring (car region) (cdr region)))
+              regions
+              "..."))
+            (1+openparen-idx (1+ openparen-idx)))
+        (setq line-string (substring-no-properties line-string))
+        (concat
+         (substring line-string
+                    0 openparen-idx)
+         (propertize (substring line-string
+                                openparen-idx 1+openparen-idx)
+                     ;; Maybe its face should be customizable.
+                     'face '(:foreground "green"))
+         (substring line-string
+                    1+openparen-idx))))))
 
 (defvar blink-paren-function 'blink-matching-open
   "Function called, if non-nil, whenever a close parenthesis is inserted.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#63089: [PATCH] Display offscreen matched openparen
  2023-04-26 13:39 bug#63089: [PATCH] Display offscreen matched openparen Shynur Xie
@ 2023-04-28  6:28 ` Eli Zaretskii
  2023-04-28 12:36   ` Shynur Xie
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2023-04-28  6:28 UTC (permalink / raw)
  To: Shynur Xie; +Cc: 63089

> 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?





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#63089: [PATCH] Display offscreen matched openparen
  2023-04-28  6:28 ` Eli Zaretskii
@ 2023-04-28 12:36   ` Shynur Xie
  2023-04-29 11:05     ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Shynur Xie @ 2023-04-28 12:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 63089@debbugs.gnu.org

[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]

>    From: Eli Zaretskii
> Subject: bug#63089: [PATCH] Display offscreen matched openparen
>    Date: Fri, 28 Apr 2023 09:28:53 +0300
>      To: Shynur Xie
>
> please add a user option to enable this behavior.

I think it can be defcustomed in file "lisp/paren.el".  New version of
the patch is complete, please see the attached.

> don't unnecessarily introduce whitespace differences into the code

Got it.

> Why not use the 'shadow' face instead?

Thanks.  Have used the face 'shadow'.

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

My _original_ modification made some lines too long with `list' and
`cons', so I used all backticks in that function.  Since there's no
such problem in the subsequent modifications, I will use `list' and
`cons' if you think backticks are unnecessary (or weird).

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

Have changed it to 'openparen-next-char-idx'.

> What is the status of your legal paperwork?

My assignment process with the FSF is complete.

Thanks again for your review of my patch!

--
shynur

[-- Attachment #2: 0001-Display-offscreen-matched-openparen.patch --]
[-- Type: application/octet-stream, Size: 6207 bytes --]

From 8644fa78a05ffe9a06d611dae0d4d45a413a34a0 Mon Sep 17 00:00:00 2001
From: Shynur <one.last.kiss@outlook.com>
Date: Fri, 28 Apr 2023 20:23:16 +0800
Subject: [PATCH] Display offscreen matched openparen

Propertize the matched openparen displayed in the echo area in order to make
it prominent; use light font for non-context characters (i.e., 'Matches').
* lisp/simple.el (blink-matching-open): Set face shadow for 'Matches'.
* lisp/simple.el (blink-paren-open-paren-line-string): Propertize the macthed
openparen's face by user option `show-paren-openparen-face-in-message'.
* lisp/paren.el (show-paren-openparen-face-in-message): Add a user option
which determines the face of the matched offscreen openparen shown in the
echo area.
---
 lisp/paren.el  |  8 ++++++
 lisp/simple.el | 68 ++++++++++++++++++++++++++++++++++----------------
 2 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/lisp/paren.el b/lisp/paren.el
index 4c91fd29490..b9ce3cbd45d 100644
--- a/lisp/paren.el
+++ b/lisp/paren.el
@@ -81,6 +81,14 @@ show-paren-when-point-in-periphery
   :type 'boolean
   :version "25.1")
 
+(defcustom show-paren-openparen-face-in-message '(:foreground "green")
+  "Set face for the matched offscreen openparen shown in the echo area.
+By default, the line containing the matched offscreen openparen is
+shown in the echo area, where the openparen's face will be propertized
+by this option."
+  :type '(choice face sexp (const nil))
+  :version "30.0")
+
 (defcustom show-paren-highlight-openparen t
   "Non-nil turns on openparen highlighting when matching forward.
 When nil, and point stands just before an open paren, the paren
diff --git a/lisp/simple.el b/lisp/simple.el
index b621e1603bd..997c600b6b6 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -9312,47 +9312,73 @@ blink-matching-open
                  (delete-overlay blink-matching--overlay)))))
        ((not show-paren-context-when-offscreen)
         (minibuffer-message
-         "Matches %s"
-         (substring-no-properties
-          (blink-paren-open-paren-line-string blinkpos))))))))
+         #("Matches %s"
+           ;; Make the following text (i.e., %s) prominent.
+           0 7 (face shadow))
+         (blink-paren-open-paren-line-string blinkpos)))))))
 
 (defun blink-paren-open-paren-line-string (pos)
-  "Return the line string that contains the openparen at POS."
+  "Return the line string that contains the openparen at POS.
+Remove the line string's properties but give the openparen a face."
   (save-excursion
     (goto-char pos)
     ;; Capture the regions in terms of (beg . end) conses whose
     ;; buffer-substrings we want to show as a context string.  Ensure
     ;; they are font-locked (bug#59527).
-    (let (regions)
-      ;; Show what precedes the open in its line, if anything.
+    (let (regions
+          openparen-idx)
       (cond
+       ;; Show what precedes the open in its line, if anything.
        ((save-excursion (skip-chars-backward " \t") (not (bolp)))
-        (setq regions (list (cons (line-beginning-position)
-                                  (1+ pos)))))
+        (let ((bol (line-beginning-position)))
+          (setq regions `((,bol . ,(1+ pos)))
+                openparen-idx (- pos bol))))
        ;; Show what follows the open in its line, if anything.
        ((save-excursion
           (forward-char 1)
           (skip-chars-forward " \t")
           (not (eolp)))
-        (setq regions (list (cons pos (line-end-position)))))
+        (setq regions `((,pos . ,(line-end-position)))
+              openparen-idx 0))
        ;; Otherwise show the previous nonblank line,
        ;; if there is one.
        ((save-excursion (skip-chars-backward "\n \t") (not (bobp)))
-        (setq regions (list (cons (progn
-                                    (skip-chars-backward "\n \t")
-                                    (line-beginning-position))
-                                  (progn (end-of-line)
-                                         (skip-chars-backward " \t")
-                                         (point)))
-                            (cons pos (1+ pos)))))
+        (setq regions `((,(let (bol)
+                            (skip-chars-backward "\n \t")
+                            (setq bol (line-beginning-position)
+                                  openparen-idx (- bol))
+                            bol)
+                         . ,(let (eol)
+                              (end-of-line)
+                              (skip-chars-backward " \t")
+                              (setq eol (point)
+                                    openparen-idx (+ openparen-idx
+                                                     eol
+                                                     ;; (length "...")
+                                                     3))
+                              eol))
+                        (,pos . ,(1+ pos)))))
        ;; There is nothing to show except the char itself.
-       (t (setq regions (list (cons pos (1+ pos))))))
+       (t (setq regions `((,pos . ,(1+ pos)))
+                openparen-idx 0)))
       ;; Ensure we've font-locked the context region.
       (font-lock-ensure (caar regions) (cdar (last regions)))
-      (mapconcat (lambda (region)
-                   (buffer-substring (car region) (cdr region)))
-                 regions
-                 "..."))))
+      (let ((line-string
+             (mapconcat
+              (lambda (region)
+                (buffer-substring (car region) (cdr region)))
+              regions
+              "..."))
+            (openparen-next-char-idx (1+ openparen-idx)))
+        (setq line-string (substring-no-properties line-string))
+        (concat
+         (substring line-string
+                    0 openparen-idx)
+         (propertize (substring line-string
+                                openparen-idx openparen-next-char-idx)
+                     'face show-paren-openparen-face-in-message)
+         (substring line-string
+                    openparen-next-char-idx))))))
 
 (defvar blink-paren-function 'blink-matching-open
   "Function called, if non-nil, whenever a close parenthesis is inserted.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#63089: [PATCH] Display offscreen matched openparen
  2023-04-28 12:36   ` Shynur Xie
@ 2023-04-29 11:05     ` Eli Zaretskii
  2023-04-30 10:09       ` Shynur Xie
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2023-04-29 11:05 UTC (permalink / raw)
  To: Shynur Xie; +Cc: 63089

> From: Shynur Xie <one.last.kiss@outlook.com>
> CC: "63089@debbugs.gnu.org" <63089@debbugs.gnu.org>
> Date: Fri, 28 Apr 2023 12:36:03 +0000
> 
> > The original code didn't use backticks, so why do you need it in the
> > new version?
> 
> My _original_ modification made some lines too long with `list' and
> `cons', so I used all backticks in that function.  Since there's no
> such problem in the subsequent modifications, I will use `list' and
> `cons' if you think backticks are unnecessary (or weird).

Backticks usually imply some run-time processing, which AFAIU here is
not required.

> > What is the status of your legal paperwork?
> 
> My assignment process with the FSF is complete.

Yes, I see it on file now.

> +(defcustom show-paren-openparen-face-in-message '(:foreground "green")
> +  "Set face for the matched offscreen openparen shown in the echo area.

 "Face for showing in the echo area matched open paren that is off-screen."

Also, I think the default value should be the default face, so that
the default behavior is not changed.

> +By default, the line containing the matched offscreen openparen is
> +shown in the echo area, where the openparen's face will be propertized
> +by this option."

"face will be propertized: is incorrect: we propertize text with a
face, we don't propertize the face.

> +  :type '(choice face sexp (const nil))
> +  :version "30.0")

This should be "30.1".  Emacs doesn't have NN.0 versions.

>  (defun blink-paren-open-paren-line-string (pos)
> -  "Return the line string that contains the openparen at POS."
> +  "Return the line string that contains the openparen at POS.
> +Remove the line string's properties but give the openparen a face."

This should include the name of the face, so that users could find it
easier.

Thanks.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#63089: [PATCH] Display offscreen matched openparen
  2023-04-29 11:05     ` Eli Zaretskii
@ 2023-04-30 10:09       ` Shynur Xie
  2023-05-01 13:17         ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Shynur Xie @ 2023-04-30 10:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 63089@debbugs.gnu.org

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]

>    From: Eli Zaretskii
> Subject: bug#63089
>    Date: Sat, 29 Apr 2023 14:05:56 +0300
>      To: Shynur Xie
>
> Backticks usually imply some run-time processing, which AFAIU here
> is not required.

Have replaced backtickes with `list's and `cons'es.

>> +  "Set face for the matched offscreen openparen shown in the echo area.
>
> "Face for showing in the echo area matched open paren that is off-screen."

Have changed to the latter.

> I think the default value should be the default face.

Its default value is nil now.  I struggled with whether `nil' (it can
be seen as an empty anonymous face) was better or `default'.

> "face will be propertized" is incorrect: we propertize text with a
> face, we don't propertize the face.

Have replaced
"the openparen's face will be propertized by this option"
with
"the openparen will be propertized with a face based on the value of
 this option".

> This should be "30.1".  Emacs doesn't have NN.0 versions.

Got it.

>> +  "Return the line string that contains the openparen at POS.
>> +Remove the line string's properties but give the openparen a face."
>
> This should include the name of the face,

The name of the face has been added:
"...... give the openparen a face based on the option
 `show-paren-openparen-face-in-message'."

______________________

New patch is attached.

Will keep your guidance in mind.  Thanks!

--
shynur

[-- Attachment #2: 0001-Display-offscreen-matched-openparen.patch --]
[-- Type: application/octet-stream, Size: 6336 bytes --]

From 03ea46b66128ee906cb829a0fa62f2da5ba68f94 Mon Sep 17 00:00:00 2001
From: Shynur <one.last.kiss@outlook.com>
Date: Sun, 30 Apr 2023 17:54:01 +0800
Subject: [PATCH] Display offscreen matched openparen

Propertize the matched openparen displayed in the echo area in order to make
it prominent; use light font for non-context characters (i.e., 'Matches').
* lisp/simple.el (blink-matching-open): Set face shadow for 'Matches'.
* lisp/simple.el (blink-paren-open-paren-line-string): Propertize the macthed
openparen's face by user option `show-paren-openparen-face-in-message'.
* lisp/paren.el (show-paren-openparen-face-in-message): Add a user option
which determines the face of the matched offscreen openparen shown in the
echo area.
---
 lisp/paren.el  |  8 ++++++
 lisp/simple.el | 68 +++++++++++++++++++++++++++++++++++---------------
 2 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/lisp/paren.el b/lisp/paren.el
index 4c91fd29490..f6a0c1c8b6b 100644
--- a/lisp/paren.el
+++ b/lisp/paren.el
@@ -81,6 +81,14 @@ whitespace there."
   :type 'boolean
   :version "25.1")
 
+(defcustom show-paren-openparen-face-in-message nil
+  "Face for showing in the echo area matched open paren that is off-screen.
+By default, the line containing the matched offscreen openparen
+is shown in the echo area, where the openparen will be propertized
+with a face based on the value of this option."
+  :type '(choice face sexp (const nil))
+  :version "30.1")
+
 (defcustom show-paren-highlight-openparen t
   "Non-nil turns on openparen highlighting when matching forward.
 When nil, and point stands just before an open paren, the paren
diff --git a/lisp/simple.el b/lisp/simple.el
index b621e1603bd..4d8f24e3252 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -9312,47 +9312,75 @@ The function should return non-nil if the two tokens do not match.")
                  (delete-overlay blink-matching--overlay)))))
        ((not show-paren-context-when-offscreen)
         (minibuffer-message
-         "Matches %s"
-         (substring-no-properties
-          (blink-paren-open-paren-line-string blinkpos))))))))
+         #("Matches %s"
+           ;; Make the following text (i.e., %s) prominent.
+           0 7 (face shadow))
+         (blink-paren-open-paren-line-string blinkpos)))))))
 
 (defun blink-paren-open-paren-line-string (pos)
-  "Return the line string that contains the openparen at POS."
+  "Return the line string that contains the openparen at POS.
+Remove the line string's properties but give the openparen a
+face based on the option `show-paren-openparen-face-in-message'."
   (save-excursion
     (goto-char pos)
     ;; Capture the regions in terms of (beg . end) conses whose
     ;; buffer-substrings we want to show as a context string.  Ensure
     ;; they are font-locked (bug#59527).
-    (let (regions)
-      ;; Show what precedes the open in its line, if anything.
+    (let (regions
+          openparen-idx)
       (cond
+       ;; Show what precedes the open in its line, if anything.
        ((save-excursion (skip-chars-backward " \t") (not (bolp)))
-        (setq regions (list (cons (line-beginning-position)
-                                  (1+ pos)))))
+        (let ((bol (line-beginning-position)))
+          (setq regions (list (cons bol (1+ pos)))
+                openparen-idx (- pos bol))))
        ;; Show what follows the open in its line, if anything.
        ((save-excursion
           (forward-char 1)
           (skip-chars-forward " \t")
           (not (eolp)))
-        (setq regions (list (cons pos (line-end-position)))))
+        (setq regions (list (cons pos (line-end-position)))
+              openparen-idx 0))
        ;; Otherwise show the previous nonblank line,
        ;; if there is one.
        ((save-excursion (skip-chars-backward "\n \t") (not (bobp)))
-        (setq regions (list (cons (progn
-                                    (skip-chars-backward "\n \t")
-                                    (line-beginning-position))
-                                  (progn (end-of-line)
-                                         (skip-chars-backward " \t")
-                                         (point)))
+        (setq regions (list (cons
+                             (let (bol)
+                               (skip-chars-backward "\n \t")
+                               (setq bol (line-beginning-position)
+                                     openparen-idx (- bol))
+                               bol)
+                             (let (eol)
+                               (end-of-line)
+                               (skip-chars-backward " \t")
+                               (setq eol (point)
+                                     openparen-idx (+ openparen-idx
+                                                      eol
+                                                      ;; (length "...")
+                                                      3))
+                               eol))
                             (cons pos (1+ pos)))))
        ;; There is nothing to show except the char itself.
-       (t (setq regions (list (cons pos (1+ pos))))))
+       (t (setq regions (list (cons pos (1+ pos)))
+                openparen-idx 0)))
       ;; Ensure we've font-locked the context region.
       (font-lock-ensure (caar regions) (cdar (last regions)))
-      (mapconcat (lambda (region)
-                   (buffer-substring (car region) (cdr region)))
-                 regions
-                 "..."))))
+      (let ((line-string
+             (mapconcat
+              (lambda (region)
+                (buffer-substring (car region) (cdr region)))
+              regions
+              "..."))
+            (openparen-next-char-idx (1+ openparen-idx)))
+        (setq line-string (substring-no-properties line-string))
+        (concat
+         (substring line-string
+                    0 openparen-idx)
+         (propertize (substring line-string
+                                openparen-idx openparen-next-char-idx)
+                     'face show-paren-openparen-face-in-message)
+         (substring line-string
+                    openparen-next-char-idx))))))
 
 (defvar blink-paren-function 'blink-matching-open
   "Function called, if non-nil, whenever a close parenthesis is inserted.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#63089: [PATCH] Display offscreen matched openparen
  2023-04-30 10:09       ` Shynur Xie
@ 2023-05-01 13:17         ` Eli Zaretskii
  2023-05-01 17:52           ` Shynur Xie
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2023-05-01 13:17 UTC (permalink / raw)
  To: Shynur Xie; +Cc: 63089

> From: Shynur Xie <one.last.kiss@outlook.com>
> CC: "63089@debbugs.gnu.org" <63089@debbugs.gnu.org>
> Date: Sun, 30 Apr 2023 10:09:18 +0000
> 
> > I think the default value should be the default face.
> 
> Its default value is nil now.  I struggled with whether `nil' (it can
> be seen as an empty anonymous face) was better or `default'.
> 
> > "face will be propertized" is incorrect: we propertize text with a
> > face, we don't propertize the face.
> 
> Have replaced
> "the openparen's face will be propertized by this option"
> with
> "the openparen will be propertized with a face based on the value of
>  this option".
> 
> > This should be "30.1".  Emacs doesn't have NN.0 versions.
> 
> Got it.
> 
> >> +  "Return the line string that contains the openparen at POS.
> >> +Remove the line string's properties but give the openparen a face."
> >
> > This should include the name of the face,
> 
> The name of the face has been added:
> "...... give the openparen a face based on the option
>  `show-paren-openparen-face-in-message'."

Thanks.  Did you try "make bootstrap" with these changes?  The fact
that the new option is in paren.el but the code is in simple.el
worries me a bit: simple.el is preloaded before paren.el, so this
variable might not be known.  I think we should move the option to
simple,el, and rename it to blink-paren-SOMETHING.

Also, it is unusual to have a defcustom that names a face without a
corresponding defface that can be used to customize the face.  So I
think we should add a defface for the face used when the user option
is non-nil.

> Propertize the matched openparen displayed in the echo area in order to make
> it prominent; use light font for non-context characters (i.e., 'Matches').
> * lisp/simple.el (blink-matching-open): Set face shadow for 'Matches'.
> * lisp/simple.el (blink-paren-open-paren-line-string): Propertize the macthed
                                                                        ^^^^^^^
Typo there.

Also, the lines in the log message are too long, please make them at
most 70 column long.

Thanks.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#63089: [PATCH] Display offscreen matched openparen
  2023-05-01 13:17         ` Eli Zaretskii
@ 2023-05-01 17:52           ` Shynur Xie
  2023-05-02 18:38             ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Shynur Xie @ 2023-05-01 17:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 63089@debbugs.gnu.org

[-- Attachment #1: Type: text/plain, Size: 1134 bytes --]

>    From: Eli Zaretskii
> Subject: Re: bug#63089
>    Date: Mon, 01 May 2023 16:17:44 +0300
>      To: Shynur Xie
>
> I think we should move the option to simple.el, and rename it to
> blink-paren-SOMETHING.

There're several blink-matching-paren-* options in file <simple.el>,
but no option's name is blink-paren-*.  So I think perhaps it's better
to rename it to blink-matching-paren-highlight-offscreen.

> Also, it is unusual to have a defcustom that names a face without a
> corresponding defface that can be used to customize the face.  So I
> think we should add a defface for the face used when the user option
> is non-nil.

Following your instruction, I defface blink-matching-paren-offscreen.

> Also, the lines in the log message are too long, please make them at
> most 70 column long.

Got it.  It seems that <www.gnu.org/software/emacs/CONTRIBUTE> need to
be changed; it said "Limit lines in commit messages to 78 characters".

____________________

New patch is attached.  I have checked that there's no conflict
between the 2 newly introduced names and the original names.

--
shynur

[-- Attachment #2: 0001-Display-matched-offscreen-openparen.patch --]
[-- Type: application/octet-stream, Size: 6518 bytes --]

From e29c92ae8137daf12a87c1ed213cbca6667ea7f1 Mon Sep 17 00:00:00 2001
From: Shynur <one.last.kiss@outlook.com>
Date: Tue, 2 May 2023 01:32:44 +0800
Subject: [PATCH] Display matched offscreen openparen

Propertize matched offscreen openparen that is showing in the echo
area in order to make it prominent; use shadow face for non-context
characters (i.e., 'Matches') for the same purpose.
* lisp/simple.el (blink-matching-paren-offscreen): Add this face for
highlighting.
* lisp/simple.el (blink-matching-paren-highlight-offscreen): Add this
option to toggle face `blink-matching-paren-offscreen'.
* lisp/simple.el (blink-paren-open-paren-line-string): Propertize the
matched offscreen openparen with a face conditionally.
---
 lisp/simple.el | 87 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 67 insertions(+), 20 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index b621e1603bd..e4a0b9549e0 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -9215,6 +9215,21 @@ blink-matching-paren-dont-ignore-comments
   :type 'boolean
   :group 'paren-blinking)
 
+(defcustom blink-matching-paren-highlight-offscreen nil
+  "If non-nil, highlight showing in the echo area matched off-screen open paren.
+This highlighting uses face `blink-matching-paren-offscreen'."
+  :type 'boolean
+  :version "30.1"
+  :group 'paren-blinking)
+
+(defface blink-matching-paren-offscreen
+  '((t :foreground "green"))
+  "Face for showing in the echo area matched open paren that is off-screen.
+This face will not be used when `blink-matching-paren-highlight-offscreen'
+is nil."
+  :version "30.1"
+  :group 'paren-blinking)
+
 (defun blink-matching-check-mismatch (start end)
   "Return whether or not START...END are matching parens.
 END is the current point and START is the blink position.
@@ -9312,47 +9327,79 @@ blink-matching-open
                  (delete-overlay blink-matching--overlay)))))
        ((not show-paren-context-when-offscreen)
         (minibuffer-message
-         "Matches %s"
-         (substring-no-properties
-          (blink-paren-open-paren-line-string blinkpos))))))))
+         #("Matches %s"
+           ;; Make the following text (i.e., %s) prominent.
+           0 7 (face shadow))
+         (blink-paren-open-paren-line-string blinkpos)))))))
 
 (defun blink-paren-open-paren-line-string (pos)
-  "Return the line string that contains the openparen at POS."
+  "Return the line string that contains the openparen at POS.
+Remove the line string's properties but give the openparen a
+face if `blink-matching-paren-highlight-offscreen' is non-nil."
   (save-excursion
     (goto-char pos)
     ;; Capture the regions in terms of (beg . end) conses whose
     ;; buffer-substrings we want to show as a context string.  Ensure
     ;; they are font-locked (bug#59527).
-    (let (regions)
-      ;; Show what precedes the open in its line, if anything.
+    (let (regions
+          openparen-idx)
       (cond
+       ;; Show what precedes the open in its line, if anything.
        ((save-excursion (skip-chars-backward " \t") (not (bolp)))
-        (setq regions (list (cons (line-beginning-position)
-                                  (1+ pos)))))
+        (let ((bol (line-beginning-position)))
+          (setq regions (list (cons bol (1+ pos)))
+                openparen-idx (- pos bol))))
        ;; Show what follows the open in its line, if anything.
        ((save-excursion
           (forward-char 1)
           (skip-chars-forward " \t")
           (not (eolp)))
-        (setq regions (list (cons pos (line-end-position)))))
+        (setq regions (list (cons pos (line-end-position)))
+              openparen-idx 0))
        ;; Otherwise show the previous nonblank line,
        ;; if there is one.
        ((save-excursion (skip-chars-backward "\n \t") (not (bobp)))
-        (setq regions (list (cons (progn
-                                    (skip-chars-backward "\n \t")
-                                    (line-beginning-position))
-                                  (progn (end-of-line)
-                                         (skip-chars-backward " \t")
-                                         (point)))
+        (setq regions (list (cons
+                             (let (bol)
+                               (skip-chars-backward "\n \t")
+                               (setq bol (line-beginning-position)
+                                     openparen-idx (- bol))
+                               bol)
+                             (let (eol)
+                               (end-of-line)
+                               (skip-chars-backward " \t")
+                               (setq eol (point)
+                                     openparen-idx (+ openparen-idx
+                                                      eol
+                                                      ;; (length "...")
+                                                      3))
+                               eol))
                             (cons pos (1+ pos)))))
        ;; There is nothing to show except the char itself.
-       (t (setq regions (list (cons pos (1+ pos))))))
+       (t (setq regions (list (cons pos (1+ pos)))
+                openparen-idx 0)))
       ;; Ensure we've font-locked the context region.
       (font-lock-ensure (caar regions) (cdar (last regions)))
-      (mapconcat (lambda (region)
-                   (buffer-substring (car region) (cdr region)))
-                 regions
-                 "..."))))
+      (let ((line-string
+             (mapconcat
+              (lambda (region)
+                (buffer-substring (car region) (cdr region)))
+              regions
+              "..."))
+            (openparen-next-char-idx (1+ openparen-idx)))
+        (setq line-string (substring-no-properties line-string))
+        (concat
+         (substring line-string
+                    0 openparen-idx)
+         (let ((matched-offscreen-openparen
+                (substring line-string
+                           openparen-idx openparen-next-char-idx)))
+           (if blink-matching-paren-highlight-offscreen
+               (propertize matched-offscreen-openparen
+                           'face 'blink-matching-paren-offscreen)
+             matched-offscreen-openparen))
+         (substring line-string
+                    openparen-next-char-idx))))))
 
 (defvar blink-paren-function 'blink-matching-open
   "Function called, if non-nil, whenever a close parenthesis is inserted.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#63089: [PATCH] Display offscreen matched openparen
  2023-05-01 17:52           ` Shynur Xie
@ 2023-05-02 18:38             ` Eli Zaretskii
  0 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2023-05-02 18:38 UTC (permalink / raw)
  To: Shynur Xie; +Cc: 63089-done

> From: Shynur Xie <one.last.kiss@outlook.com>
> CC: "63089@debbugs.gnu.org" <63089@debbugs.gnu.org>
> Date: Mon, 1 May 2023 17:52:44 +0000
> 
> New patch is attached.  I have checked that there's no conflict
> between the 2 newly introduced names and the original names.

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





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-05-02 18:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-26 13:39 bug#63089: [PATCH] Display offscreen matched openparen Shynur Xie
2023-04-28  6:28 ` Eli Zaretskii
2023-04-28 12:36   ` Shynur Xie
2023-04-29 11:05     ` Eli Zaretskii
2023-04-30 10:09       ` Shynur Xie
2023-05-01 13:17         ` Eli Zaretskii
2023-05-01 17:52           ` Shynur Xie
2023-05-02 18:38             ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).