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

[Jonas Finnemann Jensen]

Hi Luke,

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.

Well, lets just think about writing a note for the next guy, if we find a
solution :)

   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?).

I noticed that 0xFFFF is used in SelectAll... but searching for EE_PARA_ALL,
gives some results where it's used for creating selections, so that's
probably what it's a constant for...


> Perhaps it's a little better, any thoughts?

I think we finally have a solution... I just tried it out, looks to me as if
it's working, without any nasty side effects...
I noticed that QuickInsertText and QuickFormatDoc was used in
some accessibility code, so it's probably okay to use them..

Unless, you've other ideas, I suggest don't you update your patch, then I'll
push it... And please update the todo-list, you've just fixed one of the
complicated issues... :)

--
Regards Jonas Finnemann Jensen.


On Tue, Feb 15, 2011 at 02:03, Luke Dixon <6b8b4567 at gmail.com> wrote:

> 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20110215/b04e7150/attachment.htm>