[Libreoffice-commits] core.git: starmath/inc starmath/source

[Luke Dixon (via logerrit)]

 starmath/inc/caret.hxx |   13 ++---
 starmath/inc/cursor.hxx|3 +--
 starmath/source/cursor.cxx |   17 ++---
 starmath/source/view.cxx   |   16 
 4 files changed, 17 insertions(+), 32 deletions(-)

New commits:
commit 5267b6c04eed2662726bb90899eb40414779fcb3
Author: Luke Dixon 
AuthorDate: Tue Sep 8 01:37:23 2020 +0100
Commit: Noel Grandin 
CommitDate: Tue Sep 8 08:17:59 2020 +0200

starmath: stop change to caret pos graph when moving after brace

Currently, the code that moves the cursor after the brace changes the
current caret pos instead of moving to the next pos. Because of this, we
end up with one missing graph pos and a duplicate graph pos. This can be
seen by moving to the end of a brace body, pressing the key for the
closing brace and then pressing left and right to move in and out of the
brace.

This fix uses the existing code that sets the cursor position to the
next element in the graph instead of changing the cursor pos directly.
It also marks the caret pos as const to suggest not changing the caret
pos inside a graph entry.

Change-Id: I5492e54f1bddbfc90036f29698982fd8696f5e88
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102214
Tested-by: Jenkins
Reviewed-by: Noel Grandin 

diff --git a/starmath/inc/caret.hxx b/starmath/inc/caret.hxx
index 327ee1d6ed16..c0e5c4f5fca0 100644
--- a/starmath/inc/caret.hxx
+++ b/starmath/inc/caret.hxx
@@ -103,15 +103,14 @@ private:
 
 /** An entry in SmCaretPosGraph */
 struct SmCaretPosGraphEntry{
-SmCaretPosGraphEntry(SmCaretPos pos,
- SmCaretPosGraphEntry* left,
- SmCaretPosGraphEntry* right) {
-CaretPos = pos;
-Left = left;
-Right = right;
+SmCaretPosGraphEntry(SmCaretPos pos, SmCaretPosGraphEntry* left, 
SmCaretPosGraphEntry* right)
+: CaretPos{pos}
+, Left{left}
+, Right{right}
+{
 }
 /** Caret position */
-SmCaretPos CaretPos;
+const SmCaretPos CaretPos;
 /** Entry to the left visually */
 SmCaretPosGraphEntry* Left;
 /** Entry to the right visually */
diff --git a/starmath/inc/cursor.hxx b/starmath/inc/cursor.hxx
index 236485d5e04c..47218e490865 100644
--- a/starmath/inc/cursor.hxx
+++ b/starmath/inc/cursor.hxx
@@ -184,8 +184,7 @@ public:
 /** Draw the caret */
 void Draw(OutputDevice& pDev, Point Offset, bool isCaretVisible);
 
-bool IsAtTailOfBracket(SmBracketType eBracketType, SmBraceNode** 
ppBraceNode) const;
-void MoveAfterBracket(SmBraceNode* pBraceNode);
+bool IsAtTailOfBracket(SmBracketType eBracketType) const;
 
 private:
 friend class SmDocShell;
diff --git a/starmath/source/cursor.cxx b/starmath/source/cursor.cxx
index 5e69b5876a45..c476bd41228e 100644
--- a/starmath/source/cursor.cxx
+++ b/starmath/source/cursor.cxx
@@ -1327,7 +1327,8 @@ void SmCursor::RequestRepaint(){
 }
 }
 
-bool SmCursor::IsAtTailOfBracket(SmBracketType eBracketType, SmBraceNode** 
ppBraceNode) const {
+bool SmCursor::IsAtTailOfBracket(SmBracketType eBracketType) const
+{
 const SmCaretPos pos = GetPosition();
 if (!pos.IsValid()) {
 return false;
@@ -1391,23 +1392,9 @@ bool SmCursor::IsAtTailOfBracket(SmBracketType 
eBracketType, SmBraceNode** ppBra
 return false;
 }
 
-if (ppBraceNode) {
-*ppBraceNode = pBraceNode;
-}
-
 return true;
 }
 
-void SmCursor::MoveAfterBracket(SmBraceNode* pBraceNode)
-{
-mpPosition->CaretPos.pSelectedNode = pBraceNode;
-mpPosition->CaretPos.nIndex = 1;
-mpAnchor->CaretPos.pSelectedNode = pBraceNode;
-mpAnchor->CaretPos.nIndex = 1;
-RequestRepaint();
-}
-
-
 /// SmNodeListParser
 
 SmNode* SmNodeListParser::Parse(SmNodeList* list){
diff --git a/starmath/source/view.cxx b/starmath/source/view.cxx
index 19274324ada7..dd967232723b 100644
--- a/starmath/source/view.cxx
+++ b/starmath/source/view.cxx
@@ -490,7 +490,6 @@ void SmGraphicWindow::KeyInput(const KeyEvent& rKEvt)
 default:
 {
 sal_Unicode code = rKEvt.GetCharCode();
-SmBraceNode* pBraceNode = nullptr;
 
 if(code == ' ') {
 rCursor.InsertElement(BlankElement);
@@ -506,13 +505,14 @@ void SmGraphicWindow::KeyInput(const KeyEvent& rKEvt)
 rCursor.InsertElement(FactorialElement);
 }else if(code == '%') {
 rCursor.InsertElement(PercentElement);
-}else if(code == ')' && 
rCursor.IsAtTailOfBracket(SmBracketType::Round, )) {
-rCursor.MoveAfterBracket(pBraceNode);
-}else if(code == ']' && 
rCursor.IsAtTailOfBracket(SmBracketType::Square, )) {
-rCursor.MoveAfterBracket(pBraceNode);
-}else if(code == '}' && 
rCursor.IsAtTailOfBracket(SmBracketType::Curly, )) {
-   

[Libreoffice-commits] core.git: starmath/source

[Luke Dixon (via logerrit)]

 starmath/source/cursor.cxx |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

New commits:
commit c13671108fa7cee0cc1801e4c08800de1daab850
Author: Luke Dixon 
AuthorDate: Sun Aug 30 13:35:20 2020 +0100
Commit: Noel Grandin 
CommitDate: Wed Sep 2 20:55:25 2020 +0200

Fix crash when entering newline in starmath experimental editor

Currently things crash when adding a newline using the experimental
editor in Math Formula editor. It looks like this is happening because
the newly inserted line was getting destroyed when the unique_ptr goes
out of scope.

This change releases the unique_ptr so it doesn't get destroyed. As it
is now part of the tree it should be destroyed elsewhere.

Change-Id: Icfe2c0289c12302d39883dc04cef0351467aa845
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101737
Tested-by: Jenkins
Reviewed-by: Noel Grandin 

diff --git a/starmath/source/cursor.cxx b/starmath/source/cursor.cxx
index 2aae7adb555e..5e69b5876a45 100644
--- a/starmath/source/cursor.cxx
+++ b/starmath/source/cursor.cxx
@@ -765,7 +765,7 @@ bool SmCursor::InsertRow() {
 pTable->SetSubNode(i, pTable->GetSubNode(i-1));
 
 //Insert new line
-pTable->SetSubNode(nTableIndex + 1, pNewLine.get());
+pTable->SetSubNode(nTableIndex + 1, pNewLine.release());
 
 //Check if we need to change token type:
 if(pTable->GetNumSubNodes() > 2 && pTable->GetToken().eType == TBINOM) 
{
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

blanket license statement

[Luke Dixon]

All of my past  future contributions to LibreOffice may be licensed
under the MPL/LGPLv3+ dual license

Regards,
Luke Dixon
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

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

[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

[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

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

[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

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

[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] [REVIEW] Re: SmNodeToTextVisitor Fixes

[Luke Dixon]

Hello Michael, Jonas, Caolán,

   Great - pushed there;
 
   Thanks,
 
   Michael.
 

I hope I'm not waffling, but I'm very thankful to you guys for looking
at this. And thanks for taking care of closing the bugs.

Thanks,
Luke

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

Re: [Libreoffice] [PUSHED] Re: SmNodeToTextVisitor Fixes

[Luke Dixon]

Hello Jonas, Michael, list,

Sorry for not replying sooner.

On Tue, 2011-02-01 at 19:32 +0100, Jonas Finnemann Jensen wrote:
 I don't think it's critial... None of the commits that removes
 brackets and improves SmNodeToTextVisitor have been applied to
 libreoffice-3-3... The SmNodeToTextVisitor there uses a lot of
 brackets, so there's hopefully no major issues with it...

I guess I'm too late now, so maybe for 3.3.2, but maybe there are a
couple of bugs that could be fixed for libreoffice-3-3.
We could fix two of the bugs, 32759 and 32755 on the freedesktop.org
bugzilla, as they are quite simple and seem to be a fixable by removing
one character and one line (which is how it is now on master now). It
seems so simple I wonder if I'm missing something about these two?

Other than that I think Jonas is right.

Regards,
Luke
From 37084a642a789a1788783c0a3db708396e16197a Mon Sep 17 00:00:00 2001
From: Luke Dixon 6b8b4...@gmail.com
Date: Tue, 8 Feb 2011 20:15:21 +
Subject: [PATCH] Fix for bugs 32759 and 32755 on freedesktop bugzilla

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

diff --git a/starmath/source/visitors.cxx b/starmath/source/visitors.cxx
index dc5dd38..7b7dafe 100644
--- a/starmath/source/visitors.cxx
+++ b/starmath/source/visitors.cxx
@@ -2131,7 +2131,7 @@ void SmNodeToTextVisitor::Visit( SmTableNode* pNode )
 LineToText( it.Current( ) );
 if( it.Next( ) ) {
 Separate( );
-Append( ##  );
+Append( #  );
 }else
 break;
 }
@@ -2447,7 +2447,6 @@ void SmNodeToTextVisitor::Visit( SmTextNode* pNode )
 
 void SmNodeToTextVisitor::Visit( SmSpecialNode* pNode )
 {
-Append( % );
 Append( pNode-GetToken( ).aText );
 }
 
-- 
1.7.4

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

Re: [Libreoffice] starmath / cppunit breakage in master

[Luke Dixon]

Hello,

I'm very sorry the code I was playing with has caused problems.

I guess I wasn't thinking when adding the first header outside the
guards.

The assertion_traits struct, I was expecting this to be a problem, but I
didn't know what to do about it, sorry.

Apologies,
Luke

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

Re: [Libreoffice] [PUSHED] Re: SmNodeToTextVisitor Fixes

[Luke Dixon]

Hi Jonas,

I'm very sorry for not responding very quickly. And I'm very sorry for
sending pretty much the same message twice, I forgot to CC the list.

 That's not all true... I added all the brackets initially because I
 had some problems with a few isolated things...
 Try writing binom a b + c in the command text field... Then enter
 visual editor an move as much to the right as possible, e.g. the
 rightmost position in the toplevel line, and write + d
 Now +d will be in the toplevel line... But if you enter the command
 text editor and changes c to e, then +d will be in the second line of
 the binom...
 I don't think this is parsing error... But an unpleasant obscurity in
 the format... I'm not even sure I fixed this one with my excessive use
 of brackets... But the issue is there now... We should probably fix it
 and add a unit test for it to avoid regressions...
 The problem is that when binom has these obscurities maybe some of the
 other command have similar obscurities... :(

Yep, I'm very sorry for breaking things. I've attached a patch for this
particular problem (with a test).

In trying to work out the correct form for binoms, I found that even
binom {a} {b + c} + d doesn't seem to work when it goes through the
parser, so I guess it needs to be { binom {a} {b + c} } + d.

I've noticed other stuff I've messed up though, so I'm going to continue
on this some more :)

 Btw, you latest patch didn't introduce this issue... So it's probably
 been there for a while...
 
  Oh, another thing I noticed was that there seems to be a crash when
  moving to the right of an SmPlaceNode and deleting it
 Interesting... I've had that bug once, but I've never been able to
 reproduce it... Can't this time either... Do you have any more
 specific steps ?

I described this pretty badly, sorry, the steps I use are:
1. type something in the visual editor eg. A + B = C
2. press enter to go to a new line
3. press the right arrow key to move to the right of the place node
When I do this the underline that goes with the caret moves up a little
bit and the caret gets shorter.
4. press enter or backspace and it crashes (every time for me)

I really haven't looked at it any further at this point. The
libreoffice-3-3 version seems to work fine though.

Regards,
Luke
From a4a4205c85373ba1a26f8cf5930aaba483172ae4 Mon Sep 17 00:00:00 2001
From: Luke Dixon 6b8b4...@gmail.com
Date: Sun, 19 Dec 2010 15:24:46 +
Subject: [PATCH] Put brackets around binoms in SmNodeToTextVisitor, with test

---
 starmath/qa/cppunit/test_nodetotextvisitors.cxx |   37 +++
 starmath/source/visitors.cxx|3 +-
 2 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/starmath/qa/cppunit/test_nodetotextvisitors.cxx b/starmath/qa/cppunit/test_nodetotextvisitors.cxx
index 45386d1..e5b9e93 100644
--- a/starmath/qa/cppunit/test_nodetotextvisitors.cxx
+++ b/starmath/qa/cppunit/test_nodetotextvisitors.cxx
@@ -41,6 +41,7 @@
 #include document.hxx
 #include node.hxx
 #include visitors.hxx
+#include cursor.hxx
 
 #include preextstl.h
 #include cppunit/TestSuite.h
@@ -72,6 +73,14 @@ struct assertion_traitsString
 SO2_DECL_REF(SmDocShell)
 SO2_IMPL_REF(SmDocShell)
 
+class TestOutputDevice : public OutputDevice
+{
+public:
+TestOutputDevice()
+{
+}
+};
+
 using namespace ::com::sun::star;
 
 namespace {
@@ -95,6 +104,7 @@ public:
 void SimpleFormats();
 void SimpleGreekChars();
 void SimpleSpecialChars();
+void testBinomInBinHor();
 
 CPPUNIT_TEST_SUITE(Test);
 CPPUNIT_TEST(SimpleUnaryOp);
@@ -109,6 +119,7 @@ public:
 CPPUNIT_TEST(SimpleFormats);
 CPPUNIT_TEST(SimpleGreekChars);
 CPPUNIT_TEST(SimpleSpecialChars);
+CPPUNIT_TEST(testBinomInBinHor);
 CPPUNIT_TEST_SUITE_END();
 
 private:
@@ -480,6 +491,32 @@ void Test::parseandparseagain(const char *formula, const char *test_name)
 delete pNode2;
 }
 
+void Test::testBinomInBinHor()
+{
+String sInput, sExpected, sOutput;
+SmNode* pTree;
+
+// set up a binom (table) node
+sInput.AppendAscii(binom a b + c);
+pTree = SmParser().Parse(sInput);
+pTree-Prepare(xDocShRef-GetFormat(), *xDocShRef);
+
+SmCursor aCursor(pTree, xDocShRef);
+TestOutputDevice aOutputDevice;
+
+// move forward (more than) enough places to be at the end
+int i;
+for (i = 0; i  8; ++i)
+aCursor.Move(aOutputDevice, MoveRight);
+
+// tack +d on the end, which will put the binom into an SmBinHorNode
+aCursor.InsertElement(PlusElement);
+aCursor.InsertText('d');
+
+sExpected.AppendAscii( { { binom a b + c } + d } );
+CPPUNIT_ASSERT_EQUAL_MESSAGE(Binom Node in BinHor Node, sExpected, xDocShRef-GetText());
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(Test);
 
 }
diff --git a/starmath/source/visitors.cxx b/starmath/source/visitors.cxx
index 2e7e4e3..fbdb379 100644
--- a/starmath/source/visitors.cxx
+++ b/starmath/source/visitors.cxx
@@ -2163,9 +2163,10 @@ void

Re: [Libreoffice] [PUSHED] Re: SmNodeToTextVisitor Fixes

[Luke Dixon]

Hello Jonas,

Thanks for accepting these patches.

 Nice work! I didn't know there was bugs in SmNodeToTextVisitor, but
 somehow it doesn't surprise me...
 The format is slightly obscure and the visitor was in need of some love...

Sometimes the smallest bugs are the hardest to believe. There are times
I can't believe that I've missed such simple stuff. Speaking of which
I've attached another patch for the test I sent :| (though I doubt leaks
in a test matter much).

 By the way, I really like to unit tests... That is a brilliant idea,
 unit tests are perfect for this kind of thing.
 Must admit I had to ask around to figure out how to run the unit tests...
 I have a visitor to testing the visitor implementation, maybe I should
 use that in a unit test too...
 I wonder if there's anything else we could write unit tests for...
 Anyway, it's a good thing to keep in mind..

Ever since I saw that other test added there a couple of weeks ago, I've
been feeling guilty about it and kept wondering if it was put there as a
hint.

Thanks again,
Luke
From aac1d002c9e47238217c2ed13de5b2a9770075ed Mon Sep 17 00:00:00 2001
From: Luke Dixon 6b8b4...@gmail.com
Date: Wed, 24 Nov 2010 17:06:10 +
Subject: [PATCH] Fix a leak  remove leftover cout that I missed

---
 starmath/qa/cppunit/test_nodetotextvisitors.cxx |7 +++
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/starmath/qa/cppunit/test_nodetotextvisitors.cxx b/starmath/qa/cppunit/test_nodetotextvisitors.cxx
index 93def14..cd8aee0 100644
--- a/starmath/qa/cppunit/test_nodetotextvisitors.cxx
+++ b/starmath/qa/cppunit/test_nodetotextvisitors.cxx
@@ -5,9 +5,6 @@
 
 #include sal/config.h
 
-#include iostream
-using namespace std;
-
 #include cppuhelper/bootstrap.hxx
 #include comphelper/processfactory.hxx
 #include cppunit/TestAssert.h
@@ -41,7 +38,6 @@ struct assertion_traitsString
 std::string text = ByteString(x, RTL_TEXTENCODING_UTF8).GetBuffer();
 OStringStream ost;
 ost  text;
-cout  ost.str();
 return ost.str();
 }
 };
@@ -453,6 +449,9 @@ void Test::parseandparseagain(const char *formula, const char *test_name)
 CPPUNIT_ASSERT_EQUAL_MESSAGE(test_name,
 output1,
 output2);
+
+delete pNode1;
+delete pNode2;
 }
 
 CPPUNIT_TEST_SUITE_REGISTRATION(Test);
-- 
1.7.3.2

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

Re: [Libreoffice] SmNodeToTextVisitor Fixes

[Luke Dixon]

Hello again,

Sorry, forgot something.
 3. Some of the subs  sups seemed to need extra brackets when used
with
 an operator node (for example int or sum).
4. Matrices with blank elements seem to cause problems so I added more
brackets (supposed to be removing them :( ).

Also, I've made the code used to find these into cppunit tests. I took
all the formula commands from:
http://wiki.services.openoffice.org/wiki/Template:Math_commands_reference

This page is licensed with the Creative Common Attribution 3.0 license
(CC-BY), so I put a note in the code with all the contributors names
(Hopefully that covers it).

What the code does is parses the formula command, runs the
NodeToTextVisitor on it, then parses it again, then visits again.
It occurred to me that the NodeToTextVisitor should output the same text
both times.

It was quite fun writing this and learning about the different string
types that are used and working out how to do this without putting
printf all over the place.
I didn't see any of the other tests doing like this with the
assertion_traits template and CPPUNIT_ASSERT_EQUAL_MESSAGE, so I hope I
haven't gone too far.

Regards,
Luke

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

Re: [Libreoffice] SmNodeToTextVisitor Fixes

[Luke Dixon]

 Sorry, forgot something.
And of course I forgot to add the attachments :(

From 1313f82a72ca5c361eead3a86d55f3be394514e0 Mon Sep 17 00:00:00 2001
From: Luke Dixon 6b8b4...@gmail.com
Date: Tue, 23 Nov 2010 16:17:23 +
Subject: [PATCH] Fix matrices with blank elements.

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

diff --git a/starmath/source/visitors.cxx b/starmath/source/visitors.cxx
index ab2ac87..3b9bdef 100644
--- a/starmath/source/visitors.cxx
+++ b/starmath/source/visitors.cxx
@@ -2466,9 +2466,11 @@ void SmNodeToTextVisitor::Visit( SmMatrixNode* pNode )
 for ( USHORT i = 0; i  pNode-GetNumRows( ); i++ ) {
 for ( USHORT j = 0; j  pNode-GetNumCols( ); j++ ) {
 SmNode* pSubNode = pNode-GetSubNode( i * pNode-GetNumCols( ) + j );
+Append( { );
 Separate( );
 pSubNode-Accept( this );
 Separate( );
+Append( } );
 if( j != pNode-GetNumCols( ) - 1 )
 Append( # );
 }
-- 
1.7.3.2

From c37501af6c442eb4bef6a5914e83ed1d8e6fe029 Mon Sep 17 00:00:00 2001
From: Luke Dixon 6b8b4...@gmail.com
Date: Tue, 23 Nov 2010 16:49:15 +
Subject: [PATCH] Add some tests for SmNodeToTextVisitor.

---
 starmath/qa/cppunit/makefile.mk |1 +
 starmath/qa/cppunit/test_nodetotextvisitors.cxx |  462 +++
 2 files changed, 463 insertions(+), 0 deletions(-)
 create mode 100644 starmath/qa/cppunit/test_nodetotextvisitors.cxx

diff --git a/starmath/qa/cppunit/makefile.mk b/starmath/qa/cppunit/makefile.mk
index 7bdc6c8..212cb8b 100644
--- a/starmath/qa/cppunit/makefile.mk
+++ b/starmath/qa/cppunit/makefile.mk
@@ -48,6 +48,7 @@ CFLAGSCXX += $(CPPUNIT_CFLAGS)
 
 SHL1OBJS=  \
 $(SLO)/test_starmath.obj \
+$(SLO)/test_nodetotextvisitors.obj
 
 
 SHL1STDLIBS= \
diff --git a/starmath/qa/cppunit/test_nodetotextvisitors.cxx b/starmath/qa/cppunit/test_nodetotextvisitors.cxx
new file mode 100644
index 000..93def14
--- /dev/null
+++ b/starmath/qa/cppunit/test_nodetotextvisitors.cxx
@@ -0,0 +1,462 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+
+// MARKER(update_precomp.py): autogen include statement, do not remove
+#include precompiled_starmath.hxx
+
+#include sal/config.h
+
+#include iostream
+using namespace std;
+
+#include cppuhelper/bootstrap.hxx
+#include comphelper/processfactory.hxx
+#include cppunit/TestAssert.h
+
+#include vcl/svapp.hxx
+#include smdll.hxx
+
+#include document.hxx
+#include node.hxx
+#include visitors.hxx
+
+#include preextstl.h
+#include cppunit/TestSuite.h
+#include cppunit/TestFixture.h
+#include cppunit/TestCase.h
+#include cppunit/plugin/TestPlugIn.h
+#include cppunit/extensions/HelperMacros.h
+#include postextstl.h
+
+namespace CppUnit {
+template
+struct assertion_traitsString
+{
+static bool equal(const String x, const String y)
+{
+return x == y;
+}
+
+static std::string toString(const String x)
+{
+std::string text = ByteString(x, RTL_TEXTENCODING_UTF8).GetBuffer();
+OStringStream ost;
+ost  text;
+cout  ost.str();
+return ost.str();
+}
+};
+}
+
+SO2_DECL_REF(SmDocShell)
+SO2_IMPL_REF(SmDocShell)
+
+using namespace ::com::sun::star;
+
+namespace {
+
+class Test : public CppUnit::TestFixture {
+public:
+// init
+virtual void setUp();
+virtual void tearDown();
+
+// tests
+void SimpleUnaryOp();
+void SimpleBinaryOp();
+void SimpleRelationalOp();
+void SimpleSetOp();
+void SimpleFunctions();
+void SimpleOperators();
+void SimpleAttributes();
+void SimpleMisc();
+void SimpleBrackets();
+void SimpleFormats();
+void SimpleGreekChars();
+void SimpleSpecialChars();
+
+CPPUNIT_TEST_SUITE(Test);
+CPPUNIT_TEST(SimpleUnaryOp);
+CPPUNIT_TEST(SimpleBinaryOp);
+CPPUNIT_TEST(SimpleRelationalOp);
+CPPUNIT_TEST(SimpleSetOp);
+CPPUNIT_TEST(SimpleFunctions);
+CPPUNIT_TEST(SimpleOperators);
+CPPUNIT_TEST(SimpleAttributes);
+CPPUNIT_TEST(SimpleMisc);
+CPPUNIT_TEST(SimpleBrackets);
+CPPUNIT_TEST(SimpleFormats);
+CPPUNIT_TEST(SimpleGreekChars);
+CPPUNIT_TEST(SimpleSpecialChars);
+CPPUNIT_TEST_SUITE_END();
+
+private:
+uno::Reference uno::XComponentContext  m_context;
+SmDocShellRef xDocShRef;
+void parseandparseagain(const char *input, const char *test_name);
+};
+
+void Test::setUp()
+{
+m_context = cppu::defaultBootstrap_InitialComponentContext();
+
+uno::Referencelang::XMultiComponentFactory xFactory(m_context-getServiceManager());
+uno::Referencelang::XMultiServiceFactory xSM(xFactory, uno::UNO_QUERY_THROW);
+
+//Without this we're crashing because callees are using
+//getProcessServiceFactory.  In general those should be removed in favour
+//of retaining references to the root ServiceFactory as its passed around
+comphelper

Re: [Libreoffice] [PUSHED]Re: Blinking cursor progress

[Luke Dixon]

Hello Jonas, 
 I think the patch looks great, I've pushed it... But when we need to
 draw a thin solid line under the visual line that the caret is in, we
 will probably need to pass a boolean value to SmCursor::Draw in
 SmGraphicWindow::Paint and from SmCursor::Draw to
 SmCaretDrawingVisitor... So that only the vertical line blinks, but
 the solid underline of the visual line doesn't blink...
 This is as opposed to not calling SmCursor::Draw in
 SmGraphicWindow::Paint which is what currently happens...

Thanks very much for your help in pointing me in the right direction for
this.

I've attempted the other easy hack, this information meant I didn't have
to spend any time working out what to do.
I made a couple of methods for SmGraphicWindow, IsLineVisible and
ShowLine. I made them public, as I would imagine they would be used for
number 7 of the complex and non-essential tasks on the todo list.

 By the way, I think that it's really cool that you found the setting
 for the caret blink timer... Nice work!
That was all thanks to opengrok.go-oo.org (thanks for pointing that out
to me).


Regards,
Luke 
From 1bbb3fab0d224a6cae86dfdd0caf37056ddcf76c Mon Sep 17 00:00:00 2001
From: Luke Dixon 6b8b4...@gmail.com
Date: Tue, 16 Nov 2010 23:37:09 +
Subject: [PATCH] Draw a visible line under the current line in the visual formula editor.

---
 starmath/inc/cursor.hxx  |2 +-
 starmath/inc/view.hxx|6 --
 starmath/inc/visitors.hxx|3 ++-
 starmath/source/cursor.cxx   |4 ++--
 starmath/source/view.cxx |   14 --
 starmath/source/visitors.cxx |   38 +-
 6 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/starmath/inc/cursor.hxx b/starmath/inc/cursor.hxx
index 7c4fa85..139d5c4 100644
--- a/starmath/inc/cursor.hxx
+++ b/starmath/inc/cursor.hxx
@@ -226,7 +226,7 @@ public:
 static SmNode* FindTopMostNodeInLine(SmNode* pSNode, bool MoveUpIfSelected = false);
 
 /** Draw the caret */
-void Draw(OutputDevice pDev, Point Offset);
+void Draw(OutputDevice pDev, Point Offset, bool isCaretVisible);
 
 private:
 friend class SmDocShell;
diff --git a/starmath/inc/view.hxx b/starmath/inc/view.hxx
index cbaf501..e5899ce 100644
--- a/starmath/inc/view.hxx
+++ b/starmath/inc/view.hxx
@@ -57,11 +57,13 @@ class SmGraphicWindow : public ScrollableWindow
 // old style editing pieces
 Rectangle aCursorRect;
 bool  bIsCursorVisible;
-
-AutoTimer aCaretBlinkTimer;
+bool  bIsLineVisible;
+AutoTimer aCaretBlinkTimer;
 public:
 bool IsCursorVisible() const { return bIsCursorVisible; }
 void ShowCursor(bool bShow);
+bool IsLineVisible() const { return bIsLineVisible; }
+void ShowLine(bool bShow);
 const SmNode * SetCursorPos(USHORT nRow, USHORT nCol);
 protected:
 void		SetIsCursorVisible(bool bVis) { bIsCursorVisible = bVis; }
diff --git a/starmath/inc/visitors.hxx b/starmath/inc/visitors.hxx
index 2b89b6f..cff09dc 100644
--- a/starmath/inc/visitors.hxx
+++ b/starmath/inc/visitors.hxx
@@ -152,13 +152,14 @@ class SmCaretDrawingVisitor : public SmDefaultingVisitor
 {
 public:
 /** Given position and device this constructor will draw the caret */
-SmCaretDrawingVisitor( OutputDevice rDevice, SmCaretPos position, Point offset );
+SmCaretDrawingVisitor( OutputDevice rDevice, SmCaretPos position, Point offset, bool caretVisible );
 void Visit( SmTextNode* pNode );
 private:
 OutputDevice rDev;
 SmCaretPos pos;
 /** Offset to draw from */
 Point Offset;
+bool isCaretVisible;
 protected:
 /** Default method for drawing pNodes */
 void DefaultVisit( SmNode* pNode );
diff --git a/starmath/source/cursor.cxx b/starmath/source/cursor.cxx
index 948a02b..1219595 100644
--- a/starmath/source/cursor.cxx
+++ b/starmath/source/cursor.cxx
@@ -180,8 +180,8 @@ void SmCursor::AnnotateSelection(){
 SmSetSelectionVisitor(anchor-CaretPos, position-CaretPos, pTree);
 }
 
-void SmCursor::Draw(OutputDevice pDev, Point Offset){
-SmCaretDrawingVisitor(pDev, GetPosition(), Offset);
+void SmCursor::Draw(OutputDevice pDev, Point Offset, bool isCaretVisible){
+SmCaretDrawingVisitor(pDev, GetPosition(), Offset, isCaretVisible);
 }
 
 void SmCursor::Delete(){
diff --git a/starmath/source/view.cxx b/starmath/source/view.cxx
index 6d2752d..75871e6 100644
--- a/starmath/source/view.cxx
+++ b/starmath/source/view.cxx
@@ -116,6 +116,7 @@ SmGraphicWindow::SmGraphicWindow(SmViewShell* pShell):
 SetHelpId(HID_SMA_WIN_DOCUMENT);
 SetUniqueId(HID_SMA_WIN_DOCUMENT);
 
+ShowLine(false);
 CaretBlinkInit();
 }
 
@@ -224,6 +225,7 @@ void SmGraphicWindow::GetFocus()
 //Let view shell know what insertions should be done in visual editor
 pViewShell-SetInsertIntoEditWindow(false);
 SetIsCursorVisible(true);
+ShowLine(true);
 CaretBlinkStart();
 RepaintViewShellDoc();
 }
@@ -242,6 +244,7 @@ void SmGraphicWindow::LoseFocus

Re: [Libreoffice] [Pushed] Easy Task - systray build progress tracker

[Luke Dixon]

Hello Michael,

 
   Heh; I guess default_icons/ should have a bus-load of artwork in it.

I've looked there, I can't really see one that really does it. Maybe the
default icon is good enough.

   Wonderful - thanks; merged it now - I added the 'sleep' inside
 zenity_close - to avoid it being on the other code path.
Thanks, that makes sense. I saw the other commits by people that fixed
what I had broken, I'm very sorry for any trouble I've caused.

   And thank you for submitting a fix ! much appreciated; I've removed the
 task from the Easy Hacks wiki page to avoid overlap. What would really
 help is the finding of other easy hacks for others ;-) [ we are running
 shorter over time ... ]
I'll try and look out for some, but it would be a little difficult as
I'm new to this code.

   Of course, some more feature work precisely here might be nice ;-) eg.
 if we are building from an interactive terminal, on Unix, and we have
 DISPLAY= set, and we have zenity installed - it might be nice to default
 that flag to on. I also needed to do some tweaking to make it work
 properly with parallel building - eg. autogenning with:
 
   --with-num-cpus=16 --with-max-jobs=4 --enable-zenity
I forgot about DISPLAY. Thanks. I'll have to look into it.
I guess I'm going to have to try building with different option and in
different environments.

   And I hope that the open3 module works nicely on Windows / Mac -
 supposedly it is 'core'.
I thought about this, but I thought I'd see what gets said about it.
Zenity spits out lots of stuff on stderr, which I guess could just be
piped to /dev/null instead.

 
   Anyhow - thanks ! is there anything else you'd like to work on ?

I'm not really sure. I've not really looked at any of this code before
so I'm just trying to learn about it at the moment, though I haven't
looked at anything past the build system yet.

Thank you for being so welcoming.

Regards,
Luke

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