Re: [PATCH] Proposal to change cursor appearance to indicate region activation

[Kelly Dean <kelly@prtime.org>]

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

Stefan Monnier wrote:
> I'd rather see a `define-minor-mode' here, especially since you already
> chose a "...-mode" name.

Ok, the attached dynamic-cursor-mode-gross.patch does that.

Can't just add ⌜:global t⌝ to dynamic-cursor-mode, since the mode needs to be en/dis-ableable buffer-locally. That's why a pair of minor modes is necessary.

But I recommend against implementing the feature that way, since it's gross.

shift-select-mode is a defcustom, not an actual minor mode, so that's precedence. I guess it's bad precedence.

But assuming things with a ⌜-mode⌝ suffix in their names are supposed to be actual modes, would it be ok to just drop the suffix?
The attached dynamic-cursor-mode-1.patch does that.

Maybe add a ⌜use-⌝ prefix, like for use-empty-active-region (which is a defcustom)?

Or ⌜enable-⌝ prefix, like for enable-dir-local-variables, enable-recursive-minibuffers, and enable-multibyte-characters (which are all variables)?

> But if the user has set cursor-type in his .emacs, we'll now overwrite
> his choice.  So we can't enable this code by default.  Two options:
> - keep the default as nil.
> - change the code to only modify the cursor-type if it hasn't been changed.

Oops. Fixed in both of the attached patches.

Or did you mean _ever_ changed, rather than just at startup? In that case, what happens if the user first tries ⌜(setq-default cursor-type 'bar)⌝ while looking for nice features in Emacs, then later decides to try the dynamic cursor? It won't work, and that failure might seem like a bug, because by manually turning on the feature, he surely expects it to control the cursor, even though he set it to something nonstandard before.

It seems the right way would be for set-default and setq-default on cursor-type to automatically trigger a call of (setq-default dynamic-cursor nil), but I'm not aware of any feature in Emacs to trigger functions when variables are set. This is the same problem as for bug #19068, where there's no way for writes to the message-directory variable to automatically trigger updates to its dependent variables.

So I guess just keep the default as nil. :-(


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dynamic-cursor-mode-gross.patch --]
[-- Type: text/x-diff, Size: 1830 bytes --]

Third hunk adjusted to avoid conflict with current Emacs trunk.
--- emacs-24.4/lisp/simple.el
+++ emacs-24.4/lisp/simple.el
@@ -4391,6 +4391,31 @@
 (declare-function x-selection-exists-p "xselect.c"
                   (&optional selection terminal))
 
+(define-minor-mode dynamic-cursor-mode
+  "Toggle Dynamic Cursor mode.
+With a prefix argument ARG, enable Dynamic Cursor mode if ARG is
+positive, and disable it otherwise.  If called from Lisp, enable
+Dynamic Cursor mode if ARG is omitted or nil.
+
+Dynamic Cursor mode is a buffer-local minor mode.  When enabled,
+`cursor-type' is set dynamically to reflect `mark-active'.")
+
+(define-globalized-minor-mode global-dynamic-cursor-mode
+  dynamic-cursor-mode dynamic-cursor-mode)
+
+;; Enabling before init lets user disable in init file
+(global-dynamic-cursor-mode)
+
+;; Auto-disable to avoid conflict if cursor-type set in init file.
+;; Use emacs-startup-hook instead of after-init-hook in case
+;; cursor-type set in some file loaded as command line option.
+(defun maybe-disable--dynamic-cursor ()
+  (unless (eq (default-value 'cursor-type)
+	      (eval (car (get 'cursor-type 'standard-value))))
+    (global-dynamic-cursor-mode 0)))
+
+(add-hook 'emacs-startup-hook #'maybe-disable--dynamic-cursor)
+
 (defun deactivate-mark (&optional force)
   "Deactivate the mark.
 If Transient Mark mode is disabled, this function normally does
@@ -4430,6 +4455,7 @@
      ((eq transient-mark-mode 'lambda)
       (setq transient-mark-mode nil)))
     (setq mark-active nil)
+    (if dynamic-cursor-mode (setq cursor-type t))
     (run-hooks 'deactivate-mark-hook)
     (redisplay--update-region-highlight (selected-window))))
 
@@ -4445,3 +4471,4 @@
+      (if dynamic-cursor-mode (setq cursor-type 'bar))
       (run-hooks 'activate-mark-hook))))
 
 (defun set-mark (pos)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: dynamic-cursor-mode-1.patch --]
[-- Type: text/x-diff, Size: 1411 bytes --]

Third hunk adjusted to avoid conflict with current Emacs trunk.
--- emacs-24.4/lisp/simple.el
+++ emacs-24.4/lisp/simple.el
@@ -4391,6 +4391,22 @@
 (declare-function x-selection-exists-p "xselect.c"
                   (&optional selection terminal))
 
+(defcustom dynamic-cursor t
+  "If non-nil, `cursor-type' is set dynamically to reflect `mark-active'."
+  :type 'boolean
+  :version "25.1"
+  :group 'editing-basics)
+
+;; Auto-disable to avoid conflict if cursor-type set in init file.
+;; Use emacs-startup-hook instead of after-init-hook in case
+;; cursor-type set in some file loaded as command line option.
+(defun maybe-disable--dynamic-cursor ()
+  (unless (eq (default-value 'cursor-type)
+	      (eval (car (get 'cursor-type 'standard-value))))
+    (setq dynamic-cursor nil)))
+
+(add-hook 'emacs-startup-hook #'maybe-disable--dynamic-cursor)
+
 (defun deactivate-mark (&optional force)
   "Deactivate the mark.
 If Transient Mark mode is disabled, this function normally does
@@ -4430,6 +4446,7 @@
      ((eq transient-mark-mode 'lambda)
       (setq transient-mark-mode nil)))
     (setq mark-active nil)
+    (if dynamic-cursor (setq cursor-type t))
     (run-hooks 'deactivate-mark-hook)
     (redisplay--update-region-highlight (selected-window))))
 
@@ -4445,3 +4462,4 @@
+      (if dynamic-cursor (setq cursor-type 'bar))
       (run-hooks 'activate-mark-hook))))
 
 (defun set-mark (pos)