Major "Replace All" bug

Discussion Forums discussion Major "Replace All" bug

This topic contains 8 replies, has 4 voices, and was last updated by  simon 1 year, 8 months ago.

Viewing 9 posts - 1 through 9 (of 9 total)
Author Posts
Author Posts
March 4, 2012 at 6:06 am #7714

pepak
Member

Hi!

Programmer’s Notepad version 2.3.4.2350 (the version available for download) has a major bug with Replace All if national characters are searched for:

- Find what = “ěščř”

- Replace with = “anything”

- Find next – finds the next occurence of “ěščř”.

- Mark all – marks all occurences

- Replace – replaces the next occurence

- Replace all – doesn’t replace anything and reports that “0 occurence(s) replaced”

Note: I have just tried version 2.1.5.2222 and it has the same problem.

March 8, 2012 at 12:47 am #18671

CoDEmanX
Member

Known problem, see http://code.google.com/p/pnotepad/issues/detail?id=1414

I was able to reproduce it (UTF-16 LE), added your comment to the bug tracker issue #1414

Thanks

March 8, 2012 at 2:45 am #18672

NickDMax
Member

I need to work out how to submit patches. My copy of pn does not have this bug (I finished the Unicode RegEx implementation).

This actually touches the code in quite a few places and required a little “work around” since XPressive is not really UTF8 aware, so to do a search for “ěščř” is fine since that encodes to a set of bytes which XPressive can find. However, “Å™+” is different — here the Å™ encodes as two bytes, and so the quantifier + gets associated with the second char but not the first. — So my work around is to enclose unicode chars in non-capture groups. So that “Å™+” becomes “(?:Å™)+” and the search goes on as expected. I don’t really *like* this work around since the RegEx you enter is not the RegEx that gets used, but so far it seems to be working well.

Anyway. I will try to post my version/fork of PN so that Simon can take a look and merge perhaps merge this into the next release.

March 8, 2012 at 4:33 am #18673

NickDMax
Member

So one can now find the changes I made here: https://code.google.com/r/nickdmax-updates-pn/source/detail?r=21db2fc279a02836a563db91b463d990bb1a3451

April 10, 2012 at 12:32 pm #18678

simon
Key Master

@NickDMax Thanks very much for your change, I’ve pulled it and it’s now in the main tree. I hope to find time to make a release with this soon.

In the mean time, if you’re interested in getting commit rights to the main source tree just let me know – always happy to have more active developers!

April 17, 2012 at 8:11 pm #18679

NickDMax
Member

I would like to have commit rights if that is possible. I will send you an email when I have time.

I have however found a bug/defect with my *fancy* replace. It has errors at the boundaries, that is if the regex relies upon text before or after the match. For example, given the test: “This is a test” search for bs+(.) and replace with U1 — Should result it “This Is A Test” but fails to make replacements.

This happens because I made an assumption that if the RegEx matched the “match text” in the document then it would also match the “match text” in the regex_replace() function. Sadly, it does not work as well as I hoped! If the regex is looking for text outside of the current match then the replace does not happen.

so this makes bB and the look-ahead and look-behind assertions buggy with replace at the beginning or end of a match.

As of yet I have not come up with any strategy to correct this. Ideally one would apply regex_replace() directly to the document iterator but I don’t think this is compatible with Scintilla’s replace function — not to mention one would need to create new a DocumentIterator object for the replacement string.

If you did integrate my replace section into the main trunk then we need to decide if this requires pulling it, or just logging it as a new defect.

July 21, 2012 at 8:03 pm #19176

simon
Key Master

I’ve rolled back the replace section of your changeset. The rest remains for now, thanks!

July 22, 2012 at 12:59 am #19178

NickDMax
Member

Thanks, I have an updated version that will fix things (it has not been plugged into PN yet still testing at the command line). This should also allow for some future features to be added such as replacing with XML-encoding or URL-encoding (rather than just upper case/lower case).

I will try to get that change and the updates to the status bar text checked in by the end of this week.

July 25, 2012 at 10:02 pm #19187

simon
Key Master

Thanks Nick. I may release before this is ready, at least to testers, but woud like to get it in for the next round.

Viewing 9 posts - 1 through 9 (of 9 total)

You must be logged in to reply to this topic.