From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Leo Liu <sdl.web@gmail.com>
Cc: 14395@debbugs.gnu.org
Subject: bug#14395: 24.3; [PATCH] new feature smie-highlight-matching-block
Date: Wed, 15 May 2013 22:31:47 -0400 [thread overview]
Message-ID: <jwvsj1nk6qc.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <m1vc6kvhoy.fsf@gmail.com> (Leo Liu's message of "Wed, 15 May 2013 15:13:49 +0800")
> Thank you for your comments, Stefan. I have taken these into account and
> new patch attached.
See below.
> One thing in the patch that I dislike is having to forward-declare
> smie-highlight-matching-block-mode. Do you have a cleaner way?
Move the define-minor-mode before the first use of the variable?
> +(defvar smie-highlight-matching-block-mode nil) ; Silence compiler warning
A forward declaration takes the form (defvar foo), not (defvar foo nil).
> (defun smie-blink-matching-open ()
> "Blink the matching opener when applicable.
> This uses SMIE's tables and is expected to be placed on `post-self-insert-hook'."
> (let ((pos (point)) ;Position after the close token.
> token)
> (when (and blink-matching-paren
> + (not smie-highlight-matching-block-mode)
I suggest you instead remove smie-blink-matching-open from
post-self-insert-hook (and refrain from adding it back) when enabling
smie-highlight-matching-block-mode.
> +(defvar smie-highlight-matching-block-timer nil)
> +(defvar-local smie-highlight-matching-block-overlay nil)
> +(defvar-local smie-highlight-matching-block-lastpos -1)
Please use the "smie--" prefix for such internal variables.
> +(define-minor-mode smie-highlight-matching-block-mode nil
Please provide a docstring.
> + :global t :group 'smie
Don't bother with the ":group 'smie", it'll be added automatically anyway.
> + (if smie-highlight-matching-block-mode
> + (setq smie-highlight-matching-block-timer
> + (run-with-idle-timer 0.2 t #'smie-highlight-matching-block))
> + (when (timerp smie-highlight-matching-block-timer)
> + (cancel-timer smie-highlight-matching-block-timer))))
(progn
(smie-highlight-matching-block-mode 1)
(smie-highlight-matching-block-mode 1))
Will start 2 timers (and lose the reference to the first one, making
it harder to cancel). Better structure minor mode bodies as "first
unconditionally turn it off, then turn it on is needed" to avoid
such problems.
Similarly
(progn
(smie-highlight-matching-block-mode -1)
(smie-highlight-matching-block-mode -1))
will try to cancel an already canceled timer, so better reset the timer
var to nil after canceling.
Stefan
next prev parent reply other threads:[~2013-05-16 2:31 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-14 2:49 bug#14395: 24.3; [PATCH] new feature smie-highlight-matching-block Leo Liu
2013-05-14 16:02 ` Stefan Monnier
2013-05-15 7:13 ` Leo Liu
2013-05-16 2:31 ` Stefan Monnier [this message]
2013-05-16 3:27 ` Leo Liu
2013-05-16 13:24 ` Stefan Monnier
2013-05-16 16:06 ` Leo Liu
2013-05-16 17:51 ` Stefan Monnier
2013-05-16 23:09 ` Leo Liu
2013-05-17 0:40 ` Stefan Monnier
2013-05-17 1:30 ` Leo Liu
2013-05-22 18:50 ` Stefan Monnier
2013-05-16 4:45 ` Glenn Morris
2013-05-16 5:33 ` Leo Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvsj1nk6qc.fsf-monnier+emacs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=14395@debbugs.gnu.org \
--cc=sdl.web@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).