bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[linuxfever]

Hello,

This is more of a suggestion/wish, rather than an actual bug, which could
enhance significantly replacement functionality in a filled text (region,
paragraph, buffer, etc).

Currently it seems that when trying to replace an expression comprising two
words, say for example 'replace this', then the functions 'replace-string'
or 'replace-regexp' fail to locate the expression when that is split along
two lines in the text, i.e., in the above example when the word 'replace' is
at the end of one line, and the word 'this' is at the beginning of the next
one.

I learned today
(http://old.nabble.com/Search-in-filled-text-does-not-work-correctly-td33390451.html#a33392628)
that there are ways around this by specifying a newline between words, for
example, searching for 'replace C-q C-j this' would actually do the work but
there are two problems with this. First, it is inefficient as it takes
longer to type, and second it would replace only the expressions that are
actually split in two lines and leave out the ones separated by a normal
SPC.

In a nutshell, I believe enhancing the functionality of the replacement
functions to accommodate for expressions split in two lines, would make
their usability so much easier. Thanks for your time and for such a great
and versatile editor!

-- 
View this message in context: http://old.nabble.com/Replace-expressions%3A-enhance-functionality-when-searching-in-filled-paragraphs-tp33392837p33392837.html
Sent from the Emacs - Bugs mailing list archive at Nabble.com.

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Glenn Morris]

linuxfever wrote:

> Currently it seems that when trying to replace an expression comprising two
> words, say for example 'replace this', then the functions 'replace-string'
> or 'replace-regexp' fail to locate the expression when that is split along
> two lines in the text, i.e., in the above example when the word 'replace' is
> at the end of one line, and the word 'this' is at the beginning of the next
> one.

You can use query-replace-regexp for this.

Sample input:

This is text. This is text. This is text. This is text. Replace this.
This is text. This is text. This is text. This is text. This is text. Replace
this.
Replace                    this.

M-x query-replace-regexp RET \(replace\)\([ ^I^J]*\)\(this\) RET Done\2that RET

Output:

This is text. This is text. This is text. This is text. Done that.
This is text. This is text. This is text. This is text. This is text. Done
that.
Done                    that.

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Kevin Rodgers]

On 2/25/12 6:57 PM, Glenn Morris wrote:
> linuxfever wrote:
>
>> Currently it seems that when trying to replace an expression comprising two
>> words, say for example 'replace this', then the functions 'replace-string'
>> or 'replace-regexp' fail to locate the expression when that is split along
>> two lines in the text, i.e., in the above example when the word 'replace' is
>> at the end of one line, and the word 'this' is at the beginning of the next
>> one.
>
> You can use query-replace-regexp for this.
>
> Sample input:
>
> This is text. This is text. This is text. This is text. Replace this.
> This is text. This is text. This is text. This is text. This is text. Replace
> this.
> Replace                    this.
>
> M-x query-replace-regexp RET \(replace\)\([ ^I^J]*\)\(this\) RET Done\2that RET
>
> Output:
>
> This is text. This is text. This is text. This is text. Done that.
> This is text. This is text. This is text. This is text. This is text. Done
> that.
> Done                    that.

Would it make sense for replace-regexp/query-replace-regexp (and other commands
that read regexps from the user) to respect search-whitespace-regexp, just like
isearch?

-- 
Kevin Rodgers
Denver, Colorado, USA

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Dani Moncayo]

> Would it make sense for replace-regexp/query-replace-regexp (and other
> commands
> that read regexps from the user) to respect search-whitespace-regexp, just
> like
> isearch?

Good question.

Quote from the docstring of `isearch-forward-regexp':

 In regexp incremental searches, a space or spaces normally matches
 any whitespace (the variable `search-whitespace-regexp' controls
 precisely what that means).  If you want to search for a literal space
 and nothing else, enter C-q SPC.

Does anyone knows why `query-replace-regexp' don't behave that way too?

I think it should, i.e., `query-replace-regexp' should find the same
matches that `isearch-forward-regexp' would find with the same search
regexp.


-- 
Dani Moncayo

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Dani Moncayo]

> Does anyone knows why `query-replace-regexp' don't behave that way too?

