GNU bug report logs - #14225
[PATCH] Winner mode fails to run mark activation and deactivation hooks

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 14225 in the body.
You can then email your comments to 14225 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Kelly Dean <kellydeanch <at> yahoo.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Winner mode fails to run mark activation and deactivation
	hooks
Date: Wed, 17 Apr 2013 20:17:33 -0700 (PDT)

[Message part 1 (text/plain, inline)]

Using Emacs 24.3
emacs -Q
(winner-mode 1)
(add-hook 'deactivate-mark-hook (lambda () (setq cursor-type t)))
(add-hook 'activate-mark-hook (lambda () (setq cursor-type 'bar)))
C-x 2
C-c left

Notice that the cursor type is now a bar, not a block. It should be a block.
The attached winnerbug.patch fixes it.

BTW, by default in Emacs, when the region is active but empty, there's no visual indication of this status. Neither is there indication of the active region if point is one less than mark and blink-cursor-mode is off. Also, if point is greater than mark, the active region is highlighted, but the block cursor does an inverse-video highlight of the character following the region, which (speaking from experience) an Emacs newbie finds distracting, since it seems to indicate that that character is also part of the region. Setting the cursor type to bar when the region is active solves all those problems, and I recommend it as the default for Emacs.

[winnerbug.patch (text/x-diff, attachment)]

Message #8 received at 14225 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Kelly Dean <kellydeanch <at> yahoo.com>
Cc: 14225 <at> debbugs.gnu.org
Subject: Re: bug#14225: [PATCH] Winner mode fails to run mark activation and
	deactivation hooks
Date: Thu, 18 Apr 2013 09:15:11 -0400

> Notice that the cursor type is now a bar, not a block. It should be a block.
> The attached winnerbug.patch fixes it.

Thanks.  I installed a slightly different patch which uses higher-level
functions instead.

> BTW, by default in Emacs, when the region is active but empty, there's no
> visual indication of this status. Neither is there indication of the active
> region if point is one less than mark and blink-cursor-mode is off. Also, if
> point is greater than mark, the active region is highlighted, but the block
> cursor does an inverse-video highlight of the character following the
> region, which (speaking from experience) an Emacs newbie finds distracting,
> since it seems to indicate that that character is also part of the
> region. Setting the cursor type to bar when the region is active solves all
> those problems, and I recommend it as the default for Emacs.

I'd never thought about it, but it does sound like a good idea.
This said, I see two problems with it:
- we'll probably bump into more bugs along the lines of this one
  (should all be fixable, but).
- every once will be exposed to this change, and someone out there is
  bound to dislike it.
So could you send a separate email about it to emacs-devel?  No need to
participate in the ensuing flamewar, but it's important to let people
use their flamethrowers to "let it out" ;-)


        Stefan


=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2013-04-18 06:24:10 +0000
+++ lisp/ChangeLog	2013-04-18 13:08:17 +0000
@@ -1,3 +1,10 @@
+2013-04-18  Stefan Monnier  <monnier <at> iro.umontreal.ca>
+
+	* winner.el (winner-active-region): Use region-active-p, activate-mark
+	and deactivate-mark (bug#14225).
+
+	* simple.el (deactivate-mark): Don't inline it.
+
 2013-04-18  Tassilo Horn  <tsdh <at> gnu.org>
 
 	* files.el (auto-mode-alist): Delete OpenDocument and StarOffice

=== modified file 'lisp/simple.el'
--- lisp/simple.el	2013-04-18 00:12:33 +0000
+++ lisp/simple.el	2013-04-18 13:05:44 +0000
@@ -4177,7 +4177,7 @@
       (marker-position (mark-marker))
     (signal 'mark-inactive nil)))
 
-(defsubst deactivate-mark (&optional force)
+(defun deactivate-mark (&optional force)
   "Deactivate the mark.
 If Transient Mark mode is disabled, this function normally does
 nothing; but if FORCE is non-nil, it deactivates the mark anyway.

=== modified file 'lisp/winner.el'
--- lisp/winner.el	2013-01-01 09:11:05 +0000
+++ lisp/winner.el	2013-04-18 13:04:51 +0000
@@ -45,10 +45,8 @@
                         (if (featurep 'xemacs)
                             `(if ,store (zmacs-activate-region)
                                (zmacs-deactivate-region))
-                          `(setq mark-active ,store)))))
-  (if (boundp 'mark-active)
-      mark-active
-    (region-active-p)))
+                          `(if ,store (activate-mark) (deactivate-mark))))))
+  (region-active-p))
 
 (defalias 'winner-edges
   (if (featurep 'xemacs) 'window-pixel-edges 'window-edges))

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.