Re: Proposed extension of show-paren-mode: Highlight parens when point is in L or R margin.

[Alan Mackenzie]

Hi, Stefan.

On Tue, Oct 14, 2014 at 01:49:21PM -0400, Stefan Monnier wrote:
> >> > Then again, why not do the same if point is in a line comment?
> >> Sorry, I don't know what "in a line comment" means.
> > In a comment which extends to EOL, e.g. "//..." in C++, or ";..." in
> > Lisp.

> But where should the open/close paren be?  And I guess the rule applies
> also to the case where you're not inside the comment, but right in front
> of it, or between the "end of text" and the "beginning of
> comment", right?

> And really, the rule should also apply to /*..*/ comments (probably with
> some restrictions such as point being on the same line as the /* or the
> */) since Emacs's C code always uses them even when //... could be
> used instead.

Sorry, I expressed myself really badly.  I'll try again.  Dividing a
source file line into LH periphery, core, RH periphery:

            foo = /* */ bar; /* comment */
PPPPPPPPPPPP                PPPPPPPPPPPPPP
            CCCCCCCCCCCCCCCC

The periphery of a line does not extend outside that line.  An "internal
comment" inside a line (a fairly rare thing) is not part of the
periphery.  Comments before the core are vanishingly rare, hence not
included in the definition.

The candidates for a pertinent parenthesis are then:
(i) A paren that point is immediately outside of.  A closing paren takes
priority over an opening one.  (This is the existing implementation.)
(ii) (Only when point is in the periphery) a paren at the beginning or
end of the core.  The one nearest point takes priority.

> >> > + (defcustom show-paren-when-point-in-margin nil
> >> Please don't call it "margin".
> > OK.  It's now called "periphery".

> Not great, but at least it's not conflicting with existing
> terminology and I don't have a better suggestion, so: thanks.

:-)

> [ BTW, I see you used context diff instead of unified diff.  If that's
>   the form you prefer, that's perfectly OK (I can M-x
>   diff-context->unified just fine), but otherwise I prefer the unified
>   format]

OK.  I've just reconfigured my bzr to use unified.  It seems that unified
has "won" over context over the last decade.  Previously, context was the
Emacs project standard.  Unified is the only format accepted by hg, and I
wouldn't be surprised if git also only uses unified.

