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

2011-02-16 Thread Jonas Finnemann Jensen 
Hi Luke,

Here's the redo of this patch :)

Cool, pushed... :)
This is a nice step towards getting it stable... If you like searching code
and guessing what might be the right solution, copy/paste integration could
be next...


By the way, let me know if you've any ideas on how formatting should work,
without rewriting the node structure, an run into a problem with maintaining
the command text language...

--
Regards Jonas Finnemann Jensen.


On Wed, Feb 16, 2011 at 00:16, Luke Dixon <6b8b4...@gmail.com> wrote:

> Hi Jonas,
>
> On Tue, 2011-02-15 at 21:51 +0100, Jonas Finnemann Jensen wrote:
>
> > Well, lets just think about writing a note for the next guy, if we
> > find a solution :)
>
> I suppose the EditEngine::SetText functions would be be a good place to
> start.
>
> >
> > I noticed that 0x 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...
>
> It's a small matter, but not sure it is quite right as the 4th argument
> but at least there is other code that can be pointed at which does this.
>
> >
> > 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... :)
>
> Here's the redo of this patch :)
>
> Thanks for all your help :) might not have got there otherwise. Let's
> hope it will be okay.
>
> Regards,
> Luke
>
>
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

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

2011-02-15 Thread Luke Dixon 
Hi Jonas,

On Tue, 2011-02-15 at 21:51 +0100, Jonas Finnemann Jensen wrote:

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

I suppose the EditEngine::SetText functions would be be a good place to
start.

> 
> I noticed that 0x 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...

It's a small matter, but not sure it is quite right as the 4th argument
but at least there is other code that can be pointed at which does this.

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

Here's the redo of this patch :)

Thanks for all your help :) might not have got there otherwise. Let's
hope it will be okay.

Regards,
Luke

>From e39c1f9efe25e62bae09f3c0aeabaa16db174fb8 Mon Sep 17 00:00:00 2001
From: Luke Dixon <6b8b4...@gmail.com>
Date: Tue, 15 Feb 2011 22:46:54 +
Subject: [PATCH] Make Undo & Redo work with the visual formula editor

and update todo list.
---
 starmath/source/cursor.cxx   |3 ++-
 starmath/source/document.cxx |1 +
 starmath/visual-editor-todo  |   13 ++---
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/starmath/source/cursor.cxx b/starmath/source/cursor.cxx
index 8685ee5..e9eb200 100644
--- a/starmath/source/cursor.cxx
+++ b/starmath/source/cursor.cxx
@@ -1461,7 +1461,8 @@ void SmCursor::EndEdit(){
 SmNodeToTextVisitor(pTree, formula);
 //pTree->CreateTextFromNode(formula);
 pDocShell->aText = formula;
-pDocShell->GetEditEngine().SetText(formula);
+pDocShell->GetEditEngine().QuickInsertText( formula, ESelection( 0, 0, EE_PARA_ALL, EE_PARA_ALL ) );
+pDocShell->GetEditEngine().QuickFormatDoc();
 }
 
 void SmCursor::RequestRepaint(){
diff --git a/starmath/source/document.cxx b/starmath/source/document.cxx
index d98c406..eddbe9b 100644
--- a/starmath/source/document.cxx
+++ b/starmath/source/document.cxx
@@ -1174,6 +1174,7 @@ void SmDocShell::Execute(SfxRequest& rReq)
 (pTmpUndoMgr->*fnDo)( 0 );
 }
 Repaint();
+UpdateText();
 SfxViewFrame* pFrm = SfxViewFrame::GetFirst( this );
 while( pFrm )
 {
diff --git a/starmath/visual-editor-todo b/starmath/visual-editor-todo
index b19c1e7..3be3fd2 100644
--- a/starmath/visual-editor-todo
+++ b/starmath/visual-editor-todo
@@ -36,10 +36,9 @@ Complex
 
 Complex and non-essential
 -
-1. Global clipboard integration
-2. Support undo/redo with UndoManager integration
-3. Consider improving GUI for "Formula Elements"-dialog, most buttons work with visual editor
-4. Consider allowing users to enter commands in visual editor, by prefixing the command...
-5. Optimize things, for instance SmCursor::AnnotateSelection() is called way too many places...
-6. Improve handling of MoveUp and MoveDown in SmCursor::Move, SmCaretPos2LineVisitor might need improvement.
-7. Synchronized command text caret and visual editor caret.
+* Global clipboard integration
+* Consider improving GUI for "Formula Elements"-dialog, most buttons work with visual editor
+* Consider allowing users to enter commands in visual editor, by prefixing the command...
+* Optimize things, for instance SmCursor::AnnotateSelection() is called way too many places...
+* Improve handling of MoveUp and MoveDown in SmCursor::Move, SmCaretPos2LineVisitor might need improvement.
+* Synchronized command text caret and visual editor caret.
-- 
1.7.4

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

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

2011-02-15 Thread 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,
> 0x, 0x ));
>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 0x (maybe EE_PARA_ALL is the right
> one?).

