On Mon, Aug 17, 2020 at 12:02 PM Huw Davies <huw@codeweavers.com> wrote:

On Mon, Aug 17, 2020 at 05:22:57AM +0200, Damjan Jovanovic wrote:
> Signed-off-by: Damjan Jovanovic <damjan.jov@gmail.com>
> ---
>�� dlls/riched20/richole.c�� | 77 +++++++++++++++++++++++++--
>�� dlls/riched20/tests/richole.c | 98 ++++++++++++++++++++++++++++++++++-
>�� 2 files changed, 169 insertions(+), 6 deletions(-)

> diff --git a/dlls/riched20/richole.c b/dlls/riched20/richole.c
> index 7c281ab364..1fd307eb92 100644
> --- a/dlls/riched20/richole.c
> +++ b/dlls/riched20/richole.c
> @@ -2191,17 +2191,63 @@ static HRESULT WINAPI ITextRange_fnMove(ITextRange *me, LONG unit, LONG count, L
>�� return E_NOTIMPL;
>�� }
>��
> +static HRESULT textrange_movestart(ITextRange *range, LONG unit, LONG count, LONG *delta)
> +{
> +�� LONG old_start, old_end, new_start, new_end;
> +�� HRESULT hr = S_OK;
> +
> +�� if (!count)
> +�� {
> +�� if (delta)
> +�� *delta = 0;
> +�� return S_FALSE;
> +�� }
> +
> +�� ITextRange_GetStart(range, &old_start);
> +�� ITextRange_GetEnd(range, &old_end);
> +�� switch (unit)
> +�� {
> +�� case tomCharacter:
> +�� {
> +�� ME_TextEditor *editor;
> +�� ME_Cursor cursor;
> +�� LONG moved;
> +�� ITextRangeImpl *This = impl_from_ITextRange(range);
> +�� editor = This->child.reole->editor;
> +
> +�� ME_CursorFromCharOfs(editor, old_start, &cursor);
> +�� moved = ME_MoveCursorChars(editor, &cursor, count, FALSE);
> +�� new_start = old_start + moved;
> +�� new_end = old_end;
> +�� if (new_end < new_start)
> +�� new_end = new_start;
> +�� if (delta)
> +�� *delta = moved;
> +�� break;
> +�� }
> +�� default:
> +�� FIXME("unit %d is not supported\n", unit);
> +�� return E_NOTIMPL;
> +�� }
> +�� if (new_start == old_start)
> +�� hr = S_FALSE;
> +�� ITextRange_SetStart(range, new_start);
> +�� ITextRange_SetEnd(range, new_end);
> +
> +�� return hr;
> +}
> +
>�� static HRESULT WINAPI ITextRange_fnMoveStart(ITextRange *me, LONG unit, LONG count,
>�� LONG *delta)
>�� {
>�� ITextRangeImpl *This = impl_from_ITextRange(me);
>��
> -�� FIXME("(%p)->(%d %d %p): stub\n", This, unit, count, delta);
> +�� TRACE("(%p)->(%d %d %p)\n", This, unit, count, delta);
>��
>�� if (!This->child.reole)
>�� return CO_E_RELEASED;
>��
> -�� return E_NOTIMPL;
> +�� return textrange_movestart(me, unit, count, delta);
>�� }

I know textrange_moveend() is a thing, but it's not clear why.
I think all of the implementation of MoveStart should go in
ITextRange_fnMoveStart, then ITextSelection_fnMoveStart would call
ITextRage_MoveStart().

I tried that. Unfortunately ITextSelection_fnQueryInterface() implements IID_ITextRange by returning itself, resulting in ITextSelection_fnMoveEnd() calling itself until the stack overflows.

ITextRange and ITextSelection are on separate objects, we can't QI from the one to the other. Using a common textrange_moveend() style function independent of the objects, seems like the only way. Sadly even those functions are wrong in my patches, because they cannot call impl_from_ITextRange() on an ITextSelection. I think the calling functions could pass them whatever they need from their impl, such as ME_TextEditor*.

New patches will take a while.