bug#14430: [PATCH] Desktop restore runs mark activation hooks when it shouldn't

bug#14430: [PATCH] Desktop restore runs mark activation hooks when it shouldn't

[Kelly Dean]

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

Delete your .emacs.desktop, and put in init.el:

(desktop-save-mode 1)
(add-hook 'deactivate-mark-hook (lambda () (setq cursor-type t)))
(add-hook 'activate-mark-hook (lambda () (setq cursor-type 'bar)))

Start Emacs 24.3, open a file, press C-SPC C-g, then exit Emacs and save the desktop, then restart Emacs. Notice that the cursor type is now a bar, not a block. It should be a block.

The attached patch fixes it. It could be fixed without patching set-mark, by conditionally calling deactivate-mark in desktop-create-buffer after calling set-mark, but that's a hack; the mark shouldn't be activated in the first place. Besides that, I need a dont-activate option for set-mark in some of my other code, so this bug gives me an excuse to add it.

The attached patch relies on the fix for bug 13027, which was reportedly applied last November, but didn't make it into 24.3, so either apply that to 24.3, or use the current development version, but I haven't tried the latter.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: restoremarkbug.patch --]
[-- Type: text/x-diff; name="restoremarkbug.patch", Size: 1772 bytes --]

--- emacs-24.3/lisp/simple.el
+++ emacs-24.3/lisp/simple.el
@@ -4018,12 +4018,14 @@
     (unless transient-mark-mode
       (setq transient-mark-mode 'lambda))))
 
-(defun set-mark (pos)
+(defun set-mark (pos &optional dont-activate)
   "Set this buffer's mark to POS.  Don't use this function!
 That is to say, don't use this function unless you want
 the user to see that the mark has moved, and you want the previous
 mark position to be lost.
 
+Activate the mark unless optional DONT-ACTIVATE is non-nil.
+
 Normally, when a new mark is set, the old one should go on the stack.
 This is why most applications should use `push-mark', not `set-mark'.
 
@@ -4037,9 +4039,8 @@
 
   (if pos
       (progn
-	(setq mark-active t)
-	(run-hooks 'activate-mark-hook)
-	(set-marker (mark-marker) pos (current-buffer)))
+	(set-marker (mark-marker) pos (current-buffer))
+	(unless dont-activate (activate-mark)))
     ;; Normally we never clear mark-active except in Transient Mark mode.
     ;; But when we actually clear out the mark value too, we must
     ;; clear mark-active in any mode.
--- emacs-24.3/lisp/desktop.el
+++ emacs-24.3/lisp/desktop.el
@@ -1218,10 +1218,9 @@
 	       (error (message "%s" (error-message-string err)) 1))))
 	  (when desktop-buffer-mark
 	    (if (consp desktop-buffer-mark)
-		(progn
-		  (set-mark (car desktop-buffer-mark))
-		  (setq mark-active (car (cdr desktop-buffer-mark))))
-	      (set-mark desktop-buffer-mark)))
+		(set-mark (car desktop-buffer-mark)
+			  (not (car (cdr desktop-buffer-mark))))
+	      (set-mark desktop-buffer-mark t)))
 	  ;; Never override file system if the file really is read-only marked.
 	  (when desktop-buffer-read-only (setq buffer-read-only desktop-buffer-read-only))
 	  (while desktop-buffer-locals

bug#14430: [PATCH] Desktop restore runs mark activation hooks when it shouldn't

[Stefan Monnier]

> (desktop-save-mode 1)
> (add-hook 'deactivate-mark-hook (lambda () (setq cursor-type t)))
> (add-hook 'activate-mark-hook (lambda () (setq cursor-type 'bar)))

> Start Emacs 24.3, open a file, press C-SPC C-g, then exit Emacs and save the
> desktop, then restart Emacs. Notice that the cursor type is now a bar,
> not a block. It should be a block.

I installed a patch which makes desktop not run (de)activate-mark hooks.

> The attached patch fixes it. It could be fixed without patching set-mark, by
> conditionally calling deactivate-mark in desktop-create-buffer after calling
> set-mark, but that's a hack; the mark shouldn't be activated in the first

Instead, I simply use move-marker on mark-marker.


        Stefan


=== modified file 'lisp/desktop.el'
--- lisp/desktop.el	2014-02-22 02:10:49 +0000
+++ lisp/desktop.el	2014-03-10 02:17:20 +0000
@@ -1382,18 +1382,19 @@
 	  (when desktop-buffer-mark
 	    (if (consp desktop-buffer-mark)
 		(progn
-		  (set-mark (car desktop-buffer-mark))
+                  (move-marker (mark-marker) (car desktop-buffer-mark))
+                  ;; FIXME: Should we call (de)activate-mark instead?
 		  (setq mark-active (car (cdr desktop-buffer-mark))))
-	      (set-mark desktop-buffer-mark)))
+              (move-marker (mark-marker) desktop-buffer-mark)))
 	  ;; Never override file system if the file really is read-only marked.
 	  (when desktop-buffer-read-only (setq buffer-read-only desktop-buffer-read-only))
 	  (dolist (this desktop-buffer-locals)
 	    (if (consp this)
-		;; an entry of this form `(symbol . value)'
+		;; An entry of this form `(symbol . value)'.
 		(progn
 		  (make-local-variable (car this))
 		  (set (car this) (cdr this)))
-	      ;; an entry of the form `symbol'
+	      ;; An entry of the form `symbol'.
 	      (make-local-variable this)
 	      (makunbound this))))))))