Re: fdo#61135 New feature in Calc: Stepped Lines graphs - first part of code submitted
On 08/03/13 09:55, Markus Mohrhard wrote: On Wednesday 06 March 2013 17:00:56 Markus Mohrhard wrote: The import for files written by gnumeric should map the values to the corresponding ODF values and during export we only want to export them as ODF and never into the gnumeric namespace. This also means to OK. I thought that it would be nice for somebody that wrote the file with Gnumeric not to silently convert it to something that might not be read back the same in Gnumeric. That's why I added all these GNM_* values. But I can understand your POV. the problem is that an attribute can have only one value, and it's far better in the long run to write the value that is in a current ODF draft than the value that will only be understood by Gnumeric. remove all the references to the GNM_* values in the other source files. For that to correctly work we need to register the gnumeric namespace and map the elements in the chart namespace and the elements in the gnumeric namesapce to the same UNO attributes. Could you explain this a bit more. Are we talking about the XML namespace or the C++ namespace ? I see some references in the code related to Gnumeric and a link with orcus. We are talking about XML namespaces. The gnumeric elements with gnm prefix are in an gnumeric namespace. The other links you see to gnumeric code is in calc and orcus for the gnumeric import which is only a proof of concept right now. you have fallen into the same trap as i have 2 weeks ago: the gnm: prefix is part of the value, not any attribute or element name, and thus no namespace needs to be declared to support it. into the gnumeric namespace. Additionally we need to take care of the ODF version during export and make sure we only export it into ODF1.2 extended. I think Thorsten was fine with exporting the elements into the chart namespace but only for ODF 1.2 extended. First a general question. I found the option for selecting the ODF type. But how does that work when somebody requires, e.g., strict ODF1.2 or lower compliance. Is there a warning given when the file is written ? Do we hide the option for stepped lines then ? We warn the user that choosing anything except ODF 1.2 extended may loose data but we don't hide anything in the UI. We just drop the elements during export. in the latter Sun days there were plans to query the ODF version from the configuration in the UI and only allow inserting those content elements that are supported by the selected ODF version. i can't remember if anything was ever implemented for that, but in general it would be a good idea. (except it would not be worth the effort for 1.1 because there is just too much missing there; it was only intended for post-ODF 1.2 features). Secondly, can you point to an example where the above is done ? Then I can implement something similar. Sadly this one is a bit tricky. Normally the magic happens in how we specify the attribute values in the map but for this one I need to explore it a bit more. some maps have an additional element to store the required version which is then checked on export but apparently this one does not (yet). for example there is a MAP_ENTRY_ODF_EXT macro in that PropertyMap.hxx... oh actually that applies to entire attributes while here we want to only handle certain values of the attribute differently... i guess it needs some special handling then, there is some flag MID_FLAG_SPECIAL_ITEM_EXPORT which causes a call to the handleSpecialItem() method, which can then mangle the attribute in arbitrary ways, so you can use it to generate appropriate fall-back values for older ODF versions. also, your changes to xmloff/source/transform/StyleOASISTContext.cxx seem a bit pointless to me: source/transform is for the OpenOffice.org XML format, which is a predecessor of ODF and a legacy format and usually does not get new features added :) well i guess nobody will complain about some different values there. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Re: fdo#61135 New feature in Calc: Stepped Lines graphs - first part of code submitted
Hey Eric, On Wednesday 06 March 2013 17:00:56 Markus Mohrhard wrote: First we need to move the new values in CurveStyle.idl to the end and remove all the GNM_* values. Moving the values to the end ensures that the old values stay unchanged which causes as little trouble for extension developers using the API as possible. OK. I wasn't aware that the last value there was not a sentinel but needed to remain the same. It is always better to leave existing values to make it easier for potential external users of the API. The import for files written by gnumeric should map the values to the corresponding ODF values and during export we only want to export them as ODF and never into the gnumeric namespace. This also means to OK. I thought that it would be nice for somebody that wrote the file with Gnumeric not to silently convert it to something that might not be read back the same in Gnumeric. That's why I added all these GNM_* values. But I can understand your POV. remove all the references to the GNM_* values in the other source files. For that to correctly work we need to register the gnumeric namespace and map the elements in the chart namespace and the elements in the gnumeric namesapce to the same UNO attributes. Could you explain this a bit more. Are we talking about the XML namespace or the C++ namespace ? I see some references in the code related to Gnumeric and a link with orcus. We are talking about XML namespaces. The gnumeric elements with gnm prefix are in an gnumeric namespace. The other links you see to gnumeric code is in calc and orcus for the gnumeric import which is only a proof of concept right now. So the right way is to register the namespace. An exmaple for such a change is (http://cgit.freedesktop.org/libreoffice/core/commit/?id=f18a242966d3fd25ec0832c09ce7164bdae7ba2d ) where I added a namespace for calc ODF1.2 extended elements. You need to do something similar for gnumeric with the http://www.gnumeric.org/odf-extension/1.0 URL. It makes sense to use GNM or something like that as namespace alias in our code as this is more or less the official gnumeric namespace prefix. into the gnumeric namespace. Additionally we need to take care of the ODF version during export and make sure we only export it into ODF1.2 extended. I think Thorsten was fine with exporting the elements into the chart namespace but only for ODF 1.2 extended. First a general question. I found the option for selecting the ODF type. But how does that work when somebody requires, e.g., strict ODF1.2 or lower compliance. Is there a warning given when the file is written ? Do we hide the option for stepped lines then ? We warn the user that choosing anything except ODF 1.2 extended may loose data but we don't hide anything in the UI. We just drop the elements during export. Secondly, can you point to an example where the above is done ? Then I can implement something similar. Sadly this one is a bit tricky. Normally the magic happens in how we specify the attribute values in the map but for this one I need to explore it a bit more. Additionally it would be nice if you would provide at least one of your test documents from gnumeric and one of your test documents for normal ODF as MPL/LGPL licensed test documents that we can add to our test file repository. If you are interested it would also be nice if you could add a test case for it to our new chart2 import test at chart2/qa/extras/chart2import.cxx. There is a Gnumeric file attached to the bugzilla. I hereby place it under MPL/LGPL. Once I'm further, I will also add a similar file created fully in LO. Thanks I'll add it to the test file repository at http://cgit.freedesktop.org/libreoffice/contrib/test-files/ I'll have a look at the test cases and what is done there. Basically what we do there is just import a document and check through the UNO API that the feature we are interested in is correctly imported. So in your case it just means that we need to have a test document with all the stepped line featiures that you want to test. The easiest way is to have just one chart per sheet and then use the existing code to get the chart and the data series and just test for your property. However if you have problems with it I can add the test. For the UI part of the task it might make sense to first convert the relevant dialog to the new UI format and then add the new elements into it. As indicated in the UI ML, I will use a seperate dialog. As for getting up to speed with Glade, I was thinking about converting the existing dialog for the spline option first. Then, I know better how to create a new one. Great. Thanks. But first I'll amend the existing patch with the code changes you requested above. Once those are accepted I'll tackle the UI. Regards, Markus ___ LibreOffice mailing list
Re: Re: fdo#61135 New feature in Calc: Stepped Lines graphs - first part of code submitted
Hello Markus, thanks for your review. On Wednesday 06 March 2013 17:00:56 Markus Mohrhard wrote: First we need to move the new values in CurveStyle.idl to the end and remove all the GNM_* values. Moving the values to the end ensures that the old values stay unchanged which causes as little trouble for extension developers using the API as possible. OK. I wasn't aware that the last value there was not a sentinel but needed to remain the same. The import for files written by gnumeric should map the values to the corresponding ODF values and during export we only want to export them as ODF and never into the gnumeric namespace. This also means to OK. I thought that it would be nice for somebody that wrote the file with Gnumeric not to silently convert it to something that might not be read back the same in Gnumeric. That's why I added all these GNM_* values. But I can understand your POV. remove all the references to the GNM_* values in the other source files. For that to correctly work we need to register the gnumeric namespace and map the elements in the chart namespace and the elements in the gnumeric namesapce to the same UNO attributes. Could you explain this a bit more. Are we talking about the XML namespace or the C++ namespace ? I see some references in the code related to Gnumeric and a link with orcus. into the gnumeric namespace. Additionally we need to take care of the ODF version during export and make sure we only export it into ODF1.2 extended. I think Thorsten was fine with exporting the elements into the chart namespace but only for ODF 1.2 extended. First a general question. I found the option for selecting the ODF type. But how does that work when somebody requires, e.g., strict ODF1.2 or lower compliance. Is there a warning given when the file is written ? Do we hide the option for stepped lines then ? Secondly, can you point to an example where the above is done ? Then I can implement something similar. Additionally it would be nice if you would provide at least one of your test documents from gnumeric and one of your test documents for normal ODF as MPL/LGPL licensed test documents that we can add to our test file repository. If you are interested it would also be nice if you could add a test case for it to our new chart2 import test at chart2/qa/extras/chart2import.cxx. There is a Gnumeric file attached to the bugzilla. I hereby place it under MPL/LGPL. Once I'm further, I will also add a similar file created fully in LO. I'll have a look at the test cases and what is done there. Thanks a lot for your great work. It is highly appreciated and feel free to ask for any help you need. :-) For the UI part of the task it might make sense to first convert the relevant dialog to the new UI format and then add the new elements into it. As indicated in the UI ML, I will use a seperate dialog. As for getting up to speed with Glade, I was thinking about converting the existing dialog for the spline option first. Then, I know better how to create a new one. But first I'll amend the existing patch with the code changes you requested above. Once those are accepted I'll tackle the UI. Regards, Markus Thanks, Eric___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: fdo#61135 New feature in Calc: Stepped Lines graphs - first part of code submitted
Hey Eric, I have added my first real code for submission to LO :-) It's in gerrit: https://gerrit.libreoffice.org/2476. I'm going to review it in the next days. so here is the inital review for your change. Your patch looks already quite good but there are some small changes necessary before we can merge it. First we need to move the new values in CurveStyle.idl to the end and remove all the GNM_* values. Moving the values to the end ensures that the old values stay unchanged which causes as little trouble for extension developers using the API as possible. The import for files written by gnumeric should map the values to the corresponding ODF values and during export we only want to export them as ODF and never into the gnumeric namespace. This also means to remove all the references to the GNM_* values in the other source files. For that to correctly work we need to register the gnumeric namespace and map the elements in the chart namespace and the elements in the gnumeric namesapce to the same UNO attributes. Your current export code is producing invalid xml files because you were exporting the gnumeric values without registering the namespace. However this problem should be gone once we no longer export elements into the gnumeric namespace. Additionally we need to take care of the ODF version during export and make sure we only export it into ODF1.2 extended. I think Thorsten was fine with exporting the elements into the chart namespace but only for ODF 1.2 extended. The code in AreaChart.cxx looks mostly good and I will have a final look at it when all the other problems are fixed. Additionally it would be nice if you would provide at least one of your test documents from gnumeric and one of your test documents for normal ODF as MPL/LGPL licensed test documents that we can add to our test file repository. If you are interested it would also be nice if you could add a test case for it to our new chart2 import test at chart2/qa/extras/chart2import.cxx. Please let me know if I should help you with any of the above tasks. I can take care of them if neccesary. Thanks a lot for your great work. It is highly appreciated and feel free to ask for any help you need. For the UI part of the task it might make sense to first convert the relevant dialog to the new UI format and then add the new elements into it. Regards, Markus ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Re: fdo#61135 New feature in Calc: Stepped Lines graphs - first part of code submitted
Anyway, I will now create a mock for the UI and discuss this on the UI list. Once this is done, what would be the best way to add the new UI functionality ? A friendly push in the right direction would save me quite some time ;-) You should design the dialogs nowadays with Glade. We are switching from our old dialog descriptions to new glade xml files. Caolan wrote a nice wiki page about widgetllayout (https://wiki.documentfoundation.org/Development/WidgetLayout). While it focuses mostly on transfering old dialogs to the new format it should give you a basic idea how to create a new one. If you need one of the custom widgets that we have somewhere in the code or other customization of an existing widget we can help you implement that. I'm with you on new Dialogs. I could convert the existing dialog (ChartTypeDialog?) to glade, but it seems a bit ambitious at the moment for my knowledge in LO. Maybe later after getting more experience with simpler dialogs. I saw Caolan's talk on FOSDEM and it seemed not too difficult for the simpler dialogs. Well i think it belongs maybe in the Smooth lines dialog with a new title for the dialog which seems reasonable easy to convert. I think Caolan's tutorial will give you all the information you need for it. If you sill have questions about it you can just ask and we will help you with the task. An inital step would be to design the dialog in glade and send it to the ML. Caolan or I can take care of the source code part if necessary. Regards, Markus ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: Re: fdo#61135 New feature in Calc: Stepped Lines graphs - first part of code submitted
Hello Markus, I also want to add some debug code in what I have done, mainly to print a warning if new curve types are fed to LO. What's the best way to do that ? From https://wiki.documentfoundation.org/Development/How_to_debug#Macros_Contro lling_Debug_Code I would think to add the code in #ifdef SAL_LOG_WARN or use the SAL_WARN macro. You should use SAL_WARN and SAL_INFO for these tasks. Additionally we are using more and more assert if we really want an abort for a special condition. THere is also SAL_WARN_IF and SAL_INFO_IF for conditional debug statements. Thanks for the pointers. Looks like SAL_WARN_IF is what I want. I have amended the patch with this already. Anyway, I will now create a mock for the UI and discuss this on the UI list. Once this is done, what would be the best way to add the new UI functionality ? A friendly push in the right direction would save me quite some time ;-) You should design the dialogs nowadays with Glade. We are switching from our old dialog descriptions to new glade xml files. Caolan wrote a nice wiki page about widgetllayout (https://wiki.documentfoundation.org/Development/WidgetLayout). While it focuses mostly on transfering old dialogs to the new format it should give you a basic idea how to create a new one. If you need one of the custom widgets that we have somewhere in the code or other customization of an existing widget we can help you implement that. I'm with you on new Dialogs. I could convert the existing dialog (ChartTypeDialog?) to glade, but it seems a bit ambitious at the moment for my knowledge in LO. Maybe later after getting more experience with simpler dialogs. I saw Caolan's talk on FOSDEM and it seemed not too difficult for the simpler dialogs. Eric___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: fdo#61135 New feature in Calc: Stepped Lines graphs - first part of code submitted
Hey Eric, I have added my first real code for submission to LO :-) It's in gerrit: https://gerrit.libreoffice.org/2476. I'm going to review it in the next days. I looked on the LO website to find some pointers around in the code, but didn't really found it. My main helpers were OpenGrok and gdb. Did I miss a nice overview of where to find which functionality ? No. There is no real documentation. As you already discovered xmloff/ contains the ODF import and export code for charts and chart2 the whole implementation for the chart code. Additionally oox/ contains the OOXML import and export code for charts. Secondly, what would you recommend as a primer on the C++ functionalities used in LO code + how they are used ? I didn't do that advanced C++ coding before that I know a lot about templates and namespaces. It very much depends on your knowledge. I also want to add some debug code in what I have done, mainly to print a warning if new curve types are fed to LO. What's the best way to do that ? From https://wiki.documentfoundation.org/Development/How_to_debug#Macros_Controlling_Debug_Code I would think to add the code in #ifdef SAL_LOG_WARN or use the SAL_WARN macro. You should use SAL_WARN and SAL_INFO for these tasks. Additionally we are using more and more assert if we really want an abort for a special condition. THere is also SAL_WARN_IF and SAL_INFO_IF for conditional debug statements. Anyway, I will now create a mock for the UI and discuss this on the UI list. Once this is done, what would be the best way to add the new UI functionality ? A friendly push in the right direction would save me quite some time ;-) You should design the dialogs nowadays with Glade. We are switching from our old dialog descriptions to new glade xml files. Caolan wrote a nice wiki page about widgetllayout (https://wiki.documentfoundation.org/Development/WidgetLayout). While it focuses mostly on transfering old dialogs to the new format it should give you a basic idea how to create a new one. If you need one of the custom widgets that we have somewhere in the code or other customization of an existing widget we can help you implement that. Thanks a lot for your great work on this feature. Regards, Markus ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
fdo#61135 New feature in Calc: Stepped Lines graphs - first part of code submitted
Hello everyone, I have added my first real code for submission to LO :-) It's in gerrit: https://gerrit.libreoffice.org/2476. It took me a while to find my way around the code and also to get used to the new possibilities in C++. I looked on the LO website to find some pointers around in the code, but didn't really found it. My main helpers were OpenGrok and gdb. Did I miss a nice overview of where to find which functionality ? Secondly, what would you recommend as a primer on the C++ functionalities used in LO code + how they are used ? I didn't do that advanced C++ coding before that I know a lot about templates and namespaces. I also want to add some debug code in what I have done, mainly to print a warning if new curve types are fed to LO. What's the best way to do that ? From https://wiki.documentfoundation.org/Development/How_to_debug#Macros_Controlling_Debug_Code I would think to add the code in #ifdef SAL_LOG_WARN or use the SAL_WARN macro. Anyway, I will now create a mock for the UI and discuss this on the UI list. Once this is done, what would be the best way to add the new UI functionality ? A friendly push in the right direction would save me quite some time ;-) Thanks, Eric___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