I noticed that 0x 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 <6b8b4...@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, 0x, 0x ));
>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 0x (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
>
>
>
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

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

2011-02-14 Thread Luke Dixon 
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, 0x, 0x ));
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 0x (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


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

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

2011-02-14 Thread Jonas Finnemann Jensen 
hi Luke,

I should have known better than to think it was that easy. :)

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 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'm very sorry for bothering you with solutions that don't work.
>
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.

--
Regards Jonas Finnemann Jensen.


On Mon, Feb 14, 2011 at 01:13, Luke Dixon <6b8b4...@gmail.com> wrote:

> Hi Jonas,
>
> On Sun, 2011-02-13 at 21:44 +0100, Jonas Finnemann Jensen wrote:
>
> > 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...
> >
>
> Ah, I guess I got a bit too excited too quickly. I should have known
> better than to think it was that easy. :)
>
> >
> > 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).
> >
>
> Yeah, I saw nPara, and thought about it for a couple of seconds, figured
> that since SmNodeToTextVisitor puts it all on one line it would be okay.
> I thought that through badly :(
>
> >
> > 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...
> >
>
> 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);
>
> but it would really need to be done properly.
>
> >
> > 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 :)
> >
>
> Oh yeah, forgot about the formatting. Yes, I agree. I guess I just got a
> little excited.
>
> I'm very sorry for bothering you with solutions that don't work.
> Thanks for putting up with me. :)
>
> Regards,
> Luke
>
>
>
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

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

2011-02-13 Thread Luke Dixon 
Hi Jonas,

On Sun, 2011-02-13 at 21:44 +0100, Jonas Finnemann Jensen wrote:

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

Ah, I guess I got a bit too excited too quickly. I should have known
better than to think it was that easy. :)

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

Yeah, I saw nPara, and thought about it for a couple of seconds, figured
that since SmNodeToTextVisitor puts it all on one line it would be okay.
I thought that through badly :(

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

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);

but it would really need to be done properly.

> 
> 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 :)
> 

Oh yeah, forgot about the formatting. Yes, I agree. I guess I just got a
little excited.

I'm very sorry for bothering you with solutions that don't work.
Thanks for putting up with me. :)

Regards,
Luke


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

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

2011-02-13 Thread Jonas Finnemann Jensen 
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 <6b8b4...@gmail.com> wrote:

> Hi Michael, Jonas,
>
> On Fri, 2011-02-11 at 10:15 +, 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
>
>
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

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

2011-02-13 Thread Luke Dixon 
Hi Michael, Jonas,

On Fri, 2011-02-11 at 10:15 +, 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
>From 9851ff6a87a3701fa5387aeccc1a7fdd3dd05e07 Mon Sep 17 00:00:00 2001
From: Luke Dixon <6b8b4...@gmail.com>
Date: Sun, 13 Feb 2011 16:28:22 +
Subject: [PATCH] Make Undo & Redo work with the visual formula editor.

---
 starmath/source/cursor.cxx   |2 +-
 starmath/source/document.cxx |1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/starmath/source/cursor.cxx b/starmath/source/cursor.cxx
index 33ae549..4e65f48 100644
--- a/starmath/source/cursor.cxx
+++ b/starmath/source/cursor.cxx
@@ -1461,7 +1461,7 @@ void SmCursor::EndEdit(){
 SmNodeToTextVisitor(pTree, formula);
 //pTree->CreateTextFromNode(formula);
 pDocShell->aText = formula;
-pDocShell->GetEditEngine().SetText(formula);
+pDocShell->GetEditEngine().SetText(0, formula);
 }
 
 void SmCursor::RequestRepaint(){
diff --git a/starmath/source/document.cxx b/starmath/source/document.cxx
index d98c406..eddbe9b 100644
--- a/starmath/source/document.cxx
+++ b/starmath/source/document.cxx
@@ -1174,6 +1174,7 @@ void SmDocShell::Execute(SfxRequest& rReq)
 (pTmpUndoMgr->*fnDo)( 0 );
 }
 Repaint();
+UpdateText();
 SfxViewFrame* pFrm = SfxViewFrame::GetFirst( this );
 while( pFrm )
 {
-- 
1.7.4

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice