unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Kelly Dean <kelly@prtime.org>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: emacs-devel@gnu.org
Subject: Re: [PATCH] Run hook when variable is set
Date: Sat, 31 Jan 2015 09:18:03 +0000	[thread overview]
Message-ID: <NtaDoxOFEwXbdU4e8joNhILkxz4BefVum2rbC0aeWz1@local> (raw)
In-Reply-To: <jwvd25wl111.fsf-monnier+emacs@gnu.org>

Stefan Monnier wrote:
> you can drop the let-with-hook

Without it, code which would be simply:
(let-with-hook ((hooked-var foo))
	       (body))

would have to become:
(setq tmp hooked-var)
(setq-with-hook hooked-var foo)
(body)
(setq-with-hook hooked-var tmp)

That's cumbersome.

> since it would be a misfeature anyway,

If it's missing (and you don't manually do it as above), then the hooked variable and the dependent variable become unsynchronized when the former is let-bound. That's bound to introduce bugs.

> and then
> consolidate the remaining 4 into a single function.

How do you combine setq-with-hook, set-with-hook, setq-default-with-hook, set-default-with-hook, and setq-local-with-hook into a single function?

However you do it, if it's really a reasonable thing to do, then why aren't setq, set, setq-default, set-default, and setq-local also combined into a single function in the same way?

Anyway, since you said varhook, or something like it, would be useful for debugging even though you don't want to use it to implement regular features, is my implementation reasonably close to what you had in mind?


WRT DCM, in deactivate-mark, you put:
(when dynamic-cursor-mode
         (if (and dynamic-cursor-mode--set (eq cursor-type 'bar))
             (setq cursor-type t))
         (setq dynamic-cursor-mode--set nil))

and in activate-mark:
(if (and dynamic-cursor-mode (eq cursor-type t))
           (setq cursor-type 'bar dynamic-cursor-mode--set t))

That would fail in this case:
(setq-default cursor-type 'bar)
Later in the same Emacs session...
(dynamic-cursor-mode)

The user would reasonably think that DCM's failure to work in that case is a bug.

To solve that, you wrote, ⌜dynamic-cursor-mode can do (setq-default cursor-type t).⌝ IIUC, you meant:
(define-minor-mode dynamic-cursor-mode
  "docs..." :global t :init-value t
  (if dynamic-cursor-mode
      (setq-default cursor-type t)))

In that case, how do you turn it on buffer-locally? If cursor-type isn't set to t, and you do:
(setq-local dynamic-cursor-mode t)

then it won't work. If you instead do:
(make-local-variable 'dynamic-cursor-mode)
(dynamic-cursor-mode)

then DCM is enabled only locally, which is the correct thing to do, but cursor-type is globally set to t, which is a bug. Turning on DCM locally must not affect CT globally.

I guess you could fix that bug with:
(define-minor-mode dynamic-cursor-mode
  "docs..." :global t :init-value t
  (if dynamic-cursor-mode
      (if (assq 'dynamic-cursor-mode (buffer-local-variables))
	  (setq cursor-type t)
	(setq-default cursor-type t))))

> The code I provided disables the effect of dynamic-cursor-mode as soon
> as the value of cursor-type is not the expected one (without
> needing to disable dynamic-cursor-mode).

This has the edge case that if the mark happens to be active when you set cursor-type to 'bar, then the setting doesn't stick; DCM doesn't realize that CT was set outside of DCM's control, so DCM overrides the setting. Similarly, if you previously always used a non-standard cursor type, and were therefore mislead into thinking DCM was disabled, then if you set cursor-type to t, DCM unexpectedly starts operating. That isn't user-friendly.

Another poor result of your design is that if you set a non-standard cursor type in your init file, then start Emacs and do C-h m, DCM is listed as enabled, even though it's having no effect, which is a great way to confuse the user, or convince him that there's a bug.

Using varhook for DCM avoids those problems.



  reply	other threads:[~2015-01-31  9:18 UTC|newest]

Thread overview: 110+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-20  2:54 Proposal to change cursor appearance to indicate region activation Kelly Dean
2013-04-20  7:23 ` Drew Adams
2015-01-22  5:38   ` [PATCH] " Kelly Dean
2015-01-22 14:25     ` Stefan Monnier
2015-01-23  3:08       ` [PATCH] " Kelly Dean
2015-01-23  4:55         ` Stefan Monnier
2015-01-23 11:07           ` Kelly Dean
2015-01-23 17:49             ` Drew Adams
2015-01-24  3:06               ` Kelly Dean
2015-01-24  4:52                 ` Stefan Monnier
2015-01-24  9:22                   ` Kelly Dean
2015-01-25 14:29                     ` Stefan Monnier
2015-01-28  9:15                       ` [PATCH] Run hook when variable is set Kelly Dean
2015-01-28  9:23                         ` [PATCH] Proposal to change cursor appearance to indicate region activation Kelly Dean
2015-01-28 11:24                           ` David Kastrup
2015-01-28 12:13                             ` David Kastrup
2015-01-29 10:46                             ` Kelly Dean
2015-01-29 11:16                               ` David Kastrup
2015-01-30  7:20                                 ` Kelly Dean
2015-01-30  9:19                                   ` David Kastrup
2015-01-30 10:05                                     ` Kelly Dean
2015-01-30 10:12                                       ` David Kastrup
2015-01-30  9:43                                   ` Kelly Dean
2015-01-28 19:25                         ` [PATCH] Run hook when variable is set Stefan Monnier
2015-01-29  8:20                           ` Kelly Dean
2015-01-29  8:28                             ` Lars Ingebrigtsen
2015-01-29 14:58                             ` Stefan Monnier
2015-01-30  7:34                               ` Kelly Dean
2015-01-30 15:55                                 ` Stefan Monnier
2015-01-31  9:18                                   ` Kelly Dean [this message]
2015-01-31 20:48                                     ` Stefan Monnier
2015-02-02  5:40                                       ` Kelly Dean
2015-02-02 15:57                                         ` Stefan Monnier
2015-02-03 19:56                                           ` Kelly Dean
2015-02-03 22:49                                             ` Stefan Monnier
2015-02-05  3:10                                               ` [PATCH] (Updated) " Kelly Dean
2015-02-05 13:57                                                 ` Stefan Monnier
2015-02-06  5:34                                                   ` Kelly Dean
2015-02-06 14:42                                                     ` Stefan Monnier
2015-02-07 12:27                                                       ` Kelly Dean
2015-02-07 15:09                                                         ` Stefan Monnier
2015-02-09  3:24                                                           ` Kelly Dean
2015-02-12 19:58                                                             ` Stefan Monnier
2015-02-13 23:08                                                               ` Kelly Dean
2015-02-14  0:55                                                                 ` Stefan Monnier
2015-02-14 22:19                                                                   ` Kelly Dean
2015-02-15 20:25                                                                     ` Stefan Monnier
2015-02-17  2:22                                                                       ` Kelly Dean
2015-02-17 23:07                                                                         ` Richard Stallman
2015-02-18  3:19                                                                           ` The purpose of makunbound (Was: Run hook when variable is set) Kelly Dean
2015-02-18  5:48                                                                             ` The purpose of makunbound Stefan Monnier
2015-02-18  8:51                                                                               ` Kelly Dean
2015-02-18 14:34                                                                                 ` Stefan Monnier
2015-02-18 18:53                                                                                   ` Kelly Dean
2015-02-18 22:42                                                                                     ` Stefan Monnier
2015-02-19 10:36                                                                                       ` Kelly Dean
2015-02-22  0:18                                                                                   ` Kelly Dean
2015-02-19 10:45                                                                           ` Kelly Dean
2015-02-19 13:33                                                                             ` Stefan Monnier
2015-02-19 23:51                                                                               ` Kelly Dean
2015-02-20  1:59                                                                                 ` Stefan Monnier
2015-02-20  9:35                                                                                   ` Kelly Dean
2015-02-20 16:55                                                                                     ` Stefan Monnier
2015-02-20  2:58                                                                                 ` Stephen J. Turnbull
2015-02-20  0:56                                                                             ` Richard Stallman
2015-02-20  9:02                                                                               ` Kelly Dean
2015-02-20 15:41                                                                                 ` Richard Stallman
2015-02-21  5:45                                                                                   ` Stephen J. Turnbull
2015-02-22  0:32                                                                                     ` Kelly Dean
2015-02-22  8:45                                                                                       ` Andreas Schwab
2015-02-18  5:15                                                                         ` [PATCH] (Updated) Run hook when variable is set Kelly Dean
2015-02-18 22:37                                                                           ` Stefan Monnier
2015-02-18 22:37                                                                         ` Stefan Monnier
2015-02-19 10:35                                                                           ` Kelly Dean
2015-02-19 13:30                                                                             ` Stefan Monnier
2015-02-20  6:48                                                                               ` Kelly Dean
2015-02-20 19:29                                                                                 ` Stefan Monnier
2015-02-21 14:18                                                                                   ` Kelly Dean
2015-02-21 20:51                                                                                     ` Stefan Monnier
2015-02-22  0:32                                                                                       ` Kelly Dean
2015-02-22 10:40                                                                                         ` Stephen J. Turnbull
2015-02-22 21:35                                                                                         ` Stefan Monnier
2015-02-23  3:09                                                                                           ` Kelly Dean
2015-02-23  4:19                                                                                             ` Stefan Monnier
2015-02-20 20:27                                                                               ` Proposal for debugging/testing option Kelly Dean
2015-02-24 16:28                                                                                 ` Stefan Monnier
2015-02-14 20:37                                                               ` [PATCH] (Updated) Run hook when variable is set Johan Bockgård
2015-02-15 19:36                                                                 ` Stefan Monnier
2015-02-15 19:53                                                                   ` Patches: inline vs. attachment, compressed vs. uncompressed. [was: Run hook when variable is set] Alan Mackenzie
2015-02-06  9:55                                                   ` [PATCH] (Updated) Run hook when variable is set Kelly Dean
2015-01-30 23:29                                 ` [PATCH] " Richard Stallman
2015-01-31  9:23                                   ` Kelly Dean
2015-01-31 23:16                                     ` Richard Stallman
2015-02-02  5:41                                       ` Kelly Dean
2015-02-01  2:04                               ` Alexis
2015-02-01  4:05                                 ` Stefan Monnier
2015-02-01  8:58                                   ` David Kastrup
2015-01-29 16:06                             ` Eli Zaretskii
2015-01-30  7:14                               ` Kelly Dean
2015-01-30  9:08                                 ` Eli Zaretskii
2015-01-23 20:34             ` [PATCH] Proposal to change cursor appearance to indicate region activation Stefan Monnier
2015-01-24  0:25               ` Kelly Dean
2015-01-23 10:01         ` Tassilo Horn
2015-01-23 17:49           ` Drew Adams
2015-01-23 10:06         ` Eli Zaretskii
2015-01-23 11:40           ` Kelly Dean
2015-01-23 11:56             ` Eli Zaretskii
2015-01-22  5:41   ` Kelly Dean
2013-11-23 13:34 ` Stefan Monnier
2013-11-23 20:25   ` Drew Adams

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=NtaDoxOFEwXbdU4e8joNhILkxz4BefVum2rbC0aeWz1@local \
    --to=kelly@prtime.org \
    --cc=emacs-devel@gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /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).