Arrggg.  Forgive my bad English. I meant:
  Does anyone know why `query-replace-regexp' doesn't behave that way too?


-- 
"Dani Moncayo, who, some day, will learn to revise his mails before
sending them."

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

>> Would it make sense for replace-regexp/query-replace-regexp (and
>> other commands that read regexps from the user) to respect
>> search-whitespace-regexp, just like isearch?
>
> Good question.
>
> Quote from the docstring of `isearch-forward-regexp':
>
>  In regexp incremental searches, a space or spaces normally matches
>  any whitespace (the variable `search-whitespace-regexp' controls
>  precisely what that means).  If you want to search for a literal space
>  and nothing else, enter C-q SPC.
>
> Does anyone know why `query-replace-regexp' doesn't behave that way too?
>
> I think it should, i.e., `query-replace-regexp' should find the same
> matches that `isearch-forward-regexp' would find with the same search
> regexp.

Full agreement.  This patch for 24.2 brings query-replace closer to isearch:

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2012-02-24 22:46:57 +0000
+++ lisp/replace.el	2012-02-28 10:38:58 +0000
@@ -1752,6 +1752,7 @@ (defun perform-replace (from-string repl
 	  (if (and case-fold-search search-upper-case)
 	      (isearch-no-upper-case-p from-string regexp-flag)
 	    case-fold-search))
+	 (search-spaces-regexp (if regexp-flag search-whitespace-regexp))
          (nocasify (not (and case-replace case-fold-search)))
          (literal (or (not regexp-flag) (eq regexp-flag 'literal)))
          (search-function
@@ -2117,9 +2115,8 @@ (defun replace-highlight (match-beg matc
   (if query-replace-lazy-highlight
       (let ((isearch-string string)
 	    (isearch-regexp regexp)
-	    (search-whitespace-regexp nil)
 	    (isearch-case-fold-search case-fold)
 	    (isearch-forward t)
 	    (isearch-error nil))

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Glenn Morris]

Juri Linkov wrote:

> Full agreement.  This patch for 24.2 brings query-replace closer to isearch:

The doc and manual would need updating; since at present
search-whitespace-regexp is only documented to affect incremental
searches.

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

> The doc and manual would need updating; since at present
> search-whitespace-regexp is only documented to affect incremental
> searches.

There is one problem of using `search-whitespace-regexp'
in `query-replace-regexp'.

When the user doesn't want `SPC' to match any sequence of whitespace
characters in regexp isearch `C-M-s', it it possible to type `C-q SPC'.

But how to do the same in `query-replace-regexp'?

`C-M-% replace C-q SPC this' doesn't quote SPC as a literal character.

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Dani Moncayo]

On Wed, Feb 29, 2012 at 01:41, Juri Linkov <juri@jurta.org> wrote:
>> The doc and manual would need updating; since at present
>> search-whitespace-regexp is only documented to affect incremental
>> searches.
>
> There is one problem of using `search-whitespace-regexp'
> in `query-replace-regexp'.
>
> When the user doesn't want `SPC' to match any sequence of whitespace
> characters in regexp isearch `C-M-s', it it possible to type `C-q SPC'.
>
> But how to do the same in `query-replace-regexp'?
>
> `C-M-% replace C-q SPC this' doesn't quote SPC as a literal character.

FWIW: the isearch commands have this problem too, when editing the
search string from the minibuffer.

For example: C-M-s replace this M-e M-b DEL C-q SPC RET
--> The new search string doesn't have the intended quoted space.

-- 
Dani Moncayo

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

>> There is one problem of using `search-whitespace-regexp'
>> in `query-replace-regexp'.
>>
>> When the user doesn't want `SPC' to match any sequence of whitespace
>> characters in regexp isearch `C-M-s', it it possible to type `C-q SPC'.
>>
>> But how to do the same in `query-replace-regexp'?
>>
>> `C-M-% replace C-q SPC this' doesn't quote SPC as a literal character.
>
> FWIW: the isearch commands have this problem too, when editing the
> search string from the minibuffer.
>
> For example: C-M-s replace this M-e M-b DEL C-q SPC RET
> --> The new search string doesn't have the intended quoted space.

Actually this `search-spaces-regexp' thing is rather a feature of word search
intermixed with the pure regexp search into the hybrid half-word/regexp search.

What `C-q SPC' does is adding the invisible regexp `[ ]' to the search string.

In the minibuffer `C-q SPC' could do the same, not necessarily invisibly
because you still see `[ ]' when typing e.g. `C-M-s replace C-q SPC this M-e'

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

After installing
http://lists.gnu.org/archive/html/emacs-devel/2012-09/msg00008.html
this functionality is available to query-replace, but currently only
when it's started from isearch.

To use it in query-replace independently from isearch,
I see two ways to toggle whitespace matching in query-replace:

1. Add a new defcustom like `case-replace' with a name e.g.
`replace-lax-whitespace' and use it in `perform-replace'.
When non-nil, spaces will match any whitespace in replacement.

2. Add a new command like `query-replace-regexp' with a name e.g.
`query-replace-lax-whitespace', where to let-bind
`replace-lax-whitespace' to t and call `perform-replace'.

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

> 1. Add a new defcustom like `case-replace' with a name e.g.
> `replace-lax-whitespace' and use it in `perform-replace'.
> When non-nil, spaces will match any whitespace in replacement.

This is implemented in the following patch:

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2012-08-06 05:33:39 +0000
+++ lisp/replace.el	2012-09-02 11:27:15 +0000
@@ -33,6 +33,13 @@ (defcustom case-replace t
   :type 'boolean
   :group 'matching)
 
+(defcustom replace-lax-whitespace nil
+  "Non-nil means `query-replace' matches a sequence of whitespace chars.
+When you enter a space or spaces in the strings or regexps to be replaced,
+it will match any sequence matched by the regexp `search-whitespace-regexp'."
+  :type 'boolean
+  :group 'matching)
+
 (defvar query-replace-history nil
   "Default history list for query-replace commands.
 See `query-replace-from-history-variable' and
@@ -226,6 +233,10 @@ (defun query-replace (from-string to-str
 matched is all caps, or capitalized, then its replacement is upcased
 or capitalized.)
 
+If `replace-lax-whitespace' is non-nil, a space or spaces in the string
+to be replaced will match a sequence of whitespace chars defined by the
+regexp in `search-whitespace-regexp'.
+
 Third arg DELIMITED (prefix arg if interactive), if non-nil, means replace
 only matches surrounded by word boundaries.
 Fourth and fifth arg START and END specify the region to operate on.
@@ -270,6 +281,10 @@ (defun query-replace-regexp (regexp to-s
 all caps, or capitalized, then its replacement is upcased or
 capitalized.)
 
+If `replace-lax-whitespace' is non-nil, a space or spaces in the regexp
+to be replaced will match a sequence of whitespace chars defined by the
+regexp in `search-whitespace-regexp'.
+
 Third arg DELIMITED (prefix arg if interactive), if non-nil, means replace
 only matches surrounded by word boundaries.
 Fourth and fifth arg START and END specify the region to operate on.
@@ -346,6 +361,10 @@ (defun query-replace-regexp-eval (regexp
 Preserves case in each replacement if `case-replace' and `case-fold-search'
 are non-nil and REGEXP has no uppercase letters.
 
+If `replace-lax-whitespace' is non-nil, a space or spaces in the regexp
+to be replaced will match a sequence of whitespace chars defined by the
+regexp in `search-whitespace-regexp'.
+
 Third arg DELIMITED (prefix arg if interactive), if non-nil, means replace
 only matches that are surrounded by word boundaries.
 Fourth and fifth arg START and END specify the region to operate on."
@@ -437,6 +456,10 @@ (defun replace-string (from-string to-st
 \(Preserving case means that if the string matched is all caps, or capitalized,
 then its replacement is upcased or capitalized.)
 
+If `replace-lax-whitespace' is non-nil, a space or spaces in the string
+to be replaced will match a sequence of whitespace chars defined by the
+regexp in `search-whitespace-regexp'.
+
 In Transient Mark mode, if the mark is active, operate on the contents
 of the region.  Otherwise, operate from point to the end of the buffer.
 
@@ -475,6 +498,10 @@ (defun replace-regexp (regexp to-string
 Preserve case in each match if `case-replace' and `case-fold-search'
 are non-nil and REGEXP has no uppercase letters.
 
+If `replace-lax-whitespace' is non-nil, a space or spaces in the regexp
+to be replaced will match a sequence of whitespace chars defined by the
+regexp in `search-whitespace-regexp'.
+
 In Transient Mark mode, if the mark is active, operate on the contents
 of the region.  Otherwise, operate from point to the end of the buffer.
 
@@ -1717,12 +1745,12 @@ (defun replace-match-maybe-edit (newtext
   (replace-match newtext fixedcase literal)
   noedit)
 
-(defvar replace-search-function 'search-forward
+(defvar replace-search-function nil
   "Function to use when searching for strings to replace.
 It is used by `query-replace' and `replace-string', and is called
 with three arguments, as if it were `search-forward'.")
 
-(defvar replace-re-search-function 're-search-forward
+(defvar replace-re-search-function nil
   "Function to use when searching for regexps to replace.
 It is used by `query-replace-regexp', `replace-regexp',
 `query-replace-regexp-eval', and `map-query-replace-regexp'.
@@ -1755,9 +1783,16 @@ (defun perform-replace (from-string repl
          (nocasify (not (and case-replace case-fold-search)))
          (literal (or (not regexp-flag) (eq regexp-flag 'literal)))
          (search-function
-	  (if regexp-flag
-	      replace-re-search-function
-	    replace-search-function))
+	  (or (if regexp-flag
+		  replace-re-search-function
+		replace-search-function)
+	      (if replace-lax-whitespace
+		  (if regexp-flag
+		      #'re-search-forward-lax-whitespace
+		    #'search-forward-lax-whitespace)
+		(if regexp-flag
+		    #'re-search-forward
+		  #'search-forward))))
          (search-string from-string)
          (real-match-data nil)       ; The match data for the current match.
          (next-replacement nil)
@@ -2120,7 +2156,10 @@ (defun replace-highlight (match-beg matc
 	    ;; Set isearch-word to nil because word-replace is regexp-based,
 	    ;; so `isearch-search-fun' should not use `word-search-forward'.
 	    (isearch-word nil)
-	    (search-whitespace-regexp nil)
+	    (isearch-lax-whitespace
+	     (and replace-lax-whitespace (not regexp)))
+	    (isearch-regexp-lax-whitespace
+	     (and replace-lax-whitespace regexp))
 	    (isearch-case-fold-search case-fold)
 	    (isearch-forward t)
 	    (isearch-error nil))

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2012-09-02 09:31:45 +0000
+++ lisp/isearch.el	2012-09-02 11:27:15 +0000
@@ -1579,14 +1612,11 @@ (defun isearch-query-replace (&optional
 	;; set `search-upper-case' to nil to not call
 	;; `isearch-no-upper-case-p' in `perform-replace'
 	(search-upper-case nil)
-	(replace-search-function
-	 (if (and isearch-lax-whitespace (not regexp-flag))
-	     #'search-forward-lax-whitespace
-	   replace-search-function))
-	(replace-re-search-function
-	 (if (and isearch-regexp-lax-whitespace regexp-flag)
-	     #'re-search-forward-lax-whitespace
-	   replace-re-search-function))
+	(replace-lax-whitespace
+	 (and search-whitespace-regexp
+	      (if isearch-regexp
+		  isearch-regexp-lax-whitespace
+		isearch-lax-whitespace)))
 	;; Set `isearch-recursive-edit' to nil to prevent calling
 	;; `exit-recursive-edit' in `isearch-done' that terminates
 	;; the execution of this command when it is non-nil.
@@ -2956,6 +3045,10 @@ (defun isearch-lazy-highlight-search ()
 	    (isearch-regexp isearch-lazy-highlight-regexp)
 	    (isearch-word isearch-lazy-highlight-word)
 	    (search-invisible nil)	; don't match invisible text
+	    (isearch-lax-whitespace
+	     isearch-lazy-highlight-lax-whitespace)
+	    (isearch-regexp-lax-whitespace
+	     isearch-lazy-highlight-regexp-lax-whitespace)
 	    (retry t)
 	    (success nil)
 	    (isearch-forward isearch-lazy-highlight-forward)

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

> This is implemented in the following patch:
> +	      (if replace-lax-whitespace
> +		  (if regexp-flag
> +		      #'re-search-forward-lax-whitespace
> +		    #'search-forward-lax-whitespace)
> +		(if regexp-flag
> +		    #'re-search-forward
> +		  #'search-forward))))

Since query-replace is going in the direction of closer integration with
isearch, the right thing would be to get its search function from isearch.

Before calling `isearch-search-fun', `perform-replace' should bind
the same isearch related variables as in `replace-highlight'.
This will guarantee that highlighted matches are the same that it's
going to replace.

With this change query-replace will also correctly handle replacements of
symbols started from isearch-symbol mode with no additional code
in replace.el to implement this.

This patch is based on the previously sent patch and could be installed
in separate commits:

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2012-09-02 11:27:15 +0000
+++ lisp/replace.el	2012-09-05 08:33:27 +0000
@@ -1786,13 +1791,15 @@ (defun perform-replace (from-string repl
 	  (or (if regexp-flag
 		  replace-re-search-function
 		replace-search-function)
-	      (if replace-lax-whitespace
-		  (if regexp-flag
-		      #'re-search-forward-lax-whitespace
-		    #'search-forward-lax-whitespace)
-		(if regexp-flag
-		    #'re-search-forward
-		  #'search-forward))))
+	      (let ((isearch-regexp regexp-flag)
+		    (isearch-word delimited-flag)
+		    (isearch-lax-whitespace
+		     (and replace-lax-whitespace (not regexp-flag)))
+		    (isearch-regexp-lax-whitespace
+		     (and replace-lax-whitespace regexp-flag))
+		    (isearch-case-fold-search case-fold-search)
+		    (isearch-forward t))
+		(isearch-search-fun))))
          (search-string from-string)
          (real-match-data nil)       ; The match data for the current match.
          (next-replacement nil)
@@ -1846,9 +1853,6 @@ (defun perform-replace (from-string repl
                                (vector repeat-count repeat-count
                                        replacements replacements)))))
 
-    (if delimited-flag
-	(setq search-function 're-search-forward
-	      search-string (word-search-regexp from-string)))
     (when query-replace-lazy-highlight
       (setq isearch-lazy-highlight-last-string nil))
 
@@ -1934,7 +1938,7 @@ (defun perform-replace (from-string repl
 		    (replace-highlight
 		     (nth 0 real-match-data) (nth 1 real-match-data)
 		     start end search-string
-		     (or delimited-flag regexp-flag) case-fold-search))
+		     regexp-flag delimited-flag case-fold-search))
 		  (setq noedit
 			(replace-match-maybe-edit
 			 next-replacement nocasify literal
@@ -1953,7 +1957,7 @@ (defun perform-replace (from-string repl
 		  (replace-highlight
 		   (match-beginning 0) (match-end 0)
 		   start end search-string
-		   (or delimited-flag regexp-flag) case-fold-search)
+		   regexp-flag delimited-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)
@@ -2135,15 +2139,10 @@ (defun perform-replace (from-string repl
 (defvar replace-overlay nil)
 
 (defun replace-highlight (match-beg match-end range-beg range-end
-			  string regexp case-fold)
+			  string regexp word case-fold)
   (if query-replace-highlight
       (if replace-overlay
 	  (move-overlay replace-overlay match-beg match-end (current-buffer))
@@ -2153,9 +2152,7 @@ (defun replace-highlight (match-beg matc
   (if query-replace-lazy-highlight
       (let ((isearch-string string)
 	    (isearch-regexp regexp)
-	    ;; Set isearch-word to nil because word-replace is regexp-based,
-	    ;; so `isearch-search-fun' should not use `word-search-forward'.
-	    (isearch-word nil)
+	    (isearch-word word)
 	    (isearch-lax-whitespace
 	     (and replace-lax-whitespace (not regexp)))
 	    (isearch-regexp-lax-whitespace

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Stefan Monnier]

> Since query-replace is going in the direction of closer integration with
> isearch, the right thing would be to get its search function from isearch.

Agreed.

> This patch is based on the previously sent patch and could be installed
> in separate commits:

I haven't had much time to devote to this thread, but it generally looks
like it's moving in the right direction and I don't see any obvious
problem in your patch.


        Stefan

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

>> Since query-replace is going in the direction of closer integration with
>> isearch, the right thing would be to get its search function from isearch.
>
> Agreed.

Installed.  Since this enhancement request is completely implemented,
and all mentioned problems are fixed, I'm closing it.

>>  (defun replace-highlight (match-beg match-end range-beg range-end
>> -			  string regexp case-fold)
>> +			  string regexp word case-fold)
>
> Maybe we should use an isearch-state argument?

Actually these arguments correspond to the variables of `perform-replace',
not isearch state.  So I renamed them to the names used in `perform-replace',
causing isearch-related let-bindings in `replace-highlight' to be
the exact copy of let-bindings in `perform-replace'.

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Dani Moncayo]

On Thu, Sep 6, 2012 at 10:54 AM, Juri Linkov <juri@jurta.org> wrote:
>>> Since query-replace is going in the direction of closer integration with
>>> isearch, the right thing would be to get its search function from isearch.
>>
>> Agreed.
>
> Installed.  [...]

Thank you so much Juri.  I have some comments:

1.  You've defined two separate variables (`isearch-lax-whitespace'
and `isearch-regexp-lax-whitespace') to enable/disable the lax
whitespace matching in search commands: one for basic search commands
and the other for regexp search commands.  But there is only one
similar variable (replace-lax-whitespace) which controls both basic
and regexp replace commands.  Why this inconsistency?  I.e. why not
define also a `replace-regexp-lax-whitespace' variable?

2.  While in an incremental search commands, it is possible to toggle
the value of the corresponding variable with `M-s SPC'.  Why not
having the same possibility in incremental replace commands?

3.  Many users will want a consistent behavior wrt whitespace-matching
between (regexp) search and (regexp) replace commands.  So, why not
allowing to "connect" the corresponding variables?  I.e. why not
defining some special value for `replace-lax-whitespace' and
`replace-regexp-lax-whitespace' which means "get the value from the
corresponding search variable" ?

TIA.

-- 
Dani Moncayo

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

> 1.  You've defined two separate variables (`isearch-lax-whitespace'
> and `isearch-regexp-lax-whitespace') to enable/disable the lax
> whitespace matching in search commands: one for basic search commands
> and the other for regexp search commands.  But there is only one
> similar variable (replace-lax-whitespace) which controls both basic
> and regexp replace commands.  Why this inconsistency?  I.e. why not
> define also a `replace-regexp-lax-whitespace' variable?

`isearch-regexp-lax-whitespace' was necessary to provide
backward-compatibility for old functionality.  Very likely
it will be declared obsolete.  But of course, it would be better
to have `replace-regexp-lax-whitespace' for consistency until
they both will be declared obsolete simultaneously.

> 2.  While in an incremental search commands, it is possible to toggle
> the value of the corresponding variable with `M-s SPC'.  Why not
> having the same possibility in incremental replace commands?

Isearch has different implementation than query-replace.
query-replace uses the normal minibuffer to read a string to replace.
Implementing `M-s SPC' for it means more trouble:
`query-replace-read-from' should set the arg `keymap' of
`read-from-minibuffer' to a new keymap with the `M-s SPC' keybinding
bound to a function to toggle the value of the defcustom option.

> 3.  Many users will want a consistent behavior wrt whitespace-matching
> between (regexp) search and (regexp) replace commands.  So, why not
> allowing to "connect" the corresponding variables?  I.e. why not
> defining some special value for `replace-lax-whitespace' and
> `replace-regexp-lax-whitespace' which means "get the value from the
> corresponding search variable" ?

When it will be decided that isearch and query-replace should have
the same default values, then `replace-lax-whitespace' could inherit
its default value from `isearch-lax-whitespace'.

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Dani Moncayo]

On Thu, Sep 6, 2012 at 6:50 PM, Juri Linkov <juri@jurta.org> wrote:
>> 1.  You've defined two separate variables (`isearch-lax-whitespace'
>> and `isearch-regexp-lax-whitespace') to enable/disable the lax
>> whitespace matching in search commands: one for basic search commands
>> and the other for regexp search commands.  But there is only one
>> similar variable (replace-lax-whitespace) which controls both basic
>> and regexp replace commands.  Why this inconsistency?  I.e. why not
>> define also a `replace-regexp-lax-whitespace' variable?
>
> `isearch-regexp-lax-whitespace' was necessary to provide
> backward-compatibility for old functionality.  Very likely
> it will be declared obsolete.

Ah Good.  I think we all agree that this feature is
confusing/unnecessary in regexp search & replace.

>  But of course, it would be better
> to have `replace-regexp-lax-whitespace' for consistency until
> they both will be declared obsolete simultaneously.
>
>> 2.  While in an incremental search commands, it is possible to toggle
>> the value of the corresponding variable with `M-s SPC'.  Why not
>> having the same possibility in incremental replace commands?
>
> Isearch has different implementation than query-replace.
> query-replace uses the normal minibuffer to read a string to replace.
> Implementing `M-s SPC' for it means more trouble:
> `query-replace-read-from' should set the arg `keymap' of
> `read-from-minibuffer' to a new keymap with the `M-s SPC' keybinding
> bound to a function to toggle the value of the defcustom option.

But I meant to bind `M-s SPC' not while reading the arguments, but
right after that, i.e. during the interactive replacing.

>> 3.  Many users will want a consistent behavior wrt whitespace-matching
>> between (regexp) search and (regexp) replace commands.  So, why not
>> allowing to "connect" the corresponding variables?  I.e. why not
>> defining some special value for `replace-lax-whitespace' and
>> `replace-regexp-lax-whitespace' which means "get the value from the
>> corresponding search variable" ?
>
> When it will be decided that isearch and query-replace should have
> the same default values, then `replace-lax-whitespace' could inherit
> its default value from `isearch-lax-whitespace'.

But I don't meant that.  What I want is the possibility to have these
two variables "synchronized" or "connected" at every moment, even
after toggling its value with `M-s SPC'.  IOW: make Isearch and
query-replace _share_ the same setting (at every moment).

-- 
Dani Moncayo

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

> But I meant to bind `M-s SPC' not while reading the arguments, but
> right after that, i.e. during the interactive replacing.

If you meant `query-replace-map' then it lacks other isearch
keybindings too, e.g. toggling case-fold, regexp and word mode.
It has a similar feature to edit the replacement string but it's bound
to the `E' key unlike `M-e' to edit the search string in isearch.

> But I don't meant that.  What I want is the possibility to have these
> two variables "synchronized" or "connected" at every moment, even
> after toggling its value with `M-s SPC'.  IOW: make Isearch and
> query-replace _share_ the same setting (at every moment).

They are already shared and synchronized when you run query-replace
from isearch like e.g. `C-s replace this M-w SPC M-%'.

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

> from isearch like e.g. `C-s replace this M-w SPC M-%'.
                                           ===
                                           M-s

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Dani Moncayo]

On Thu, Sep 6, 2012 at 9:11 PM, Juri Linkov <juri@jurta.org> wrote:
>> But I meant to bind `M-s SPC' not while reading the arguments, but
>> right after that, i.e. during the interactive replacing.
>
> If you meant `query-replace-map' then it lacks other isearch
> keybindings too, e.g. toggling case-fold, regexp and word mode.

Well, I think all these commands would make sense during a
query-replace, for adjusting the search (that is taking place as part
of the replace operation) in the same way that they would adjust an
Isearch.  So, why not binding them in `query-replace-map'?  That would
make Isearch and query-replace more coherent.

> It has a similar feature to edit the replacement string but it's bound
> to the `E' key unlike `M-e' to edit the search string in isearch.

IMO, here `M-e' would be better, for consistency with Isearch.

>> But I don't meant that.  What I want is the possibility to have these
>> two variables "synchronized" or "connected" at every moment, even
>> after toggling its value with `M-s SPC'.  IOW: make Isearch and
>> query-replace _share_ the same setting (at every moment).
>
> They are already shared and synchronized when you run query-replace
> from isearch like e.g. `C-s replace this M-s SPC M-%'.

That's fine, but it's not the case I'm talking about.

It seems that my English is worse than I thought :)  I'll try to
explain it in yet another way: I'd like to configure my Emacs so that,
at any moment, an Isearch would find exactly the same matches than a
query-replace (with the same search string, of course), and without
the need to start the query-replace from an Isearch.

One way to achieve that goal would be to define a special value for
`replace-lax-whitespace' whose meaning be "use the current value of
`isearch-lax-whitespace'".


-- 
Dani Moncayo

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Dani Moncayo]

>> It has a similar feature to edit the replacement string but it's bound
>> to the `E' key unlike `M-e' to edit the search string in isearch.
>
> IMO, here `M-e' would be better, for consistency with Isearch.

Well, not really.  Obviously `M-e' should be used for editing the
_search_ string (i.e. the string to replace), not the replacement
string.

-- 
Dani Moncayo

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Stefan Monnier]

> `isearch-regexp-lax-whitespace' was necessary to provide
> backward-compatibility for old functionality.  Very likely
> it will be declared obsolete.  But of course, it would be better
> to have `replace-regexp-lax-whitespace' for consistency until
> they both will be declared obsolete simultaneously.

I don't think it makes sense to add something only for "consistency"
reasons, knowing it'll be obsoleted soon.


        Stefan

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Dani Moncayo]

On Thu, Sep 6, 2012 at 11:25 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> `isearch-regexp-lax-whitespace' was necessary to provide
>> backward-compatibility for old functionality.  Very likely
>> it will be declared obsolete.  But of course, it would be better
>> to have `replace-regexp-lax-whitespace' for consistency until
>> they both will be declared obsolete simultaneously.
>
> I don't think it makes sense to add something only for "consistency"
> reasons, knowing it'll be obsoleted soon.

I think replace-regexp-lax-whitespace is needed while the Isearch
counterpart (isearch-regexp-lax-whitespace) exists, if you want to
support the option I was asking for, whereby the lax-whitespace
settings in query-replace[-regexp] are "connected" (or "redirected")
to the Isearch counterparts.

And IMO this option is important, for a consistent user-experience.

But FWIW, as I said, I consider this lax-whitespace feature confusing
and unnecesary in regexp search/replace.  Thus, I'll rather remove
both isearch-regexp-lax-whitespace and replace-regexp-lax-whitespace.

-- 
Dani Moncayo

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

> I think replace-regexp-lax-whitespace is needed while the Isearch
> counterpart (isearch-regexp-lax-whitespace) exists, if you want to
> support the option I was asking for, whereby the lax-whitespace
> settings in query-replace[-regexp] are "connected" (or "redirected")
> to the Isearch counterparts.
>
> And IMO this option is important, for a consistent user-experience.

The reason why replace-regexp-lax-whitespace is important to have
is not just for consistency but because replace-lax-whitespace
should not affect regexp replacements.  But currently its value applies
to regexp replacements because it depends on calls from isearch that has
a separate variable for regexp that starts a lax-whitespace regexp replacement
to replace the same regexp matches as in isearch.  So actually a lax-whitespace
regexp replacement feature exists in replace.el implicitly.  A clean way to
fix this problem is to add a new option replace-regexp-lax-whitespace:

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2012-09-06 09:12:16 +0000
+++ lisp/replace.el	2012-09-07 09:27:35 +0000
@@ -35,7 +35,15 @@ (defcustom case-replace t
 
 (defcustom replace-lax-whitespace nil
   "Non-nil means `query-replace' matches a sequence of whitespace chars.
-When you enter a space or spaces in the strings or regexps to be replaced,
+When you enter a space or spaces in the strings to be replaced,
+it will match any sequence matched by the regexp `search-whitespace-regexp'."
+  :type 'boolean
+  :group 'matching
+  :version "24.3")
+
+(defcustom replace-regexp-lax-whitespace nil
+  "Non-nil means `query-replace-regexp' matches a sequence of whitespace chars.
+When you enter a space or spaces in the regexps to be replaced,
 it will match any sequence matched by the regexp `search-whitespace-regexp'."
   :type 'boolean
   :group 'matching
@@ -282,7 +290,7 @@ (defun query-replace-regexp (regexp to-s
 all caps, or capitalized, then its replacement is upcased or
 capitalized.)
 
-If `replace-lax-whitespace' is non-nil, a space or spaces in the regexp
+If `replace-regexp-lax-whitespace' is non-nil, a space or spaces in the regexp
 to be replaced will match a sequence of whitespace chars defined by the
 regexp in `search-whitespace-regexp'.
 
@@ -362,7 +370,7 @@ (defun query-replace-regexp-eval (regexp
 Preserves case in each replacement if `case-replace' and `case-fold-search'
 are non-nil and REGEXP has no uppercase letters.
 
-If `replace-lax-whitespace' is non-nil, a space or spaces in the regexp
+If `replace-regexp-lax-whitespace' is non-nil, a space or spaces in the regexp
 to be replaced will match a sequence of whitespace chars defined by the
 regexp in `search-whitespace-regexp'.
 
@@ -499,7 +507,7 @@ (defun replace-regexp (regexp to-string
 Preserve case in each match if `case-replace' and `case-fold-search'
 are non-nil and REGEXP has no uppercase letters.
 
-If `replace-lax-whitespace' is non-nil, a space or spaces in the regexp
+If `replace-regexp-lax-whitespace' is non-nil, a space or spaces in the regexp
 to be replaced will match a sequence of whitespace chars defined by the
 regexp in `search-whitespace-regexp'.
 
@@ -1790,9 +1798,9 @@ (defun perform-replace (from-string repl
 	      (let ((isearch-regexp regexp-flag)
 		    (isearch-word delimited-flag)
 		    (isearch-lax-whitespace
-		     (and replace-lax-whitespace (not regexp-flag)))
+		     replace-lax-whitespace)
 		    (isearch-regexp-lax-whitespace
-		     (and replace-lax-whitespace regexp-flag))
+		     replace-regexp-lax-whitespace)
 		    (isearch-case-fold-search case-fold-search)
 		    (isearch-forward t))
 		(isearch-search-fun))))
@@ -2151,9 +2159,9 @@ (defun replace-highlight (match-beg matc
 	    (isearch-regexp regexp-flag)
 	    (isearch-word delimited-flag)
 	    (isearch-lax-whitespace
-	     (and replace-lax-whitespace (not regexp-flag)))
+	     replace-lax-whitespace)
 	    (isearch-regexp-lax-whitespace
-	     (and replace-lax-whitespace regexp-flag))
+	     replace-regexp-lax-whitespace)
 	    (isearch-case-fold-search case-fold-search)
 	    (isearch-forward t)
 	    (isearch-error nil))

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2012-09-06 09:12:16 +0000
+++ lisp/isearch.el	2012-09-07 09:27:42 +0000
@@ -1613,10 +1613,9 @@ (defun isearch-query-replace (&optional
 	;; `isearch-no-upper-case-p' in `perform-replace'
 	(search-upper-case nil)
 	(replace-lax-whitespace
-	 (and search-whitespace-regexp
-	      (if isearch-regexp
-		  isearch-regexp-lax-whitespace
-		isearch-lax-whitespace)))
+	 isearch-lax-whitespace)
+	(replace-regexp-lax-whitespace
+	 isearch-regexp-lax-whitespace)
 	;; Set `isearch-recursive-edit' to nil to prevent calling
 	;; `exit-recursive-edit' in `isearch-done' that terminates
 	;; the execution of this command when it is non-nil.

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

> The reason why replace-regexp-lax-whitespace is important to have
> is not just for consistency but because replace-lax-whitespace
> should not affect regexp replacements.  But currently its value applies
> to regexp replacements because it depends on calls from isearch that has
> a separate variable for regexp that starts a lax-whitespace regexp replacement
> to replace the same regexp matches as in isearch.  So actually a lax-whitespace
> regexp replacement feature exists in replace.el implicitly.  A clean way to
> fix this problem is to add a new option replace-regexp-lax-whitespace:

I see no way to avoid adding replace-regexp-lax-whitespace
as counterpart of isearch-regexp-lax-whitespace, so I installed it.

Other features proposed by Dani (adding isearch-like keybindings to
`query-replace-map' and connecting isearch and query-replace default values)
don't belong to bug#10885 and could be discussed on emacs-devel because
there are many problems with them like a problem that `query-replace-map'
is used by `y-or-n-p' and other packages, and some of bindings
(e.g. `scroll-down') are implemented only in `y-or-n-p'
but not in `perform-replace'.

Perhaps, a new common keymap should be separated from
`query-replace-map', so `query-replace-map' could inherit from it
and add own keybindings including isearch-like keybindings.

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Stefan Monnier]

>  (defun replace-highlight (match-beg match-end range-beg range-end
> -			  string regexp case-fold)
> +			  string regexp word case-fold)

Maybe we should use an isearch-state argument?


        Stefan

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[linuxfever]

Glenn Morris-3 wrote:
> 
> 
> You can use query-replace-regexp for this.
> 
> M-x query-replace-regexp RET \(replace\)\([ ^I^J]*\)\(this\) RET
> Done\2that RET
> 
> 

Thanks, but this command does not seem to work for me. Moreover, it is still
quite involved with all the extra characters between the two words, and the
idea is to avoid this complication.
-- 
View this message in context: http://old.nabble.com/Replace-expressions%3A-enhance-functionality-when-searching-in-filled-paragraphs-tp33392837p33393821.html
Sent from the Emacs - Bugs mailing list archive at Nabble.com.

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Stefan Monnier]

>> M-x query-replace-regexp RET \(replace\)\([ ^I^J]*\)\(this\) RET
>> Done\2that RET

I think the "\2" should be replaced by just a space (it's probably fine
in the above example, but for a general solution the user would want
some re-filling afterwards).


        Stefan

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Dani Moncayo]

> In a nutshell, I believe enhancing the functionality of the replacement
> functions to accommodate for expressions split in two lines, would make
> their usability so much easier.

Adding a "word-type mode" to query-replace (analogous of the word-type
mode of Isearch) would do what you want, and would be a good feature,
IMO.

That is: add an option to query-replace whereby the matches are found
using a word-type search, instead of the (default) normal search.

-- 
Dani Moncayo

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Juri Linkov]

>> In a nutshell, I believe enhancing the functionality of the replacement
>> functions to accommodate for expressions split in two lines, would make
>> their usability so much easier.
>
> Adding a "word-type mode" to query-replace (analogous of the word-type
> mode of Isearch) would do what you want, and would be a good feature,
> IMO.
>
> That is: add an option to query-replace whereby the matches are found
> using a word-type search, instead of the (default) normal search.

There exists already a "word query-replace mode" with `C-u M-%',
but currently it is very limited.

To bring it in sync with the "word search" is possible
with this patch (for 24.2):

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2012-02-24 22:46:57 +0000
+++ lisp/replace.el	2012-02-27 10:55:04 +0000
@@ -1813,10 +1813,7 @@ (defun perform-replace (from-string repl
 
     (if delimited-flag
 	(setq search-function 're-search-forward
-	      search-string (concat "\\b"
-				    (if regexp-flag from-string
-				      (regexp-quote from-string))
-				    "\\b")))
+	      search-string (word-search-regexp from-string)))
     (when query-replace-lazy-highlight
       (setq isearch-lazy-highlight-last-string nil))
 

bug#10885: Replace expressions: enhance functionality when searching in filled paragraphs

[Dani Moncayo]

>> Adding a "word-type mode" to query-replace (analogous of the word-type
>> mode of Isearch) would do what you want, and would be a good feature,
>> IMO.
>>
>> That is: add an option to query-replace whereby the matches are found
>> using a word-type search, instead of the (default) normal search.
>
> There exists already a "word query-replace mode" with `C-u M-%',
> but currently it is very limited.
>
> To bring it in sync with the "word search" is possible
> with this patch (for 24.2):

This patch seems to work too.  Thank you!


-- 
Dani Moncayo