Re: [Libreoffice] Undo Redo Re: [REVIEW] Re: SmNodeToTextVisitor Fixes

Hi Jonas,

On Mon, 2011-02-14 at 20:17 +0100, Jonas Finnemann Jensen wrote:

>         
> I think it ought to be that easy... More documentation would speedup
> development and reduce bugs dramatically...
> (Maybe we should write a few doxygen comments for EditEngine, if we
> figure out how it works).

I noticed a few comments in ImpEditEngine that I found informative (with
the aid of google translate), which I think would be a lot better in
doxygen comments on the actual functions instead of buried in the
implementation ones (assuming I've guessed right that Imp means
implementation, which seems right to me by how that class is used).

Would be cool, would be a bit difficult to document it when I don't know
what it's supposed to be doing though. That isn't an excuse though, I
don't mind trying to work out a few of them.

>         I had a very bad solution that seemed to work but made the
>         unit tests
>         not work.
>         I replaced SetText with:
>            SmGetActiveView()->GetEditWindow()->SelectAll();
>            SmGetActiveView()->GetEditWindow()->InsertText(formula);
> Why is this a bad solution ?
> Any idea, why this doesn't work with the unit tests ?
> It seems like it might be a fairly good solution... Unless undo/redo
> manages selections too... So that this becomes two steps ?

I thought it was bad because I'm not sure what unintended consequences
calling SmEditWindow::SelectAll() would have. I might be worried for
nothing though.

It doesn't seem to make it 2 steps.

I don't know why it doesn't pass the tests, I didn't look (I would guess
something doesn't get initialized right or at all).
I've got this instead of the 2 SmGetActiveView... lines which seems to
work without breaking the tests:

    pDocShell->GetEditEngine().QuickInsertText(formula, ESelection( 0,
0, 0xFFFF, 0xFFFF ));
    pDocShell->GetEditEngine().QuickFormatDoc();

I don't know why it is called quick, perhaps because it doesn't update
the text in the window. I saw QuickFormat called after it sometimes,
which updates the text in the window.
ESelection is how it is in SmEditWindow::SelectAll(). Perhaps there is
something better to use than 0xFFFF (maybe EE_PARA_ALL is the right
one?).

Perhaps it's a little better, any thoughts?

> Don't be, it also happens to me... That's why the visual formula
> editor had bugs to begin with, and it's no unlikely that we'll find
> more :)
> Besides if we don't propose patches, or partially working fixes... and
> discuss these we'll never get undo integration to work properly.

Thanks for the encouragement, it makes me feel a lot better about
this. :)

Regards,
Luke