Review Request: Added Footnotes/Endnotes in References Tool

Brijesh Patel brijesh3105 at gmail.com
Thu Sep 8 16:26:22 BST 2011



> On Sept. 8, 2011, 4:58 a.m., Thorsten Zachmann wrote:
> > libs/kotext/KoInlineNote.cpp, lines 164-165
> > <http://git.reviewboard.kde.org/r/102552/diff/2/?file=35543#file35543line164>
> >
> >     Maybe you can remove the count variable and do the renumbering where a new footnote is added to the inline object manager. Then you will be sure it works correctly also when there are more then one document.

that poses a problem when the footnotes are deleted...


> On Sept. 8, 2011, 4:58 a.m., Thorsten Zachmann wrote:
> > libs/odf/KoOdfStylesReader.cpp, lines 289-305
> > <http://git.reviewboard.kde.org/r/102552/diff/2/?file=35552#file35552line289>
> >
> >     Do you want to set the default values only if it was loaded from the document. I think it might make sense to set it also when no configuration was loaded.

this part is doing that only,whenever no configuration was loaded,at that time 
d->globalFootnoteConfiguration.numberFormat().formatSpecification() = KoOdfNumberDefinition::Empty
and 
d->globalEndnoteConfiguration.numberFormat().formatSpecification() == KoOdfNumberDefinition::RomanLowerCase

thats why i am comparing like this,
if it doesn't match,no need to load the configuration again.


> On Sept. 8, 2011, 4:58 a.m., Thorsten Zachmann wrote:
> > libs/textlayout/KoTextLayoutArea.cpp, lines 337-350
> > <http://git.reviewboard.kde.org/r/102552/diff/2/?file=35554#file35554line337>
> >
> >     If that code is not needed please remove. If it is still needed please give a comment on why it is commented out at the moment.

this code is not needed right now but will be modified soon
it will enable the cursor to blink in the foot notes frame
but right now it is buggy as the subFrame i am taking is not the correct one


> On Sept. 8, 2011, 4:58 a.m., Thorsten Zachmann wrote:
> > libs/textlayout/KoTextLayoutEndNotesArea.cpp, line 59
> > <http://git.reviewboard.kde.org/r/102552/diff/2/?file=35556#file35556line59>
> >
> >     What does this magic number 15 do?

this is just shifting the endnotes area little below the divider line which lies at top()+10.
it is just to give a proper gap between the line and the text in endnotes frame.


> On Sept. 8, 2011, 4:58 a.m., Thorsten Zachmann wrote:
> > libs/textlayout/KoTextLayoutEndNotesArea.cpp, line 128
> > <http://git.reviewboard.kde.org/r/102552/diff/2/?file=35556#file35556line128>
> >
> >     Maybe you can somewhere define what all these numbers are for

when we were doing  painter->drawLine(left(), top(), right(), top());
the divider line was not looking elegant since it covers the whole space from left to right in the doc and also very much sticking to the normal text of the document above the line.

so give proper spacing and increase the elegance,i reduced the width of the line and shifted it somewhat below than it was before.


> On Sept. 8, 2011, 4:58 a.m., Thorsten Zachmann wrote:
> > libs/kotext/KoInlineNote.h, line 105
> > <http://git.reviewboard.kde.org/r/102552/diff/2/?file=35542#file35542line105>
> >
> >     Using a static variable here will not work as the variable will be used for different documents when more then one document is opened. Maybe you can describe a bit for what this variable is needed so that we come up with a solution for this.

that variable stores the previous count of visible notes,so when it is compared with the current count which is returned by KoInlineTextObjectManager::visibleAutoNumberedNotes() and if it doesn't match
we can renumber the autonumbered notes


> On Sept. 8, 2011, 4:58 a.m., Thorsten Zachmann wrote:
> > libs/kotext/KoInlineNote.cpp, lines 286-290
> > <http://git.reviewboard.kde.org/r/102552/diff/2/?file=35543#file35543line286>
> >
> >     The notes configuration should be saved only once and not with each footnote. Please ping me on IRC if you have a question on how that can be done.

I am doing a check for that only,
see line 283: if (KoTextDocument(d->textFrame->document()).inlineTextObjectManager()->getFirstNote(d->textFrame->document()->begin())->id() == this-   
              >id()) {

it will go inside "if" only if it is the first visible note of the document otherwise it wont go


> On Sept. 8, 2011, 4:58 a.m., Thorsten Zachmann wrote:
> > libs/kotext/KoInlineNote.cpp, line 233
> > <http://git.reviewboard.kde.org/r/102552/diff/2/?file=35543#file35543line233>
> >
> >     Is there a reason to remove this line here. I think it only makes sure that the initial value of the variable is set correctly.

I dont remember the reason exactly why i removed,
but it was something like this
the notes which are inserted in this session only get the autonumbering tag while loading,but previously inserted notes don't.
so counting begins from 1 in each session.


- Brijesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102552/#review6323
-----------------------------------------------------------


On Sept. 7, 2011, 5:17 p.m., Brijesh Patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102552/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2011, 5:17 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> I worked on implementation of Footnotes and Endnotes in Calligra Words as part of my SoK project.
> Footnotes/Endnotes can now be inserted,saved,loaded and also be configured too.
> Can somebody review my work and notify me about bugs/crashes?
> 
> 
> Diffs
> -----
> 
>   libs/kotext/KoInlineNote.h 515a2f3 
>   libs/kotext/KoInlineNote.cpp 89200fd 
>   libs/kotext/KoInlineTextObjectManager.h 44990e9 
>   libs/kotext/KoInlineTextObjectManager.cpp da8d9ca 
>   libs/kotext/KoTextEditor.h ba28354 
>   libs/kotext/KoTextEditor.cpp 4d3e5b18 
>   libs/kotext/opendocument/KoTextLoader.cpp e9d47da 
>   libs/kotext/styles/KoSectionStyle.cpp 2f3a1ad 
>   libs/odf/KoOdfNotesConfiguration.cpp bc26e31 
>   libs/odf/KoOdfNumberDefinition.cpp d35a07f 
>   libs/odf/KoOdfStylesReader.cpp 5a8324e 
>   libs/textlayout/KoTextLayoutArea.h ea7fd8e 
>   libs/textlayout/KoTextLayoutArea.cpp 504de0c 
>   libs/textlayout/KoTextLayoutEndNotesArea.h 7b0c732 
>   libs/textlayout/KoTextLayoutEndNotesArea.cpp bb746db 
>   plugins/textshape/CMakeLists.txt 3df7aa5 
>   plugins/textshape/ReferencesTool.h 40676b2 
>   plugins/textshape/ReferencesTool.cpp 0bbd38d 
>   plugins/textshape/dialogs/NotesConfigurationDialog.h PRE-CREATION 
>   plugins/textshape/dialogs/NotesConfigurationDialog.cpp PRE-CREATION 
>   plugins/textshape/dialogs/NotesConfigurationDialog.ui PRE-CREATION 
>   plugins/textshape/dialogs/SimpleFootEndNotesWidget.h 1af35a8 
>   plugins/textshape/dialogs/SimpleFootEndNotesWidget.cpp 374b266 
>   plugins/textshape/dialogs/SimpleFootEndNotesWidget.ui 77f33e7 
>   plugins/textshape/pics/settings-icon1_1.png PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102552/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Brijesh
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20110908/d6b2bbb8/attachment.htm>


More information about the calligra-devel mailing list