Bug 140970 - replace: placeholder \0 (fullmatch) does not work
Summary: replace: placeholder \0 (fullmatch) does not work
Status: VERIFIED FIXED
Alias: None
Product: kdelibs
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: openSUSE Linux
: NOR wishlist
Target Milestone: ---
Assignee: David Faure
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-31 20:34 UTC by Maciej Pilichowski
Modified: 2010-05-21 00:57 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch to fix (813 bytes, patch)
2007-02-01 21:53 UTC, Philip Rodrigues
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Pilichowski 2007-01-31 20:34:02 UTC
Version:            (using KDE KDE 3.5.5)
Installed from:    SuSE RPMs

This is NOT a duplicate of http://bugs.kde.org/show_bug.cgi?id=134101

Let's say we have text 
 vector a; 
 vector b; 
 
 Now, replace 
 vector 
 with 
 std::\0 
 with placeholders enabled. The result? 
 
 std::\0 a; 
 std::\0 b;
Comment 1 Philip Rodrigues 2007-02-01 21:22:01 UTC
Reassigning to kdelibs based on comment 10 in bug 134101. 

Notes from other comments in that report:
* The bug is partly the inconsistency in the UI: the "use placeholders" checkbox should be disabled when not in regex mode. 
* Additionally, using \0 in non-regex mode might be useful.
Comment 2 Philip Rodrigues 2007-02-01 21:53:34 UTC
Created attachment 19504 [details]
Patch to fix

Patch to fix. Disables the placeholder stuff when regex is disabled. Apply in
kdelibs/kutils
Comment 3 Maciej Pilichowski 2007-02-01 22:11:56 UTC
Philip -- it is not solution.

It looks like this -- Konqueror does not print selection, "fix" it by disabling printing.

I think, that a lot of people are just biased my the term "placeholder". Stop thinking about regexps -- user could replace text with text itself but extended.
Text by PrefixTextSuffix. Where both Prefix and Suffix are optional.

Existing option, placeholder, is handy, but if anyone likes, please name it "entered_text".
Comment 4 Anders Lund 2007-02-01 23:07:52 UTC
I gave this some more thought on my way to work this morning, and my 
conclusion is that we should just disable the use backreferences button in 
non-regex replacement.

If you want to use it, just do a regex replace.

We avoid mixing concepts, and dealing with how to replace '\0' in non-regex 
replace.
Comment 5 Philip Rodrigues 2007-02-01 23:13:05 UTC
I agree with Anders. Trying to mix "normal" and regex methods is just going to lead to confusion, which isn't worth it when you can just enable regex mode to do what you want
Comment 6 Maciej Pilichowski 2007-02-02 08:19:11 UTC
Anders, Philip -- don't look at replace function through regexps. It is not mixing concepts.

It is just extending text. And there is no confusion, is that really so hard to imagine (real case) to replace
vector
with
std::vector
?

string
with
std::string
?

But it quite obvious that the replace is really

something
with
std::the_source_text

So even it there is term used placeholder and there is \0 to use it, it has nothing to do with regexp.

And you cannot just enable regexp.
Don't you *see* it? :-)

Without regexp I can replace above sentence _fast_, with regexp -- not.

> dealing with how to replace '\0' in non-regex replace. 

And this is really a problem? With the entered text to replace. No rocket science here. 
 
Comment 7 Anders Lund 2007-02-03 16:52:56 UTC
> And you cannot just enable regexp. 

Why not?

I don't know how you work, but i would typically place the cursor in a word to replace such as 'string', then press c-r, a-x (enable regex), a-l (enable placeholders, you had to do that anyways), a-w (enable whole words only, you'd have to do that anyways), enter 'std::\0' in the replacement box, hit return.

There is exactly two extra keypresses or one mouse click extra, if that slows you down, you should install ktouch and practice a bit really ;)

In cases where your search string contains regex operators, you would need to escape those of course, but none of the examples you posted has that issue. And since we just picks up word characters when launching the dialog, you would be typing in the find entry anyways.

> Don't you *see* it? :-)

NO.
Comment 8 Maciej Pilichowski 2007-02-03 17:26:28 UTC
Anders,

because in regexp
(
does not mean
(

I hope now you can see you cannot just copy&paste sentence I gave as an example. Which means without simple replace with "use the text I typed" it is extra work to carefully check _the expression_. Not _text_ any more.
Comment 9 Maciej Pilichowski 2007-02-03 17:28:16 UTC
Sorry for the omission:
> And since we just picks up word characters when launching the dialog, you
> would be typing in the find entry anyways. 

Nope, when I work with KDevelop it is very useful to shift+arrow-key to select a word, ctrl+f and voila all you have to do is set the text for replacement.
Comment 10 Sabine Faure 2010-05-11 22:46:19 UTC
I retested this today in trunk and the bug is still there as it was reported in the description above:
- Launch Kmail
- Click on 'New button'
- Type the following words in the text editor:
*test a 
*test b
- Go to Edit menu/replace
- Replace 'te+st' with '\0 hello' using the 'Complete Match' placeholder 

Instead of having 'test a hello' and 'test b hello' there is '\0 hello a' and 'te\0 hello'. 
The second time there seems to be an offset with the replacement.

Trunk, Svn Rev 1125510
Comment 11 David Faure 2010-05-18 19:08:49 UTC
SVN commit 1128206 by dfaure:

Use the KReplace API the intended way, so that replacements with placeholders (\0) work.
CCBUG: 140970


 M  +2 -3      ktextedit.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1128206
Comment 12 David Faure 2010-05-18 19:10:12 UTC
SVN commit 1128208 by dfaure:

Use the KReplace API the intended way, so that replacements with placeholders (\0) work.
BUG: 140970
FIXED-IN: 4.4.4


 M  +2 -3      ktextedit.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1128208
Comment 13 Sabine Faure 2010-05-21 00:57:34 UTC
It is corrected now.

Note: in the steps above I forgot to check the 'Regular expression' check box but if it is not done the replacement won't work.

The replace placeholder functionality works properly now.

Trunk, Svn Rev 1128989