Hrvoje Niksic <hniksic(a)xemacs.org> writes:
"Ben Wing" <ben(a)666.com> writes:
> I don't think this is the proper solution. Either we should make
> the zmacs-region be per-buffer or we should change region-active-p
> to only return t when the zmacs region is in the current buffer. We
> don't want to have to special-case every use of region-active-p.
I agree with the latter solution, i.e. moving the buffer check to
region-active-p. I don't think that would break anything. Existing
code that thought region was active when it was located in another
buffer was broken anyway.
Hi Hrvoje,
makes sense to me after some checking.
I found the idiom
(when (and (region-active-p)
(eq (current-buffer) (zmacs-region-buffer)))
used in cmdloop.el and minibuf.el so I didn't question it at first and
just created the obvious patch.
cmdloop.el:
-----------
1.1 (steve 08-Nov-97): ;; moved here from pending-del.
1.1 (steve 08-Nov-97): (defun keyboard-quit ()
1.1 (steve 08-Nov-97): "Signal a `quit' condition.
1.1 (steve 08-Nov-97): If this character is typed while lisp code is
executing, it will be treated
1.1 (steve 08-Nov-97): as an interrupt.
1.1 (steve 08-Nov-97): If this character is typed at top-level, this simply
beeps.
1.1 (steve 08-Nov-97): If `zmacs-regions' is true, and the zmacs region is
active in this buffer,
1.1 (steve 08-Nov-97): then this key deactivates the region without beeping or
signalling."
1.1 (steve 08-Nov-97): (interactive)
1.1 (steve 08-Nov-97): (if (and (region-active-p)
1.1 (steve 08-Nov-97): (eq (current-buffer) (zmacs-region-buffer)))
1.1 (steve 08-Nov-97): ;; pseudo-zmacs compatibility: don't beep if
this ^G is simply
1.1 (steve 08-Nov-97): ;; deactivating the region. If it is inactive,
beep.
1.1 (steve 08-Nov-97): nil
1.1 (steve 08-Nov-97): (signal 'quit nil)))
minibuf.el:
-----------
1.1 (steve 08-Nov-97): (defun minibuffer-keyboard-quit ()
1.1 (steve 08-Nov-97): "Abort recursive edit.
1.1 (steve 08-Nov-97): If `zmacs-regions' is true, and the zmacs region is
active in this buffer,
1.1 (steve 08-Nov-97): then this key deactivates the region without
beeping."
1.1 (steve 08-Nov-97): (interactive)
1.1 (steve 08-Nov-97): (if (and (region-active-p)
1.1 (steve 08-Nov-97): (eq (current-buffer) (zmacs-region-buffer)))
1.1 (steve 08-Nov-97): ;; pseudo-zmacs compatibility: don't beep if
this ^G is simply
1.1 (steve 08-Nov-97): ;; deactivating the region. If it is inactive,
beep.
1.1 (steve 08-Nov-97): nil
1.1 (steve 08-Nov-97): (abort-recursive-edit)))
Based on Ben's and your feedback I checked the xemacs-packages.
zmacs-region-buffer is not used anywhere.
(region-active-p) is used > 90 times and I have not found the uses
guarded as shown above.
So bugs could potentially be lurking in there.
Norbert, could you please also review this?
I guess the fix should go into simple.el:
Index: simple.el
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/lisp/simple.el,v
retrieving revision 1.49
diff -u -u -r1.49 simple.el
--- simple.el 17 Jan 2005 11:23:01 -0000 1.49
+++ simple.el 21 Feb 2005 21:20:03 -0000
@@ -3805,7 +3805,8 @@
[ ... ... :active (region-exists-p)]
Which correctly caters to the user's setting of `zmacs-regions'."
- (and zmacs-regions zmacs-region-extent))
+ (and zmacs-regions zmacs-region-extent
+ (eq (current-buffer) (zmacs-region-buffer))))
(defvar zmacs-activate-region-hook nil
"Function or functions called when the region becomes active;
Is this what people agree to?
cmdloop.el and minibuf.el should then only check for (region-active-p)
as well, and replace.el would not have to have the (region-active-p)
testing changed.
Please advise,
Adrian
--
Adrian Aichner
mailto:adrian@xemacs.org
http://www.xemacs.org/