> > +(defun show-paren--categorize-paren (pos)
> > +  "Determine whether the character after POS has paren syntax,
> > +and if so, return a cons (DIR . OUTSIDE).  For an open paren, DIR
> > +is 1 and OUTSIDE is the position before the paren.  For a close
> > +paren, DIR is -1 and OUTSIDE is the position after the paren.  If
> > +the character isn't a paren, return nil."
> > +  (cond
> > +   ((eq (syntax-class (syntax-after pos)) 4)
> > +    (cons 1 pos))
> > +   ((eq (syntax-class (syntax-after pos)) 5)
> > +    (cons -1 (1+ pos)))
> > +   (t nil)))
> > +
> >  (defvar show-paren-data-function #'show-paren--default
> >    "Function to find the opener/closer at point and its match.
> >  The function is called with no argument and should return either nil
> > @@ -120,22 +141,50 @@
> >  Where HERE-BEG..HERE-END is expected to be around point.")
> >
> >  (defun show-paren--default ()
> > -  (let* ((oldpos (point))
> > -         (dir (cond ((eq (syntax-class (syntax-after (1- (point)))) 5) -1)
> > -                    ((eq (syntax-class (syntax-after (point)))      4) 1)))
> > -         (unescaped
> > -          (when dir
> > -            ;; Verify an even number of quoting characters precede the 
> > paren.
> > -            ;; Follow the same logic as in `blink-matching-open'.
> > -            (= (if (= dir -1) 1 0)
> > -               (logand 1 (- (point)
> > -                            (save-excursion
> > -                              (if (= dir -1) (forward-char -1))
> > -                              (skip-syntax-backward "/\\")
> > -                              (point)))))))
> > -         (here-beg (if (eq dir 1) (point) (1- (point))))
> > -         (here-end (if (eq dir 1) (1+ (point)) (point)))
> > -         pos mismatch)
> > +  (let* ((ind-pos (save-excursion (back-to-indentation) (point)))
> > +    (bol-pos (save-excursion (beginning-of-line) (point)))

> Aka line-beginning-position.

Thanks!  I didn't know about that.  There's line-end-position, too.

> > +    (eol-pos (save-excursion (end-of-line)
> > +                             (let ((s (syntax-ppss)))
> > +                               (if (nth 4 s)
> > +                                   (goto-char (max (nth 8 s)
> > +                                                   (point-min))))

> I don't think we need this `max' thingy.

I think we do, but it should be (line-beginning-position), not
(point-min).  But I see I need to beef up the logic to cope with a
closed /* */ style comment being on the line.

> > +                               (skip-chars-backward " \t"))
> > +                             (point)))
> > +    (oldpos (point))

> Please keep oldpos as the first entry in the let* (makes the patch
> slightly shorter and makes it more obvious that point hasn't moved yet).

Will do.

> Still looks complicated.  Can't we do something more like

>    (defun show-paren--move-in-periphery ()
>      (cond
>       ((save-excursion (skip-chars-backward " \t") (bolp))
>        (skip-chars-forward " \t"))
>       ((<= (line-end-position)
>            (save-excursion (forward-comment (point-max)) (point)))
>        (skip-chars-backward " \t"))))


>    (defun show-paren--default ()
>      (save-excursion
>        (when show-paren-when-point-in-periphery
>          (show-paren--move-in-periphery))
>        ..same-old-code-as-before..))

> This would make it clear that the behavior is unchanged when
> show-paren-when-point-in-periphery is nil, and also makes it clear what
> is the effect of setting show-paren-when-point-in-periphery to non-nil.

That's neat.  But it wouldn't be quite the same.  It would be a quite
restricted version of what I'm proposing: e.g. point in the LH periphery
wouldn't then trigger a paren at "end of core", nor even a close paren at
"beginning of core" (which happens a little in lisp, a lot in C).

Incidentally, I think `show-paren-highlight-openparen', which is a defvar
really ought to be a customisable variable.  Maybe
`show-paren-data-function' should be, too.  A little amendment of the
Emacs manual wrt paren matching also wouldn't go amiss.

Anyhow, here's the latest version of the patch, this time in unified
format.  It's not as big as it looks: about half of it is because I
removed a superfluous `save-excursion'.




=== modified file 'lisp/paren.el'
--- lisp/paren.el       2014-02-10 01:34:22 +0000
+++ lisp/paren.el       2014-10-14 22:14:55 +0000
@@ -72,6 +72,14 @@
   :group 'paren-showing
   :version "20.3")
 
+(defcustom show-paren-when-point-in-periphery nil
+  "If non-nil, show parens when point is in the line's periphery.
+The periphery is either the whitespace at the beginning of a
+line, or a comment \(or whitespace) at the end of a line."
+  :type 'boolean
+  :group 'paren-showing
+  :version "25.1")
+
 (define-obsolete-face-alias 'show-paren-match-face 'show-paren-match "22.1")
 
 (define-obsolete-face-alias 'show-paren-mismatch-face
@@ -112,6 +120,19 @@
     (delete-overlay show-paren--overlay)
     (delete-overlay show-paren--overlay-1)))
 
+(defun show-paren--categorize-paren (pos)
+  "Determine whether the character after POS has paren syntax,
+and if so, return a cons (DIR . OUTSIDE).  For an open paren, DIR
+is 1 and OUTSIDE is the position before the paren.  For a close
+paren, DIR is -1 and OUTSIDE is the position after the paren.  If
+the character isn't a paren, return nil."
+  (cond
+   ((eq (syntax-class (syntax-after pos)) 4)
+    (cons 1 pos))
+   ((eq (syntax-class (syntax-after pos)) 5)
+    (cons -1 (1+ pos)))
+   (t nil)))
+
 (defvar show-paren-data-function #'show-paren--default
   "Function to find the opener/closer at point and its match.
 The function is called with no argument and should return either nil
@@ -121,67 +142,97 @@
 
 (defun show-paren--default ()
   (let* ((oldpos (point))
-         (dir (cond ((eq (syntax-class (syntax-after (1- (point)))) 5) -1)
-                    ((eq (syntax-class (syntax-after (point)))      4) 1)))
-         (unescaped
-          (when dir
-            ;; Verify an even number of quoting characters precede the paren.
-            ;; Follow the same logic as in `blink-matching-open'.
-            (= (if (= dir -1) 1 0)
-               (logand 1 (- (point)
-                            (save-excursion
-                              (if (= dir -1) (forward-char -1))
-                              (skip-syntax-backward "/\\")
-                              (point)))))))
-         (here-beg (if (eq dir 1) (point) (1- (point))))
-         (here-end (if (eq dir 1) (1+ (point)) (point)))
-         pos mismatch)
+        (ind-pos (save-excursion (back-to-indentation) (point)))
+        (eol-pos
+         (save-excursion
+           (end-of-line)
+           (let ((s (syntax-ppss)))
+             (if (nth 4 s)
+                 (goto-char (max (nth 8 s)
+                                 (line-beginning-position)))))
+           (save-restriction
+             (narrow-to-region (line-beginning-position) (point-max))
+             (forward-comment (- (current-column))))
+           (point)))
+        dir paren-details unescaped pos mismatch here-beg here-end)
+    (cond
+     ;; Point is at a paren.
+     ((eq (syntax-class (syntax-after (1- (point)))) 5)
+      (setq dir -1))
+     ((eq (syntax-class (syntax-after (point))) 4)
+      (setq dir 1))
+     ;; Point is in the WS before the code.
+     ((and show-paren-when-point-in-periphery
+          (< (point) ind-pos))
+      (setq paren-details
+           (or (show-paren--categorize-paren ind-pos)
+               (show-paren--categorize-paren (1- eol-pos)))))
+     ;; Point is in a comment or whitespace after the code.
+     ((and show-paren-when-point-in-periphery
+          (>= (point) eol-pos))
+      (setq paren-details
+           (or (show-paren--categorize-paren (1- eol-pos))
+               (show-paren--categorize-paren ind-pos)))))
+    (when paren-details
+      (setq dir (car paren-details)
+           oldpos (cdr paren-details)))
+
+    (when dir
+      (setq unescaped
+           (= (if (= dir -1) 1 0)
+              (logand 1 (- oldpos
+                           (save-excursion
+                             (goto-char oldpos)
+                             (if (= dir -1) (backward-char))
+                             (skip-syntax-backward "/\\")
+                             (point)))))))
+    (setq here-beg (if (eq dir 1) oldpos (1- oldpos))
+          here-end (if (eq dir 1) (1+ oldpos) oldpos))
     ;;
     ;; Find the other end of the sexp.
     (when unescaped
-      (save-excursion
-        (save-restriction
-          ;; Determine the range within which to look for a match.
-          (when blink-matching-paren-distance
-            (narrow-to-region
-             (max (point-min) (- (point) blink-matching-paren-distance))
-             (min (point-max) (+ (point) blink-matching-paren-distance))))
-          ;; Scan across one sexp within that range.
-          ;; Errors or nil mean there is a mismatch.
-          (condition-case ()
-              (setq pos (scan-sexps (point) dir))
-            (error (setq pos t mismatch t)))
-          ;; Move back the other way and verify we get back to the
-          ;; starting point.  If not, these two parens don't really match.
-          ;; Maybe the one at point is escaped and doesn't really count,
-          ;; or one is inside a comment.
-          (when (integerp pos)
-            (unless (condition-case ()
-                        (eq (point) (scan-sexps pos (- dir)))
-                      (error nil))
-              (setq pos nil)))
-          ;; If found a "matching" paren, see if it is the right
-          ;; kind of paren to match the one we started at.
-          (if (not (integerp pos))
-              (if mismatch (list here-beg here-end nil nil t))
-            (let ((beg (min pos oldpos)) (end (max pos oldpos)))
-              (unless (eq (syntax-class (syntax-after beg)) 8)
-                (setq mismatch
-                      (not (or (eq (char-before end)
-                                   ;; This can give nil.
-                                   (cdr (syntax-after beg)))
-                               (eq (char-after beg)
-                                   ;; This can give nil.
-                                   (cdr (syntax-after (1- end))))
-                               ;; The cdr might hold a new paren-class
-                               ;; info rather than a matching-char info,
-                               ;; in which case the two CDRs should match.
-                               (eq (cdr (syntax-after (1- end)))
-                                   (cdr (syntax-after beg)))))))
-              (list here-beg here-end
-                    (if (= dir 1) (1- pos) pos)
-                    (if (= dir 1) pos (1+ pos))
-                    mismatch))))))))
+      (save-restriction
+       ;; Determine the range within which to look for a match.
+       (when blink-matching-paren-distance
+         (narrow-to-region
+          (max (point-min) (- (point) blink-matching-paren-distance))
+          (min (point-max) (+ (point) blink-matching-paren-distance))))
+       ;; Scan across one sexp within that range.
+       ;; Errors or nil mean there is a mismatch.
+       (condition-case ()
+           (setq pos (scan-sexps oldpos dir))
+         (error (setq pos t mismatch t)))
+       ;; Move back the other way and verify we get back to the
+       ;; starting point.  If not, these two parens don't really match.
+       ;; Maybe the one at point is escaped and doesn't really count,
+       ;; or one is inside a comment.
+       (when (integerp pos)
+         (unless (condition-case ()
+                     (eq oldpos (scan-sexps pos (- dir)))
+                   (error nil))
+           (setq pos nil)))
+       ;; If found a "matching" paren, see if it is the right
+       ;; kind of paren to match the one we started at.
+       (if (not (integerp pos))
+           (if mismatch (list here-beg here-end nil nil t))
+         (let ((beg (min pos oldpos)) (end (max pos oldpos)))
+           (unless (eq (syntax-class (syntax-after beg)) 8)
+             (setq mismatch
+                   (not (or (eq (char-before end)
+                                ;; This can give nil.
+                                (cdr (syntax-after beg)))
+                            (eq (char-after beg)
+                                ;; This can give nil.
+                                (cdr (syntax-after (1- end))))
+                            ;; The cdr might hold a new paren-class
+                            ;; info rather than a matching-char info,
+                            ;; in which case the two CDRs should match.
+                            (eq (cdr (syntax-after (1- end)))
+                                (cdr (syntax-after beg)))))))
+           (list here-beg here-end
+                 (if (= dir 1) (1- pos) pos)
+                 (if (= dir 1) pos (1+ pos))
+                 mismatch)))))))
 
 ;; Find the place to show, if there is one,
 ;; and show it until input arrives.
@@ -215,7 +266,7 @@
         ;; Otherwise, turn off any such highlighting.
         (if (or (not here-beg)
                 (and (not show-paren-highlight-openparen)
-                     (> here-end (point))
+                    (= here-beg (point))
                      (integerp there-beg)))
             (delete-overlay show-paren--overlay-1)
           (move-overlay show-paren--overlay-1
@@ -234,7 +285,7 @@
                                           (1- there-end) (1+ there-beg))))
                          (not (pos-visible-in-window-p closest)))))
               (move-overlay show-paren--overlay
-                            (point)
+                           (if (< there-beg here-beg) here-end here-beg)
                             (if (< there-beg here-beg) there-beg there-end)
                             (current-buffer))
             (move-overlay show-paren--overlay


>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).