* query-replace highlighting
@ 2004-12-10 2:06 Juri Linkov
2004-12-13 19:51 ` Richard Stallman
2005-01-12 1:54 ` Juri Linkov
0 siblings, 2 replies; 6+ messages in thread
From: Juri Linkov @ 2004-12-10 2:06 UTC (permalink / raw)
isearch and query-replace are similar operations in the sense that
both of them search for a string and highlight the current string.
But query-replace misses a useful feature of isearch which highlights
all other matches in a window using `isearch-lazy-highlight-face'.
This can be added to query-replace as well with a simple change in
replace.el. The value `isearch' of the `query-replace-highlight'
variable specifies that query-replace uses isearch highlighting.
While testing it, I noticed a small bug in perform-replace:
when automatic replacement is requested with ! in literal mode,
it still highlights every remaining occurrence quickly while
automatically replacing them. This is especially well visible with
isearch highlighting while every replacement flashes in isearch face.
I think the following patch fixes it correctly:
Index: lisp/replace.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/replace.el,v
retrieving revision 1.192
diff -u -r1.192 replace.el
--- lisp/replace.el 3 Dec 2004 00:19:52 -0000 1.192
+++ lisp/replace.el 10 Dec 2004 01:12:54 -0000
@@ -1380,7 +1419,7 @@
(if (not query-flag)
(let ((inhibit-read-only
query-replace-skip-read-only))
- (unless noedit
+ (unless (or literal noedit)
(replace-highlight (nth 0 real-match-data)
(nth 1 real-match-data)))
(setq noedit
And here is a patch for isearch highlighting in perform-replace:
Index: lisp/replace.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/replace.el,v
retrieving revision 1.192
diff -u -r1.192 replace.el
--- lisp/replace.el 3 Dec 2004 00:19:52 -0000 1.192
+++ lisp/replace.el 10 Dec 2004 02:15:07 -0000
@@ -1281,6 +1311,9 @@
;; (match-data); otherwise it is t if a match is possible at point.
(match-again t)
+ (isearch-string nil)
+ (isearch-regexp nil)
+
(message
(if query-flag
(substitute-command-keys
@@ -1313,6 +1346,11 @@
(if regexp-flag from-string
(regexp-quote from-string))
"\\b")))
+
+ (if (eq query-replace-highlight 'isearch)
+ (setq isearch-string search-string
+ isearch-regexp regexp-flag))
+
(push-mark)
(undo-boundary)
(unwind-protect
@@ -1528,7 +1569,14 @@
(setq unread-command-events
(append (listify-key-sequence key)
unread-command-events))
- (setq done t))))
+ (setq done t)))
+ (when (eq query-replace-highlight 'isearch)
+ ;; Force isearch rehighlighting
+ (if (not (memq def '(skip backup)))
+ (setq isearch-lazy-highlight-last-string nil))
+ ;; Restore isearch data in case of isearching during edit
+ (setq isearch-string search-string
+ isearch-regexp regexp-flag)))
;; Record previous position for ^ when we move on.
;; Change markers to numbers in the match data
;; since lots of markers slow down editing.
@@ -1563,27 +1611,38 @@
(if (= replace-count 1) "" "s")))
(and keep-going stack)))
-(defcustom query-replace-highlight t
- "*Non-nil means to highlight words during query replacement."
- :type 'boolean
+(defcustom query-replace-highlight
+ (if (and search-highlight isearch-lazy-highlight) 'isearch t)
+ "*Non-nil means to highlight words during query replacement.
+If `isearch', use isearch highlighting for query replacement."
+ :type '(choice (const :tag "Highlight" t)
+ (const :tag "No highlighting" nil)
+ (const :tag "Isearch highlighting" 'isearch))
:group 'matching)
(defvar replace-overlay nil)
(defun replace-dehighlight ()
- (and replace-overlay
- (progn
- (delete-overlay replace-overlay)
- (setq replace-overlay nil))))
+ (cond ((eq query-replace-highlight 'isearch)
+ (setq isearch-lazy-highlight-last-string nil)
+ (isearch-dehighlight t)
+ (isearch-lazy-highlight-cleanup isearch-lazy-highlight-cleanup))
+ (query-replace-highlight
+ (when replace-overlay
+ (delete-overlay replace-overlay)
+ (setq replace-overlay nil)))))
(defun replace-highlight (start end)
- (and query-replace-highlight
- (if replace-overlay
- (move-overlay replace-overlay start end (current-buffer))
- (setq replace-overlay (make-overlay start end))
- (overlay-put replace-overlay 'face
- (if (facep 'query-replace)
- 'query-replace 'region)))))
+ (cond ((eq query-replace-highlight 'isearch)
+ (isearch-highlight start end)
+ (isearch-lazy-highlight-new-loop))
+ (query-replace-highlight
+ (if replace-overlay
+ (move-overlay replace-overlay start end (current-buffer))
+ (setq replace-overlay (make-overlay start end))
+ (overlay-put replace-overlay 'face
+ (if (facep 'query-replace)
+ 'query-replace 'region))))))
;; arch-tag: 16b4cd61-fd40-497b-b86f-b667c4cf88e4
;;; replace.el ends here
--
Juri Linkov
http://www.jurta.org/emacs/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: query-replace highlighting
2004-12-10 2:06 query-replace highlighting Juri Linkov
@ 2004-12-13 19:51 ` Richard Stallman
2004-12-14 10:34 ` Juri Linkov
2004-12-15 12:00 ` Juri Linkov
2005-01-12 1:54 ` Juri Linkov
1 sibling, 2 replies; 6+ messages in thread
From: Richard Stallman @ 2004-12-13 19:51 UTC (permalink / raw)
Cc: emacs-devel
This seems like a good feature. However, it is inconsistent
to control it by giving query-replace-highlight a third possible value.
For the case of isearch, we did not give search-highlight a third
possible value. We made a new option, isearch-lazy-highlight.
So we should make a new option, query-replace-lazy-highlight.
Could you please do that?
With this new feature, most of the options in the group
isearch-lazy-highlight are no longer specific to isearch. It would be
better to call the custom group lazy-highlight-matches, and rename all
the options accordingly, making the old names aliases and marking them
obsolete.
The option isearch-lazy-highlight should not be renamed. But the
function isearch-lazy-highlight-new-loop should not test it. Instead,
the caller in isearch.el should test isearch-lazy-highlight before
calling isearch-lazy-highlight-new-loop. The caller in replace.el
would test query-replace-lazy-highlight instead.
The custom group lazy-highlight-matches should have multiple parent
groups, `isearch' and `matching'. That way, people will find it
in both contexts.
Would you like to do these things to finish it up right,
then install your patch?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: query-replace highlighting
2004-12-13 19:51 ` Richard Stallman
@ 2004-12-14 10:34 ` Juri Linkov
2004-12-15 14:58 ` Richard Stallman
2004-12-15 12:00 ` Juri Linkov
1 sibling, 1 reply; 6+ messages in thread
From: Juri Linkov @ 2004-12-14 10:34 UTC (permalink / raw)
Cc: emacs-devel
Richard Stallman <rms@gnu.org> writes:
> This seems like a good feature. However, it is inconsistent
> to control it by giving query-replace-highlight a third possible value.
I retained the value t for users who wish to use old highlighting
method with from-string highlighted in `region' face.
However, to make this possible, a new `query-replace' face inheriting
from `isearch' face should be added. Everyone who wishes to restore
old highlighting method, can change `query-replace' face to inherit
from `region' face, and set `query-replace-lazy-highlight' option to nil.
> For the case of isearch, we did not give search-highlight a third
> possible value. We made a new option, isearch-lazy-highlight.
> So we should make a new option, query-replace-lazy-highlight.
> Could you please do that?
OK. A new option query-replace-lazy-highlight, and then a new face
query-replace inheriting from isearch face.
But what about isearch-lazy-highlight-face? To create a duplicate
query-replace-lazy-highlight-face, or to rename
isearch-lazy-highlight-face to lazy-highlight-face?
> With this new feature, most of the options in the group
> isearch-lazy-highlight are no longer specific to isearch. It would be
> better to call the custom group lazy-highlight-matches, and rename all
> the options accordingly, making the old names aliases and marking them
> obsolete.
As I see it the question is about four options:
isearch-lazy-highlight-cleanup
isearch-lazy-highlight-initial-delay
isearch-lazy-highlight-interval
isearch-lazy-highlight-max-at-a-time
I am not sure if renaming them is the good thing. Creating additional
`lazy-highlight-' name subspace while internal functions still have
`isearch-lazy-highlight-' prefix is a potential source of confusion.
And there is an interactive command `isearch-lazy-highlight-cleanup'
which needs to be renamed. And there are possible other inconsistencies.
What is the benefit of renaming these options if all internal
functions and variables retain the old names? If it is only
for users of Customize to help them to find these options via the
Customize UI, then a much simpler change is just to add the parent
group `matching' to the `isearch-lazy-highlight' group.
--
Juri Linkov
http://www.jurta.org/emacs/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: query-replace highlighting
2004-12-13 19:51 ` Richard Stallman
2004-12-14 10:34 ` Juri Linkov
@ 2004-12-15 12:00 ` Juri Linkov
1 sibling, 0 replies; 6+ messages in thread
From: Juri Linkov @ 2004-12-15 12:00 UTC (permalink / raw)
Cc: emacs-devel
Richard Stallman <rms@gnu.org> writes:
> Would you like to do these things to finish it up right,
> then install your patch?
I reverted the option query-replace-highlight and added a new option
query-replace-lazy-highlight and new face query-replace, with changes
in isearch-lazy-highlight-new-loop.
However, I doubt that isearch-lazy-highlight-* options should be renamed
to lazy-highlight-*. This gives nothing but unnecessary hassles
caused by renaming and making functions and variables of one feature
to have two different prefixes.
Since historically the name of the feature that highlights matches in
the buffer, is `isearch-lazy-highlight', and all its variables and
functions already use that prefix, instead of concealing from users
the fact that query-replace uses the same feature, it would be better
to document this fact in the manual and doc strings.
--
Juri Linkov
http://www.jurta.org/emacs/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: query-replace highlighting
2004-12-14 10:34 ` Juri Linkov
@ 2004-12-15 14:58 ` Richard Stallman
0 siblings, 0 replies; 6+ messages in thread
From: Richard Stallman @ 2004-12-15 14:58 UTC (permalink / raw)
Cc: emacs-devel
> This seems like a good feature. However, it is inconsistent
> to control it by giving query-replace-highlight a third possible value.
I retained the value t for users who wish to use old highlighting
method with from-string highlighted in `region' face.
Maybe we're not talking about the same question. My point is that I
don't like the interface that you proposed (and installed) for the
values of query-replace-highlight.
But what about isearch-lazy-highlight-face? To create a duplicate
query-replace-lazy-highlight-face, or to rename
isearch-lazy-highlight-face to lazy-highlight-face?
Let's rename it to lazy-highlight-face. Thanks.
As I see it the question is about four options:
isearch-lazy-highlight-cleanup
isearch-lazy-highlight-initial-delay
isearch-lazy-highlight-interval
isearch-lazy-highlight-max-at-a-time
I am not sure if renaming them is the good thing.
I am not sure if installing the feature without renaming
them is a good thing.
What is the benefit of renaming these options if all internal
functions and variables retain the old names?
The benefit of renaming these options is to give them names that fit
their meaning.
Renaming the internal functions and variables is a separate question.
Perhaps it is a good idea to rename them too. This is a less
important question, because it is merely an internal question.
So it can wait until we've decided the external one.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: query-replace highlighting
2004-12-10 2:06 query-replace highlighting Juri Linkov
2004-12-13 19:51 ` Richard Stallman
@ 2005-01-12 1:54 ` Juri Linkov
1 sibling, 0 replies; 6+ messages in thread
From: Juri Linkov @ 2005-01-12 1:54 UTC (permalink / raw)
For commands that use lazy highlighting it would be cleaner
to use the values of global variables active at the time of calling
`isearch-lazy-highlight-new-loop', than values that are active
at the time of calling the timer function `isearch-lazy-highlight-update'.
In other words, I propose to use in `isearch-lazy-highlight-search'
the values `isearch-lazy-highlight-last-string',
`isearch-lazy-highlight-regexp', `isearch-lazy-highlight-case-fold-search'
with the values set during calling `isearch-lazy-highlight-new-loop'
instead of the values of `isearch-string', `isearch-case-fold-search'.
The main reason for this change is to avoid the need to change and
maintain the global values of `isearch-string', `isearch-regexp'
and `isearch-case-fold-search' for the timer function. It should be
the care of code that calls `isearch-lazy-highlight-new-loop' to call
it again with new parameters when the matching string changes.
Index: lisp/isearch.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/isearch.el,v
retrieving revision 1.254
diff -u -r1.254 isearch.el
--- lisp/isearch.el 11 Jan 2005 23:03:01 -0000 1.254
+++ lisp/isearch.el 12 Jan 2005 00:52:53 -0000
@@ -2377,10 +2377,11 @@
(defun isearch-lazy-highlight-search ()
"Search ahead for the next or previous match, for lazy highlighting.
Attempt to do the search exactly the way the pending isearch would."
- (let ((case-fold-search isearch-case-fold-search)
+ (let ((case-fold-search isearch-lazy-highlight-case-fold-search)
+ (isearch-regexp isearch-lazy-highlight-regexp)
(search-spaces-regexp search-whitespace-regexp))
(funcall (isearch-search-fun)
- isearch-string
+ isearch-lazy-highlight-last-string
(if isearch-forward
(if isearch-lazy-highlight-wrapped
isearch-lazy-highlight-start
And below is the consequently simplified code in replace.el.
Index: lisp/replace.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/replace.el,v
retrieving revision 1.201
diff -u -r1.201 replace.el
--- lisp/replace.el 11 Jan 2005 23:04:16 -0000 1.201
+++ lisp/replace.el 12 Jan 2005 00:56:42 -0000
@@ -1322,9 +1322,6 @@
;; (match-data); otherwise it is t if a match is possible at point.
(match-again t)
- (isearch-string isearch-string)
- (isearch-regexp isearch-regexp)
- (isearch-case-fold-search isearch-case-fold-search)
(message
(if query-flag
(substitute-command-keys
@@ -1358,10 +1355,7 @@
(regexp-quote from-string))
"\\b")))
(when query-replace-lazy-highlight
- (setq isearch-string search-string
- isearch-regexp (or delimited-flag regexp-flag)
- isearch-case-fold-search case-fold-search
- isearch-lazy-highlight-last-string nil))
+ (setq isearch-lazy-highlight-last-string nil))
(push-mark)
(undo-boundary)
@@ -1431,8 +1425,10 @@
(let ((inhibit-read-only
query-replace-skip-read-only))
(unless (or literal noedit)
- (replace-highlight (nth 0 real-match-data)
- (nth 1 real-match-data)))
+ (replace-highlight
+ (nth 0 real-match-data) (nth 1 real-match-data)
+ search-string (or delimited-flag regexp-flag)
+ case-fold-search))
(setq noedit
(replace-match-maybe-edit
next-replacement nocasify literal
@@ -1448,7 +1444,10 @@
;; `real-match-data'.
(while (not done)
(set-match-data real-match-data)
- (replace-highlight (match-beginning 0) (match-end 0))
+ (replace-highlight
+ (match-beginning 0) (match-end 0)
+ search-string (or delimited-flag regexp-flag)
+ case-fold-search)
;; Bind message-log-max so we don't fill up the message log
;; with a bunch of identical messages.
(let ((message-log-max nil))
@@ -1580,11 +1579,6 @@
unread-command-events))
(setq done t)))
(when query-replace-lazy-highlight
- ;; Restore isearch data for lazy highlighting
- ;; in case of isearching during recursive edit
- (setq isearch-string search-string
- isearch-regexp (or delimited-flag regexp-flag)
- isearch-case-fold-search case-fold-search)
;; Force lazy rehighlighting only after replacements
(if (not (memq def '(skip backup)))
(setq isearch-lazy-highlight-last-string nil))))
@@ -1624,7 +1618,7 @@
(defvar replace-overlay nil)
-(defun replace-highlight (beg end)
+(defun replace-highlight (beg end &optional string regexp case-fold)
(if query-replace-highlight
(if replace-overlay
(move-overlay replace-overlay beg end (current-buffer))
@@ -1632,7 +1626,10 @@
(overlay-put replace-overlay 'priority 1) ;higher than lazy overlays
(overlay-put replace-overlay 'face 'query-replace)))
(if query-replace-lazy-highlight
- (isearch-lazy-highlight-new-loop)))
+ (let ((isearch-string string)
+ (isearch-regexp regexp)
+ (isearch-case-fold-search case-fold))
+ (isearch-lazy-highlight-new-loop))))
(defun replace-dehighlight ()
(when replace-overlay
--
Juri Linkov
http://www.jurta.org/emacs/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2005-01-12 1:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-12-10 2:06 query-replace highlighting Juri Linkov
2004-12-13 19:51 ` Richard Stallman
2004-12-14 10:34 ` Juri Linkov
2004-12-15 14:58 ` Richard Stallman
2004-12-15 12:00 ` Juri Linkov
2005-01-12 1:54 ` Juri Linkov
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).