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

Hi Luke,

It seems that undo/redo is the major one and I just worked out why
> undo/redo wasn't working, noticed it while looking at Caolán's unit
> tests for starmath. I've attached the patch.

It would be major step to get that issue fixed... And you're going the right
way, but your patch doesn't work if the formula is written in multiple
lines...
try, writing the formula 3+3+3 \n \n \n \n +4+4 (replace \n by line break
not NEWLINE but whitespace linebreak).
Then in visual editor delete +3, go into the command editor again and back
into the visual editor and notice that it now says 3+3+4+4+4+4.
The command editor will say  { 3 + 3 + 4 + 4 }  \n \n \n \n +4+4, because
the SetText(0, formula) only replaced the first paragraph...

What I understand from editeng_8.cxx line 1621 (see [1]) is that the number
you pass in setText(0, formula), is the paragraph to be replaced by the
text...
The method signature is EditEngine::SetText( sal_uInt16 nPara, const
XubString& rTxt ), and it calls SelectParagraph(nPara)........
At least that's what I can decrypt from the sources... (I think I've seen
ROT13 variants that was easier to read).

I have no idea how to solve this... Maybe we can
call EditEngine::UndoActionStart(USHORT nId) (see [2])
and EditEngine::UndoActionEnd(USHORT nId), before and after the called to
EditEngine::SetText(formula)...
But in that case I don't know if we can pass any value as nId, the existing
ones are defined in [3] (line 64).
The only place I can see EDITUNDO_INSERT used is to call UndoActionStart/End
and in one switch case where labels are generated...

But again, there's no documentation, what so ever! So unless we can find the
guy who wrote it, maybe we should just try something and hope it doesn't
introduce a bug somewhere we don't notice...

[1] http://docs.libreoffice.org/editeng/html/editeng_8cxx-source.html#l01621
[2]
http://docs.libreoffice.org/editeng/html/classEditEngine.html#c7a0ff1067cb9e6054be7cfaaa91872c
[3]
http://docs.libreoffice.org/editeng/html/editdata_8hxx-source.html#l00064

It would be great to have Jonas' great work on by default, and with
> undo/redo, perhaps it is now possible?

Undo/redo would be a great step in the right direction, if we don't have it
users will probably consider it a regression...
I recently pushed line deletion using backspace... Now I just need to
implement it using delete...
But apart from that the visual editor does still discards bold, font, color
and alignment information, it wouldn't be hard to retain it... But it would
be hard to edit it intuitively using visual editor, which is why it just
discards this now...
But I would imagine that some people would get rather mad, if we enable a
feature that removes formatting from their formulas...

We can probably enable the visual editor as an non-default editing mode
(i.e. activated from toolbar) once the formatting issue is addressed... Then
when we've integrated it properly, so that users of the command editor
doesn't feel any uncomfort, we can make it default...
But as long as it discards formatting information (font, bold, color,
alignment, etc) we probably shouldn't enable it by default... Users hate
regressions, especially if it eats all their formatting information without
asking nicely first :)

--
Regards Jonas Finnemann Jensen.


On Sun, Feb 13, 2011 at 18:12, Luke Dixon <6b8b4567@gmail.com> wrote:

> Hi Michael, Jonas,
>
> On Fri, 2011-02-11 at 10:15 +0000, Michael Meeks wrote:
> >       Quick question, while I'm here: what are the blockers for enabling
> this
> > by default on master ? was it undo/redo ? or do we have any malingering
> > known crashers / mis-features left.
>
> I think Jonas fixed the crash I noticed.
> There are 2 more bugs at bugzilla.freedesktop.org, I don't think they
> would be too difficult to fix.
>
> It seems that undo/redo is the major one and I just worked out why
> undo/redo wasn't working, noticed it while looking at Caolán's unit
> tests for starmath. I've attached the patch.
>
> I made it also update the formula when undoing or redoing stuff. I
> didn't enable updating only as an experimental feature, as I don't quite
> get why it doesn't update already. It looked a little bad when undoing
> from the visual formula editor. I hope it will be okay :)
>
> I noticed that when undoing the cursor position goes to the front of the
> formula, not sure that is too easy to fix, but I figure having undo is
> better than not. Perhaps I could update the Easy Hack to be for this
> instead.
>
> It would be great to have Jonas' great work on by default, and with
> undo/redo, perhaps it is now possible?
>
> Regards,
> Luke
>
> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice