Closed Timothep closed 8 years ago
Thanks for this @Timothep! A couple of questions I have:
You're right.
Manual test
Simply paste following text in a blank page and search&replace the word "test" with something longer and something shorter than 4 characters:
Lorem ipsum dolor sit amet, consectetur adipiscing test elit. Sed et mi ac neque condimentum consequat quis id dolor. Ut aliquam tortor sed suscipit pharetra. Praesent nec accumsan augue. Morbi commodo nulla magna, in blandit tortor test venenatis in. Nunc mollis ligula quis orci fringilla, ac mollis enim porta. Praesent pharetra lacinia purus ut gravida. Suspendisse dignissim ligula nec neque finibus test vehicula. Nam eget orci magna. Proin et dignissim purus. Praesent pellentesque vulputate nisi, at molestie elit congue et.
With this new logic in place, after you replaced an occurence of "test", the next "test" should be selected right away. After the third (last) one, the selection should be canceled.
Automated Test
I didn't see any simple way to write a regression test without having to refactor quite a lot of code or write a massive integration test.
The Replace()
method I extracted could be tested but since it works on defining the next TextSegment
, a test would imply having a filled SearchHits
list, which means testing the DoSearch()
method as a whole...
@Timothep :thumbsup: on skipping the regression test for now
I had a try at manually verifying this behaviour, but I wasn't seeing this behaviour:
With this new logic in place, after you replaced an occurence of "test", the next "test" should be selected right away. After the third (last) one, the selection should be canceled.
Here's a GIF of how it's behaving on my side:
@shiftkey hum. We are not following the same scenario, here's a gif of the one I was talking about (I cannot compile C# code right now, so here's the scenario on the published version - thus without my changes):
@Timothep you're right, I didn't test it right. Thanks for clarifying!
Having now tested it correctly, I'm :thumbsup: on taking this in.
Thanks for all your hard work on this @Timothep!
@shiftkey
Hi there, I fixed the selection logic following a replace action (#401).
var foundHit = (SearchHits as TextSegmentCollection<TextSegment>).FindFirstSegmentWithStartAfter(startLookingFrom);
I hope I didn't miss any corner case.
Cheers, Tim
Closed: #401