----------------------------------------------------------- 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 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 Same foreach astyle mess-up here. /trunk/koffice/kword/part/KWView.cpp ditto... Best grep for foreach | fgrep -v "{" or something like that. /trunk/koffice/kword/part/dockers/KWRdfDockerTree.cpp 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 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 No eagle eyes, mine, but return type and method name should be on the same line. /trunk/koffice/libs/main/rdf/KoDocumentRdfEditWidget.h 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