On Thu, Feb 21, 2013 at 11:09 AM, Richard Heck <rgh...@lyx.org> wrote:
> On 02/20/2013 12:23 AM, Scott Kostyshak wrote:
>>
>> I think I am close to correctly fixing #8543, where search finds the
>> same word twice when switching directions.
>>
>> See the patch here:
>>
>> http://www.lyx.org/trac/attachment/ticket/8543/0001-This-patch-flips-the-cursor-but-sel-is-lost.patch
>>
>> The only problem I have is that LFUN_SELECTION_FLIP loses the
>> selection (this is a problem because it looks bad and if you change
>> directions twice in a row we are back to the original problem):
>>
>> Why is the selection lost? Where is it being redrawn?
>> How can I use cur.selectionBegin(), cur.selectionEnd(),
>> cur.selBegin(), and cur.selEnd() to do one of the following?
>> (1) keep the selection
>> or
>> (2) lose it but redraw it how it was after I move the cursor
>
>
> I think the issue is that the anchor is now where the cursor is. We need to
> reset that, since we're now selecting in the opposite direction. The
> attached works for me. It needs minor cleaning (don't simply break where the
> errors are).
Yes, you are right. That does the trick. I had a feeling the anchor
was important but didn't (and still don't) understand its purpose.
Thanks for this correction.
>
> A few other comments.
>
>> diff --git a/src/Text3.cpp b/src/Text3.cpp
>> index 2893e08..5f35071 100644
>> --- a/src/Text3.cpp
>> +++ b/src/Text3.cpp
>> @@ -551,6 +551,30 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
>> break;
>> }
>>
>> + case LFUN_SELECTION_FLIP: {
>> + CursorSlice from;
>> + CursorSlice to;
>> + if (cur.pos() == cur.selectionBegin().pos()) {
>> + from = cur.selBegin();
>> + to = cur.selEnd();
>> + }
>> + else if (cur.pos() == cur.selectionEnd().pos()) {
>> + to = cur.selBegin();
>> + from = cur.selEnd();
>> + }
>> + // else assert?
>>
>
> At least print an error and return. What follows will be invalid in that
> case.
>
>> + if (cur.top() != from)
>> + setCursor(cur, from.pit(), from.pos());
>>
> I'm not sure I follow the logic, but it looks as if, in this case, you set
> the cursor here, and then set it again immediately thereafter. So I took
> this out.
>
>> + if (to == from)
>> + return;
>>
>
> We have an empty selection if this is true. We should not have that, but we
> can just test on cur.selection() to start.
>
> Richard
>
I appreciate your comments, Richard. I was close to finishing the
patch when JMarc had to come along and ruin the fun :)
I learned a lot from experimenting and from your comments.
Thanks,
Scott
