emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Proposed extension of show-paren-mode: Highlight parens when point i


From: Stefan Monnier
Subject: Re: Proposed extension of show-paren-mode: Highlight parens when point is in L or R margin.
Date: Tue, 14 Oct 2014 13:49:21 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4.50 (gnu/linux)

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

>> > + (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.

>> This is a code duplication.  Please move it to a separate helper function.
> No it's not.  It appears just once, after my patch is applied.

Sorry, I guess I misread it.
[ 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]

> +(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.

> +      (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.

> +                                 (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).

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.


        Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]