[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: Review Request: RDF support for KWord.
From:       "Boudewijn Rempt" <boud () valdyas ! org>
Date:       2010-01-29 9:02:55
Message-ID: 20100129090255.6985.28310 () localhost
[Download RAW message or body]


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


Well, I cannot test it since it doesn't seem to apply atm. Putting \
KoDocumentRDF in flake/KoResourceManager and duplicating FindSoprano.cmake \
are real issues. I was not clear on whether this would be an optional \
feature or not, hence my confusion about soprano -- if it's a hard \
dependency for the koffice libs, it should indeed stop cmake from \
continuing.

Sidenote: I guess we'd better copy the documentation from \
http://monkeyiq.blogspot.com/ to a wiki page. Otherwise it'll be hard to \
figure out what it's all for.

I don't claim to have Thomas' eagle eyes for style violations in diffs yet, \
but there are still issues, like often putting the return type of a method \
on its own line, having big blogs of comment characters sprinkled around or \
commented out lines that are unclear why they are commented out or stay in. \
Like:


/trunk/koffice/kword/part/KWView.cpp
<http://reviewboard.kde.org/r/2611/#comment3339>

    This is a well-know bug in astyle. If the foreach block isn't in {}, \
astyle will remove the indentation for the block. In my opinion, all for \
and foreach blocks should have {} -- so I'd add them here.



/trunk/koffice/kword/part/KWView.cpp
<http://reviewboard.kde.org/r/2611/#comment3340>

    Same foreach astyle mess-up here.



/trunk/koffice/kword/part/KWView.cpp
<http://reviewboard.kde.org/r/2611/#comment3341>

    ditto... Best grep for foreach | fgrep -v "{" or something like that.



/trunk/koffice/kword/part/dockers/KWRdfDockerTree.cpp
<http://reviewboard.kde.org/r/2611/#comment3342>

    before committing, best remove lines that are commented out without a \
comment about why they should stay around.



/trunk/koffice/libs/main/rdf/KoDocumentRdf.cpp
<http://reviewboard.kde.org/r/2611/#comment3343>

    Same in this method: if the commented-out stuff is important, we need \
to know why, otherwise it shouldn't be there.



/trunk/koffice/libs/main/rdf/KoDocumentRdf.cpp
<http://reviewboard.kde.org/r/2611/#comment3344>

    No eagle eyes, mine, but return type and method name should be on the \
same line.



/trunk/koffice/libs/main/rdf/KoDocumentRdfEditWidget.h
<http://reviewboard.kde.org/r/2611/#comment3345>

    Style: don't use big blocks of many lines of *** or /// 


- Boudewijn


On 2010-01-29 01:32:54, Ben Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2611/
> -----------------------------------------------------------
> 
> (Updated 2010-01-29 01:32:54)
> 
> 
> Review request for KOffice.
> 
> 
> Summary
> -------
> 
> RDF support for KWord. As can be seen from the files, KoDocumentRDF is \
> the main new addition, with changes to other areas for load/save, \
> copy/paste of RDF enriched document elements. For interaction with RDF \
> one can either use the triple model directly through KoDocumentRDF or \
> obtain various "semantic items" from KoDocRDF. These semantic items \
> (semitems) come from the RDF triples and go back to them. The semitems \
> are a convenience allowing interaction at a more conceptual level, for \
> example as a single "Contact" instead of remembering the 10+ constituent \
> triples. 
> Instead of looking at lower level issues, I'm hoping that initial \
> comments will focus more on the errors and/or misuses I've made as a \
> newbie to KOffice hacking. Once I fix those up we can move to a more \
> serious review? 
> There is a KoDocument::textEditor() method which I added temporarily and \
> have removed most references to. The two main places that use it are \
> after you edit a semantic item a slot() in KoDocumentRDF grabs \
> this->document()->textEditor() to update the semitem's text in the \
> document to reflect your changes. This could be mitigated by adding a \
> slot() in KWDocument that does the same call but has access to the \
> KoTextEditor. The one problem area is in the "File/Document Information" \
> window where I allow the default stylesheets to be set. The best solution \
> in my mind here is once again signals/slots, the infodialog could tell \
> the KoDocumentRDF what to do which through sig/slots could send the \
> information to KWDocument to cause a relayout of semitems using the new \
> stylesheet preference. 
> For ideas of what the patch allows, see my recent blog entries:
> http://monkeyiq.blogspot.com/
> 
> A few known issues:
> 
> * Assumption that marble will be found, there are some ifdefs but
> I should improve those and perform a full compile with marble-dev
> explicitly hidden
> 
> * Two kDebug() numbers used but unreserved as yet.
> 30015        koffice (rdf)
> 30016        koffice (rdf semantic)
> 
> * I will be fixing a few minor bugs in KoDocumentRDFEditWidget during 
> the review process, these should be fairly contained changes.
> 
> * Some additions for KoBookmark copy and paste. If you select only half \
> the text for the KoBookmark and copy, with this patch, it will now \
> produce a correct ODF file. On paste, \
> KoTextLoader::createUniqBookmarkName() is used to ensure that a new \
> bookmark name is made, while respecting that the bookmark-end wants the \
> previous unique bookmark name instead of the next in series. 
> * After editing a semantic item or applying a stylesheet I'm getting a 
> green highlight now that wasn't there on the 23rd of December. 
> The 23rd was my previous rebase.
> 
> 
> Diffs
> -----
> 
> /trunk/koffice/CMakeLists.txt 1081673 
> /trunk/koffice/cmake/modules/FindSoprano.cmake PRE-CREATION 
> /trunk/koffice/kword/part/CMakeLists.txt 1081673 
> /trunk/koffice/kword/part/KWDocument.cpp 1081673 
> /trunk/koffice/kword/part/KWOdfLoader.cpp 1081673 
> /trunk/koffice/kword/part/KWOdfWriter.cpp 1081673 
> /trunk/koffice/kword/part/KWView.h 1081673 
> /trunk/koffice/kword/part/KWView.cpp 1081673 
> /trunk/koffice/kword/part/dockers/KWRdfDocker.h PRE-CREATION 
> /trunk/koffice/kword/part/dockers/KWRdfDocker.cpp PRE-CREATION 
> /trunk/koffice/kword/part/dockers/KWRdfDocker.ui PRE-CREATION 
> /trunk/koffice/kword/part/dockers/KWRdfDockerFactory.h PRE-CREATION 
> /trunk/koffice/kword/part/dockers/KWRdfDockerFactory.cpp PRE-CREATION 
> /trunk/koffice/kword/part/dockers/KWRdfDockerTree.h PRE-CREATION 
> /trunk/koffice/kword/part/dockers/KWRdfDockerTree.cpp PRE-CREATION 
> /trunk/koffice/kword/part/frames/KWTextDocumentLayout.cpp 1081673 
> /trunk/koffice/kword/part/kword.rc 1081673 
> /trunk/koffice/libs/flake/KoResourceManager.h 1081673 
> /trunk/koffice/libs/flake/KoResourceManager.cpp 1081673 
> /trunk/koffice/libs/kotext/CMakeLists.txt 1081673 
> /trunk/koffice/libs/kotext/KoBookmark.cpp 1081673 
> /trunk/koffice/libs/kotext/KoInlineObject.h 1081673 
> /trunk/koffice/libs/kotext/KoInlineObject.cpp 1081673 
> /trunk/koffice/libs/kotext/KoInlineObject_p.h 1081673 
> /trunk/koffice/libs/kotext/KoTextBlockData.cpp 1081673 
> /trunk/koffice/libs/kotext/KoTextDocument.h 1081673 
> /trunk/koffice/libs/kotext/KoTextDocument.cpp 1081673 
> /trunk/koffice/libs/kotext/KoTextDrag.cpp 1081673 
> /trunk/koffice/libs/kotext/KoTextEditor.cpp 1081673 
> /trunk/koffice/libs/kotext/KoTextInlineRdf.h PRE-CREATION 
> /trunk/koffice/libs/kotext/KoTextInlineRdf.cpp PRE-CREATION 
> /trunk/koffice/libs/kotext/KoTextMeta.h PRE-CREATION 
> /trunk/koffice/libs/kotext/KoTextMeta.cpp PRE-CREATION 
> /trunk/koffice/libs/kotext/KoTextOdfSaveHelper.h 1081673 
> /trunk/koffice/libs/kotext/KoTextOdfSaveHelper.cpp 1081673 
> /trunk/koffice/libs/kotext/KoTextPaste.h 1081673 
> /trunk/koffice/libs/kotext/KoTextPaste.cpp 1081673 
> /trunk/koffice/libs/kotext/KoTextRdfCore.h PRE-CREATION 
> /trunk/koffice/libs/kotext/KoTextRdfCore.cpp PRE-CREATION 
> /trunk/koffice/libs/kotext/opendocument/KoTextLoader.h 1081673 
> /trunk/koffice/libs/kotext/opendocument/KoTextLoader.cpp 1081673 
> /trunk/koffice/libs/kotext/opendocument/KoTextSharedSavingData.h 1081673 
> /trunk/koffice/libs/kotext/opendocument/KoTextSharedSavingData.cpp \
>                 1081673 
> /trunk/koffice/libs/kotext/opendocument/KoTextWriter.cpp 1081673 
> /trunk/koffice/libs/kotext/styles/KoCharacterStyle.h 1081673 
> /trunk/koffice/libs/kotext/styles/KoTableCellStyle.h 1081673 
> /trunk/koffice/libs/main/CMakeLists.txt 1081673 
> /trunk/koffice/libs/main/KoDocument.h 1081673 
> /trunk/koffice/libs/main/KoDocument.cpp 1081673 
> /trunk/koffice/libs/main/KoDocumentInfoDlg.h 1081673 
> /trunk/koffice/libs/main/KoDocumentInfoDlg.cpp 1081673 
> /trunk/koffice/libs/main/KoMainWindow.cpp 1081673 
> /trunk/koffice/libs/main/rdf/InsertSemanticObjectActionBase.h \
>                 PRE-CREATION 
> /trunk/koffice/libs/main/rdf/InsertSemanticObjectActionBase.cpp \
>                 PRE-CREATION 
> /trunk/koffice/libs/main/rdf/InsertSemanticObjectCreateAction.h \
>                 PRE-CREATION 
> /trunk/koffice/libs/main/rdf/InsertSemanticObjectCreateAction.cpp \
>                 PRE-CREATION 
> /trunk/koffice/libs/main/rdf/InsertSemanticObjectReferenceAction.h \
>                 PRE-CREATION 
> /trunk/koffice/libs/main/rdf/InsertSemanticObjectReferenceAction.cpp \
>                 PRE-CREATION 
> /trunk/koffice/libs/main/rdf/KoChangeTrackerDisabledRAII.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/KoChangeTrackerDisabledRAII.cpp PRE-CREATION \
>                 
> /trunk/koffice/libs/main/rdf/KoDocumentRdf.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/KoDocumentRdf.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/KoDocumentRdfEditWidget.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/KoDocumentRdfEditWidget.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/KoDocumentRdfEditWidget.ui PRE-CREATION 
> /trunk/koffice/libs/main/rdf/KoDocumentRdf_p.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/KoSopranoTableModel.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/KoSopranoTableModel.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/KoSopranoTableModelDelegate.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/KoSopranoTableModelDelegate.cpp PRE-CREATION \
>                 
> /trunk/koffice/libs/main/rdf/RdfCalendarEvent.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfCalendarEvent.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfCalendarEventEditWidget.ui PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfCalendarEventTreeWidgetItem.h \
>                 PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfCalendarEventTreeWidgetItem.cpp \
>                 PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfFoaF.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfFoaF.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfFoaFEditWidget.ui PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfFoaFTreeWidgetItem.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfFoaFTreeWidgetItem.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfForward.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfLocation.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfLocation.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfLocationEditWidget.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfLocationEditWidget.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfLocationEditWidget.ui PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfLocationTreeWidgetItem.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfLocationTreeWidgetItem.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfLocationViewWidget.ui PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfPrefixMapping.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfPrefixMapping.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfSemanticItem.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfSemanticItem.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfSemanticItemViewSite.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfSemanticItemViewSite.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfSemanticTree.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfSemanticTree.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfSemanticTreeWidgetAction.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfSemanticTreeWidgetAction.cpp PRE-CREATION \
>                 
> /trunk/koffice/libs/main/rdf/RdfSemanticTreeWidgetItem.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfSemanticTreeWidgetItem.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfSemanticTreeWidgetSelectAction.h \
>                 PRE-CREATION 
> /trunk/koffice/libs/main/rdf/RdfSemanticTreeWidgetSelectAction.cpp \
>                 PRE-CREATION 
> /trunk/koffice/libs/main/rdf/SemanticStylesheet.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/SemanticStylesheet.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/SemanticStylesheetsEditor.h PRE-CREATION 
> /trunk/koffice/libs/main/rdf/SemanticStylesheetsEditor.cpp PRE-CREATION 
> /trunk/koffice/libs/main/rdf/SemanticStylesheetsEditor.ui PRE-CREATION 
> /trunk/koffice/libs/odf/KoXmlNS.h 1081673 
> /trunk/koffice/libs/odf/KoXmlNS.cpp 1081673 
> /trunk/koffice/plugins/textshape/CMakeLists.txt 1081673 
> /trunk/koffice/plugins/textshape/TextTool.cpp 1081673 
> /trunk/koffice/plugins/textshape/commands/TextPasteCommand.cpp 1081673 
> 
> Diff: http://reviewboard.kde.org/r/2611/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ben
> 
> 

_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


[prev in list] [next in list] [prev in thread] [next in thread]

Configure | About | News | Add a list | Sponsored by KoreLogic