Regexp find & replace & replace all implementation #202

Merged
merged 8 commits into from Jan 10, 2017

Conversation

Projects
None yet
5 participants
Contributor

barkovv commented Nov 20, 2016

Resolves #89
It handles case and wrap search correctly. I guess this feature is usefull since support for python plugins was dropped.

Contributor

barkovv commented Nov 25, 2016

Please review and merge it.

Contributor

barkovv commented Dec 7, 2016

Member

raveit65 commented Dec 7, 2016

As we are a small team it is helpful to provide simple steps to test.
something which work now but not before.

Contributor

barkovv commented Dec 9, 2016

@raveit65 @XRevan86 @lukefromdc @sc0w
Steps to test:
Type some text in main window (e.g. "some text").
Open "Search dialog"via ether menu or hotkey
Turn on "Match regular expression" checkbox
Type some regular expression into search entry (e.g "t.*")
As a result matching text must be selected in main window (in our example "text" is selected)

Member

lukefromdc commented Dec 9, 2016

I found a nasty bug, when using "test" and "grtry" as test lines. A search on "t." worked, selecting everything after the "t" but a seach on "t" caused a segfault:

luke@ubuntu:~$ pluma

(pluma:10546): Gtk-CRITICAL **: real_set_mark: assertion '_gtk_text_iter_get_btree (where) == tree' failed

(pluma:10546): Gtk-CRITICAL **: real_set_mark: assertion '_gtk_text_iter_get_btree (where) == tree' failed
Segmentation fault
Contributor

barkovv commented Dec 19, 2016

@lukefromdc
Very strange, I've just tested that, and it works as it must.
Builded for this versions:
webkit2gtk 2.14.2-2
gtksourceview3 3.22.2-1
iso-codes 3.70-1
mate-desktop-schemas 1.16.1-1
zenity 3.22.0+1+gcd1647c-1
itstool 2.0.2+5+g676f3f7-2
mate-common 1.16.0-1
yelp-tools 3.18.0+1+g193c2bd-1

Contributor

barkovv commented Dec 19, 2016

@lukefromdc
I've merged changes from pstream and recheckerd.
Text is "grtry test"
Search queries: "t.st" , "t.", "t.*", "t" (no quotes in query)
Could you give me more information for reproduce?

Member

lukefromdc commented Dec 19, 2016

This has a lot of problems, (and no recent change in behavior-still segfaults).

OK, I used a test file (one I cannot publish) with a lot of numbers in them today. Will use no quotes in the search terms below for clarity:

*A search for 105 finds instances of 105 throughout the file, with or without "match regular expression" checked. This is as expected.

A search for 105 I would expect to find lines beginning with 105 and those portions of lines after occurances of 105-but instead if finds all instances of 10 and does NOT select anything after the occurrance of 10 in that line. Clearly * is causing the last character of the search term to be cut off the string.

A search for 105.* returns all lines beginning with 105 and those portions of lines after occurances of 105 This, however, is what I would have expected from 105* and not 105.* Apparently the pair of characters .* is now doing what I would expect * to do by itself.

Finally, a search for 5.* returns everything after an instance of 5 in a line, and a search for 5* closes Pluma with a segfault, apparently from trying to search for a string of zero length. This makes sense, as adding the * glob is causing the last character of the string to be cut off from the search term. When there is only one character in the search string, cutting it off reduces the length of the string to zero and thus the segfault.

Looks like there is a string handling issue here, possibly with handling the EOF

Contributor

barkovv commented Dec 20, 2016

@lukefromdc

"150.*" matches "150a", "150abc", "150asdcd", "150 000", "150000", etc
"150*" matches "150", "1500", "1500", "15000",etc

This is correct behaviour ( see https://developer.gnome.org/glib/stable/glib-regex-syntax.html )
I guess you've messed up wildcard and regular expression.
I'll see what's going on with cutting symbol off, thanks for finding this bug.

Contributor

barkovv commented Dec 20, 2016

@lukefromdc
"5*" matches "","5","55","555","5555", etc
"5*" means "digit five repeated zero or more times". So even empty string matches this query. I agree that segmentation fault isn't correct reaction for that.

Member

lukefromdc commented Dec 21, 2016

I am only familiar with wildcard use, this is not something I've ever used so no I didn't see the expected behavior was different. Didn't expect the segfaults or the cut off characters though so at least I found something useful with these tests.

Contributor

barkovv commented Jan 8, 2017

@lukefromdc
I fixed the bug you've found. Could you merge it? I am not familiar with contributing to this project and don't know what to do for merge.

Member

lukefromdc commented Jan 8, 2017

I don't have merge authority for Pluma but will test this

Member

raveit65 commented Jan 10, 2017

@lukefromdc
Is this PR fine now?

Member

lukefromdc commented Jan 10, 2017

Segfaults are gone, though 150* still matches 15 , not sure if that is the intended behavior or not. I really don't have the experience using regular expressions to know what is correct.

epgfm commented Jan 10, 2017

I believe it is correct, the expression would parse to "15" followed by zero or more "0"
To match things starting with 150 you would use 150.* (15 followed by zero or more of any characters)

Member

lukefromdc commented Jan 10, 2017

Than this should be good to go, though I do not have merge authority on Pluma

Contributor

barkovv commented Jan 10, 2017

@raveit65
I guess it is ready to merge

@raveit65 raveit65 merged commit a79faa6 into mate-desktop:master Jan 10, 2017

Member

raveit65 commented Jan 10, 2017

Thank you and thanks to @lukefromdc for the review.
For some reasons we have 2 useless merge commits.
I did a mistake and fired up 'git push' to early on master :-/

Member

monsta commented Oct 31, 2017

@barkovv: please have a look at #254 - is it valid?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment