Download it now!
Bug 38885 - Remove CreateTextFromNode methods
Summary: Remove CreateTextFromNode methods
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Formula Editor (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other All
: medium normal
Assignee: Advait Ranade
URL:
Whiteboard:
Keywords: difficultyInteresting, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2011-07-01 07:30 UTC by Björn Michaelsen
Modified: 2020-05-21 14:16 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Michaelsen 2011-07-01 07:30:01 UTC
Remove CreateTextFromNode methods

Background: SmNode have a virtual method called CreateTextFromNode, this method is overwritten in some subclasses. However, the method is somewhat buggy/incomplete, similar functionality have been implemented in the SmNodeToTextVisitor, which should be more complete. So we should remove the CreateTextFromNode methods, and reuse any useful parts, if any, in SmNodeToTextVisitor. Calls to CreateTextFromNode should be replaced with calls to SmNodeToTextVisitor, and this should be tested.

Skills: Building, C++
Comment 1 Florian Reisinger 2012-05-18 09:02:00 UTC
Deteted "Easyhack" from summary
Comment 2 Ivan Timofeev (retired) 2012-07-21 18:57:19 UTC
CreateTextFromNode is used only in mathmlimport AFAICS.

But mathmlimport produces a structurally-wrong tree, which *can* be converted to text via CreateTextFromNode, but SmNodeToTextVisitor works only with "right" trees and fails to produce something sensible from that tree from mathmlimport.

It would be better if mathmlimport could convert mathml->starmath I guess, now it does mathml->fake tree->[CreateTextFromNode]->starmath.
Comment 3 Ricardo Montania 2013-09-06 12:32:25 UTC
I have interest on it.

On a first look, I saw that SmNodeToTextVisitor is a class constructor, and need an pNode and a OUString rText as argument to be constructed. But SmNode::CreateTextFromNode need only an OUString, and the pNode is created inside the method. Is that a problem?

And, in SmNode::CreateTextFromNode there is a verification for the number of subNodes, and in SmNodeToTextVisitor we have this virification only in Visitor method in according with the node. Then, to replace CreateTextFromNode should I call SmNodeToTextVisitor constructor and then call the Visitor method according to the case?

Thanks.
Comment 4 Julien Nabet 2013-09-29 15:13:29 UTC
Ivan: is there still the pb of structurally-wrong tree for mathml import?

Frédéric: thinking you might be interested in this, I put you in cc.
Comment 5 Ivan Timofeev (retired) 2013-09-30 11:10:39 UTC
(In reply to comment #4)
> Ivan: is there still the pb of structurally-wrong tree for mathml import?

Most probably yes, but I forgot everything on this topic...
Comment 6 Björn Michaelsen 2013-10-04 18:46:04 UTC
adding LibreOffice developer list as CC to unresolved EasyHacks for better visibility.

see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Comment 7 Regina Henschel 2015-07-08 10:44:20 UTC
I think, that it should be a long time goal to switch from StarMath to MathML. But as long as there exists no direct editing of MathML, it is too early to remove generating StarMath source.
Comment 8 Robinson Tryon (qubit) 2015-12-14 05:03:39 UTC Comment hidden (obsolete)
Comment 9 Robinson Tryon (qubit) 2016-02-18 14:51:21 UTC Comment hidden (obsolete)
Comment 10 jani 2016-05-05 13:39:43 UTC
Missing code pointer, mandatory for easy hacks
Comment 11 jani 2016-07-06 06:12:02 UTC
oox/source/mathml/import.cxx  (mathhtml import)

starmath/source/node.cxx
starmath/inc/node.hxx
Comment 12 Julien Nabet 2017-01-29 14:19:41 UTC
If there's an assignee, let's put this tracker as ASSIGNED
Comment 13 jani 2017-05-14 07:42:50 UTC Comment hidden (obsolete)
Comment 14 Fakabbir amin 2017-05-14 11:42:48 UTC
(In reply to jani from comment #13)
> A polite ping, still working on this bug

Hmm, Will try to submit a patch soon.
Comment 15 prakritisharma80@gmail.com 2019-03-13 16:48:48 UTC
Hi,
I would like to take this up as my first issue.
Comment 16 Abhishek 2020-02-14 14:09:47 UTC
In which file the function "SmNodeToTextVisitor" is decleard ?
Comment 17 Julien Nabet 2020-02-14 14:15:46 UTC
(In reply to Abhishek from comment #16)
> In which file the function "SmNodeToTextVisitor" is decleard ?

You can use Opengrok:
See
https://opengrok.libreoffice.org/search?project=core&full=&defs=SmNodeToTextVisitor&refs=&path=&hist=&type=&si=full
Comment 18 Abhishek 2020-02-27 14:39:00 UTC
How can I remove these kind of warinings?
"home/abhishek/core/starmath/source/node.cxx:765:47: warning: unused parameter ‘pNode’ [-Wunused-parameter]
 void SmRootNode::SmNodeToTextVisitor( SmNode* pNode, OUString &eText )"
Comment 19 Julien Nabet 2020-02-27 14:44:03 UTC
(In reply to Abhishek from comment #18)
> How can I remove these kind of warinings?
> "home/abhishek/core/starmath/source/node.cxx:765:47: warning: unused
> parameter ‘pNode’ [-Wunused-parameter]
>  void SmRootNode::SmNodeToTextVisitor( SmNode* pNode, OUString &eText )"

It could be interesting you submit your patch on gerrit with WIP (Work In Progress) mark so devs may give their opinion.
Meanwhile, without seeing more code, you can change the line into:
void SmRootNode::SmNodeToTextVisitor( SmNode* /* pNode */, OUString &eText )
Comment 20 Abhishek 2020-02-27 18:08:25 UTC Comment hidden (obsolete)
Comment 21 Abhishek 2020-02-27 18:09:07 UTC Comment hidden (obsolete)
Comment 22 Buovjaga 2020-02-27 18:46:11 UTC
(In reply to Abhishek from comment #21)
> Actually I have met a dead end while debugging. 
> Is it possible to submit a patch even if it isn't buliding ?

Yes. You can submit it directly as work in progress using the logerrit script by:

./logerrit submit-wip master
Comment 23 Abhishek 2020-02-29 13:10:03 UTC
Sorry for the delay. I have submitted the patch with commit message "WIP test 38885". Please review it.
Comment 24 Buovjaga 2020-02-29 13:29:09 UTC
(In reply to Abhishek from comment #23)
> Sorry for the delay. I have submitted the patch with commit message "WIP
> test 38885". Please review it.

You forgot to link to it, but here it is:
https://gerrit.libreoffice.org/c/core/+/89756
Comment 25 Abhishek 2020-03-02 10:13:17 UTC
In my submitted patch there are some error messages such as "undefined references for function SmNodeToTextVisitor " but they are not showing where exactly the error is occuring? Please Help!
Comment 26 Abhishek 2020-03-02 12:35:34 UTC Comment hidden (obsolete)
Comment 27 Julien Nabet 2020-03-03 21:04:10 UTC
(In reply to Abhishek from comment #25)
> In my submitted patch there are some error messages such as "undefined
> references for function SmNodeToTextVisitor " but they are not showing where
> exactly the error is occuring? Please Help!

On your local sources, type:
git pull -r

then try again to build starmath module.
Comment 28 Abhishek 2020-03-03 22:38:54 UTC
(In reply to Julien Nabet from comment #27)
> (In reply to Abhishek from comment #25)
> > In my submitted patch there are some error messages such as "undefined
> > references for function SmNodeToTextVisitor " but they are not showing where
> > exactly the error is occuring? Please Help!
> 
> On your local sources, type:
> git pull -r
> 
> then try again to build starmath module.

I have removed all the warning messages but there are still undefined refernces.
Sorry for irrelevant commit.
My current error message is

/home/abhishek/core/workdir/CxxObject/starmath/source/node.o:(.data.rel.ro._ZTV6SmNode[_ZTV6SmNode]+0x50): undefined reference to `SmNode::SmNodeToTextVisitor(SmNode*, rtl::OUString&)'
/home/abhishek/core/workdir/CxxObject/starmath/source/node.o:(.data.rel.ro._ZTV15SmStructureNode[_ZTV15SmStructureNode]+0x50): undefined reference to `SmStructureNode::SmNodeToTextVisitor(SmNode*, rtl::OUString&)'
/home/abhishek/core/workdir/CxxObject/starmath/source/node.o:(.data.rel.ro._ZTV13SmVisibleNode[_ZTV13SmVisibleNode]+0x50): undefined reference to `SmNode::SmNodeToTextVisitor(SmNode*, rtl::OUString&)'
/home/abhishek/core/workdir/CxxObject/starmath/source/node.o:(.data.rel.ro._ZTV13SmGraphicNode[_ZTV13SmGraphicNode]+0x50): undefined reference to `SmNode::SmNodeToTextVisitor(SmNode*, rtl::OUString&)'
/home/abhishek/core/workdir/CxxObject/starmath/source/node.o:(.data.rel.ro._ZTV11SmTableNode[_ZTV11SmTableNode]+0x50): undefined reference to `SmStructureNode::SmNodeToTextVisitor(SmNode*, rtl::OUString&)'
/home/abhishek/core/workdir/CxxObject/starmath/source/node.o:(.data.rel.ro._ZTV10SmLineNode[_ZTV10SmLineNode]+0x50): undefined reference to `SmStructureNode::SmNodeToTextVisitor(SmNode*, rtl::OUString&)'
/home/abhishek/core/workdir/CxxObject/starmath/source/node.o:(.data.rel.ro._ZTV11SmUnHorNode[_ZTV11SmUnHorNode]+0x50): undefined reference to `SmStructureNode::SmNodeToTextVisitor(SmNode*, rtl::OUString&)'
/home/abhishek/core/workdir/CxxObject/starmath/source/node.o:(.data.rel.ro._ZTV12SmBinHorNode[_ZTV12SmBinHorNode]+0x50): undefined reference to `SmStructureNode::SmNodeToTextVisitor(SmNode*, rtl::OUString&)'
/home/abhishek/core/workdir/CxxObject/starmath/source/node.o:(.data.rel.ro._ZTV17SmBinDiagonalNode[_ZTV17SmBinDiagonalNode]+0x50): undefined reference to `SmStructureNode::SmNodeToTextVisitor(SmNode*, rtl::OUString&)'
/home/abhishek/core/workdir/CxxObject/starmath/source/node.o:(.data.rel.ro._ZTV15SmBracebodyNode[_ZTV15SmBracebodyNode]+0x50): more undefined references to `SmStructureNode::SmNodeToTextVisitor(SmNode*, rtl::OUString&)' follow
/home/abhishek/core/workdir/CxxObject/starmath/source/node.o:(.data.rel.ro._ZTV14SmPolyLineNode[_ZTV14SmPolyLineNode]+0x50): undefined reference to `SmNode::SmNodeToTextVisitor(SmNode*, rtl::OUString&)'
collect2: error: ld returned 1 exit status
/home/abhishek/core/starmath/Library_sm.mk:10: recipe for target '/home/abhishek/core/instdir/program/libsmlo.so' failed
make: *** [/home/abhishek/core/instdir/program/libsmlo.so] Error 1
Comment 29 Julien Nabet 2020-03-03 22:43:16 UTC
try:
make starmath.clean && make starmath

if still stuck, please upload your new version on gerrit by using --amend option to update the existing patch instead of creating a new one when using logerrit submit
Comment 30 Abhishek 2020-03-03 23:34:03 UTC
I tried make starmath.clean && make starmath 
but it is still showing me the same error messages.
Comment 31 Julien Nabet 2020-03-04 07:58:39 UTC
(In reply to Abhishek from comment #30)
> I tried make starmath.clean && make starmath 
> but it is still showing me the same error messages.

Whereas in my previous comment I had talked about amending the existing commit to avoid dups, I see there are 2 patches now:
- https://gerrit.libreoffice.org/c/core/+/89938
- https://gerrit.libreoffice.org/c/core/+/89940

Both with same description:
WIP 38885 removed warinings but there are still undefined referneces
but not containing the same thing.

1) Abandon one of these to have only 1 patch.
2) Change description to:
tdf#38885: Remove CreateTextFromNode methods

Indeed description shouldn't be used to indicate the state of the patch.
Comment 32 Abhishek 2020-03-04 16:25:59 UTC
(In reply to Julien Nabet from comment #31)
> (In reply to Abhishek from comment #30)
> > I tried make starmath.clean && make starmath 
> > but it is still showing me the same error messages.
> 
> Whereas in my previous comment I had talked about amending the existing
> commit to avoid dups, I see there are 2 patches now:
> - https://gerrit.libreoffice.org/c/core/+/89938
> - https://gerrit.libreoffice.org/c/core/+/89940
> 
> Both with same description:
> WIP 38885 removed warinings but there are still undefined referneces
> but not containing the same thing.
> 
> 1) Abandon one of these to have only 1 patch.
> 2) Change description to:
> tdf#38885: Remove CreateTextFromNode methods
> 
> Indeed description shouldn't be used to indicate the state of the patch.

Okay. Done.
Comment 33 Abhishek 2020-03-12 15:15:59 UTC
Can someone help me with the merge confilct I am facing in 
  https://gerrit.libreoffice.org/c/core/+/89938
Comment 34 Buovjaga 2020-03-12 15:25:46 UTC
(In reply to Abhishek from comment #33)
> Can someone help me with the merge confilct I am facing in 
>   https://gerrit.libreoffice.org/c/core/+/89938

I guess it is because you have created your work on top of this which has never been merged: https://gerrit.libreoffice.org/c/core/+/89756/

See the link "Parent 08bded6".
Comment 35 Abhishek 2020-03-13 09:04:54 UTC
I am facing some errors in 
https://gerrit.libreoffice.org/c/core/+/89756/
Please help.
Comment 36 Buovjaga 2020-03-13 11:08:14 UTC
Abhishek: in https://gerrit.libreoffice.org/c/core/+/89938 Julien gave you detailed advice. Can you try it?
Comment 37 Abhishek 2020-03-15 14:45:44 UTC
Yes , I tried it. I have uploaded new patch after that.There are few more errors that I am not able to understand. Please help.
Comment 38 Julien Nabet 2020-03-15 15:41:55 UTC
(In reply to Abhishek from comment #37)
> Yes , I tried it. I have uploaded new patch after that.There are few more
> errors that I am not able to understand. Please help.

I don't see any upload after my comment in https://gerrit.libreoffice.org/c/core/+/89938.
Comment 39 Abhishek 2020-03-15 17:15:55 UTC
I was hoping to have help in  https://gerrit.libreoffice.org/c/core/+/89756/ because https://gerrit.libreoffice.org/c/core/+/89938 is an indirect ancestor of it.
Comment 40 Abhishek 2020-03-15 17:26:39 UTC
Also, should I abandon https://gerrit.libreoffice.org/c/core/+/89938 ?
Comment 41 Julien Nabet 2020-03-15 17:42:35 UTC
(In reply to Abhishek from comment #40)
> Also, should I abandon https://gerrit.libreoffice.org/c/core/+/89938 ?

Unless, you know how to rebase, resolve conflicts, etc. the simplest thing to do would be to abandon both then:
- update your local repository to be synchronized with master sources
- make your changes (you can help from your abandoned patch but don't copy paste from them)
- check it builds locally
if ok, submit your patch to gerrit.

If it's not ok and you need several days to fix this, before submitting again on gerrit, please rebase your local repo and check there's no conflicts and it still builds.
Comment 42 Abhishek 2020-03-15 19:19:03 UTC
I think I have solved the conflict problem in "https://gerrit.libreoffice.org/c/core/+/89756/ ".In the current patch, I have build error named "undefined references with no particular location".
Comment 43 Buovjaga 2020-05-21 14:16:38 UTC
Removing easyHack keyword as this is lacking a mentor