Re: [Q] xemacs-21.5: Use region-exists-p in replace.el to fix ediff-regions-* and dired-ediff for zmacs-regions t
4:35 p.m.
Scott Evans <gse(a)antisleep.com> writes:
On Fri, 22 Oct 2004, Steve Youngs wrote:
> * Stephen J Turnbull <stephen(a)xemacs.org> writes:
>
> > Steve Youngs did the commit, patch was by Scott Evans. I can't
> > find either the patch itself
>
> <http://list-archive.xemacs.org/xemacs-patches/200209/msg00115.html>
>
> > or the approval on xemacs-patches,
>
> <http://list-archive.xemacs.org/xemacs-patches/200210/msg00181.html>
>
> You really don't expect me to remember anything about it do you?
I remember writing the code... :) But I'm coming into this discussion
cold since I don't subscribe to xemacs-patches. What's up?
Hi Scott, All!
I finally found time looking at this again.
replace.el<1.8> introduced support for regions to operate on.
dired-ediff no longer worked since then, when using zmacs-regions t.
I think I found the issue:
While Emacs' mark-active is per-buffer, XEmacs' (region-active-p) is
global.
Therefor I think the fix for XEmacs is the following:
I intend to commit after a few days of testing and waiting for feedback.
Best regards,
Adrian
lisp/ChangeLog addtion:
2005-02-20 Adrian Aichner <adrian(a)xemacs.org>
* replace.el (delete-non-matching-lines): Honor region only when
active in current buffer.
* replace.el (kill-non-matching-lines): Ditto.
* replace.el (copy-non-matching-lines): Ditto.
* replace.el (delete-matching-lines): Ditto.
* replace.el (kill-matching-lines): Ditto.
* replace.el (copy-matching-lines): Ditto.
* replace.el (perform-replace): Ditto.
Index: replace.el
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/lisp/replace.el,v
retrieving revision 1.10
diff -u -u -r1.10 replace.el
--- replace.el 12 May 2003 05:12:10 -0000 1.10
+++ replace.el 20 Feb 2005 16:16:52 -0000
＠＠ -296,7 +296,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-non-matching-lines regexp t nil beg end))
＠＠ -313,7 +314,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-non-matching-lines regexp t t beg end))
＠＠ -330,7 +332,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-non-matching-lines regexp nil t beg end))
＠＠ -392,7 +395,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-matching-lines regexp t nil beg end))
＠＠ -409,7 +413,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-matching-lines regexp t t beg end))
＠＠ -426,7 +431,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-matching-lines regexp nil t beg end))
＠＠ -831,7 +837,8 ＠＠
(substitute-command-keys
"Query replacing %s with %s: (\\<query-replace-map>\\[help] for help)
"))))
;; If the region is active, operate on region.
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
;; Original Per Abrahamsen's code simply narrowed the region,
;; thus providing a visual indication of the search boundary.
;; Stallman, on the other hand, handles it like this.
--
Adrian Aichner
mailto:adrian＠xemacs.org
http://www.xemacs.org/
3:40 a.m.
New subject: [Q] xemacs-21.5: Use region-exists-p in replace.el to fix ediff-regions-* and dired-ediff for zmacs-regions t
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.
While Emacs' mark-active is per-buffer, XEmacs'
(region-active-p) is global.
Therefor I think the fix for XEmacs is the following:
I intend to commit after a few days of testing and waiting
for feedback.
Best regards,
Adrian
lisp/ChangeLog addtion:
2005-02-20 Adrian Aichner <adrian(a)xemacs.org>
* replace.el (delete-non-matching-lines): Honor region only when
active in current buffer.
* replace.el (kill-non-matching-lines): Ditto.
* replace.el (copy-non-matching-lines): Ditto.
* replace.el (delete-matching-lines): Ditto.
* replace.el (kill-matching-lines): Ditto.
* replace.el (copy-matching-lines): Ditto.
* replace.el (perform-replace): Ditto.
Index: replace.el
===================================================================
RCS file: /pack/xemacscvs/XEmacs/xemacs/lisp/replace.el,v
retrieving revision 1.10
diff -u -u -r1.10 replace.el
--- replace.el 12 May 2003 05:12:10 -0000 1.10
+++ replace.el 20 Feb 2005 16:16:52 -0000
＠＠ -296,7 +296,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-non-matching-lines regexp t nil
beg end)) ＠＠ -313,7 +314,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-non-matching-lines regexp t t
beg end)) ＠＠ -330,7 +332,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-non-matching-lines regexp nil t
beg end)) ＠＠ -392,7 +395,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-matching-lines regexp t nil beg
end)) ＠＠ -409,7 +413,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-matching-lines regexp t t beg
end)) ＠＠ -426,7 +431,8 ＠＠
(let ((beg nil)
(end nil)
(count nil))
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
(setq beg (region-beginning))
(setq end (region-end)))
(setq count (operate-on-matching-lines regexp nil t beg
end)) ＠＠ -831,7 +837,8 ＠＠
(substitute-command-keys
"Query replacing %s with %s:
(\\<query-replace-map>\\[help] for help) "))))
;; If the region is active, operate on region.
- (when (region-active-p)
+ (when (and (region-active-p)
+ (eq (current-buffer) (zmacs-region-buffer)))
;; Original Per Abrahamsen's code simply narrowed the region,
;; thus providing a visual indication of the search boundary.
;; Stallman, on the other hand, handles it like this.
11:32 a.m.
New subject: [Q] xemacs-21.5: Use region-exists-p in replace.el to fix ediff-regions-* and dired-ediff for zmacs-regions t
"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.
9:30 p.m.
New subject: [Q] xemacs-21.5: Use region-exists-p in replace.el to fix ediff-regions-* and dired-ediff for zmacs-regions t
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/
9:51 p.m.
New subject: [Q] xemacs-21.5: Use region-exists-p in replace.el to fix ediff-regions-* and dired-ediff for zmacs-regions t
Adrian Aichner <adrian(a)xemacs.org> writes:
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.
Interestingly enough, it was originally added by me. I was annoyed
that C-g in the minibuffer deactivated the region instead of
cancelling the minibuffer.
10:19 p.m.
New subject: [Q] xemacs-21.5: Use region-exists-p in replace.el to fix ediff-regions-* and dired-ediff for zmacs-regions t
Hrvoje Niksic <hniksic(a)xemacs.org> writes:
Adrian Aichner <adrian(a)xemacs.org> writes:
> 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.
Interestingly enough, it was originally added by me. I was annoyed
that C-g in the minibuffer deactivated the region instead of
cancelling the minibuffer.
That's what I guessed from the ChangeLog but I did not try to find the
actual patch from Nov 1997.
Call me lazy :-)
Are you OK with my suggestion?
--
Adrian Aichner
mailto:adrian＠xemacs.org
http://www.xemacs.org/
7004
days inactive
7005
days old
5 comments
3 participants
participants (3)
-
Adrian Aichner -
Ben Wing -
Hrvoje Niksic