Review Request: KoAbstraction Library, iteration #3

Jarosław Staniek staniek at kde.org
Tue Jan 11 08:13:26 GMT 2011 


> On Jan. 11, 2011, 6:42 a.m., Mani Chandrasekar wrote:
> > Patch looks good to me. 
> > 
> > I have a suggestion for Abstraction library, 
> > As I see it growing generic and complete, Do we still need to have it inside tools directory? I understand that it can be build as a library, But won't it make sense to put it inside libs folder ship it with Calligra package and any applications using it can link against it.

Thanks for the review, Mani.
Yes, I am willing to move the library to libs/. To avoid addding complexity to the current patch, that would be a separate patch. Something like calligraappabstraction lib and lib/abstraction/ dir.


- Jarosław


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


On Jan. 5, 2011, 12:22 a.m., Jarosław Staniek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100304/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2011, 12:22 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> KoAbstraction Library
> 
> This is the third iteration, the previous (http://reviewboard.kde.org/r/5635/) was discarded because works on this one were already quite advanced.
> 
> More discussion at http://community.kde.org/Calligra/Libs/KoAbstraction
> 
> == Here's a copy from http://reviewboard.kde.org/r/5635/:
> 
> This is library utilizing facade design pattern in order to simplify implementation of custom graphical interfaces for various applications. This way standalone KOffice viewer widget can be delivered too.
> 
> * added koabstraction in tools/koabstraction/
> * it's dependent on KSpread so for my understanding does not fit to libs/
> * moved code that belong to koabstraction from f-office/, that is:
> ** KoAbstractApplication intermediate class
> ** KoAbstractApplicationController
> ** FoCellTool (renamed to KoCellTool)
> ** FoCellToolFactory (renamed to KoCellToolFactory, made private class)
> ** RemoveSheet (renamed to RemoveSheetCommand for more clarity, made private class)
> * FoCellEditor temporarily moved to koabstraction/ as private class, will be back here as soon as interface abstracting cell editor is ready
> * finished simplifying f-office/CMakeLists.txt (it looked like standalone project before with using find_package(Qt4 4.5.0 REQUIRED), etc.)
> * KoAbstractApplicationImpl.cpp stays in tools/f-office/ as it's a wrapper for building KoAbstractApplication.moc properly
> * added KoExternalEditorInterface class which is implemented by FoExternalEditor in f-office, abstraction uses it instead of concrete implementation of editor
> * FoImageSelectionWidget is not instatiated at all so constructor was removed
> * PanTool_ID, TextTool_ID, CellTool_ID constants from f-office replaced by KoAbstractApplicationController::panToolFactoryId(), etc. in koabstraction, so the tools can be freely accessed in any implementation
> * KoAbstractApplicationController uses controller() which is now KoController only; in contexts when KoControllerWidget is needed, appropriate dynamic_case is defined as KoAbstractApplicationController::controllerWidget(); this leads to API that could be usable for graphics-view-based UIs
> * PresentationTool is back in f-office's MainWindow now, same for StoreButtonPreview; code related to them is moved from abstraction to f-office
> 
> == Changes compared to http://reviewboard.kde.org/r/5635/:
> 
> * ported to Calligra: renamed namespaces, libs and paths; full namespaces used everywhere for improved readability
> * as requested by maemo devs, f-office/ now builds using non-kde4 buildsystem (no automoc)
> * KoAbstractApplicationOpenDocumentArguments added, provided setting on/off editing as default mode, implementation-independent way for multiple documents support (e.g. using multiple instances), and opening document as template
> * editing mode is now a flag of the controller, is set/unset automatically when needed by the abstraction, can be also set by the GUIs
> * added KoAbstractApplication::openDocumentAsTemplate()
> * temporarily, MS documents are saved as ODF counterparts because of mixed quality of filters
> * pointer optional to a splash screen moved to abstraction, displayed/closed when necessary
> * KoAbstractApplicationController:
> ** added supportedExtensions() and added supportedFilters() for improved code reuse
> ** added documentFileName() that can be used by GUIs
> ** added setHorizontalScrollBarVisible(bool), setVerticalScrollBarVisible(bool) abstractions
> ** added setOnlyDisplayDocumentNameInTitle(bool): if set to true only document name is displayed and not application name; This is to conserve space on small displays
> 
> Summing up, these are modified paths:
> 
> #       modified:   CMakeLists.txt
> #       modified:   tools/CMakeLists.txt
> #       modified:   tools/f-office/CMakeLists.txt
> #       modified:   tools/f-office/Common.h
> #       modified:   tools/f-office/FoExternalEditor.cpp
> #       modified:   tools/f-office/FoExternalEditor.h
> #       modified:   tools/f-office/FoImageSelectionWidget.cpp
> #       modified:   tools/f-office/FoImageSelectionWidget.h
> #       modified:   tools/f-office/KoAbstractApplicationImpl.cpp
> #       modified:   tools/f-office/Main.cpp
> #       modified:   tools/f-office/MainWindow.cpp
> #       modified:   tools/f-office/MainWindow.h
> #       modified:   tools/f-office/NotifyDialog.cpp
> #       modified:   tools/f-office/NotifyDialog.h
> #       new file:   tools/koabstraction/CMakeLists.txt
> #       renamed:    tools/f-office/FoCellEditor.cpp -> tools/koabstraction/FoCellEditor.cpp
> #       renamed:    tools/f-office/FoCellEditor.h -> tools/koabstraction/FoCellEditor.h
> #       renamed:    tools/f-office/KoAbstractApplication.h -> tools/koabstraction/KoAbstractApplication.h
> #       renamed:    tools/f-office/KoAbstractApplicationController.cpp -> tools/koabstraction/KoAbstractApplicationController.cpp
> #       renamed:    tools/f-office/KoAbstractApplicationController.h -> tools/koabstraction/KoAbstractApplicationController.h
> #       renamed:    tools/f-office/KoAbstractApplicationImpl.h -> tools/koabstraction/KoAbstractApplicationImpl.h
> #       renamed:    tools/f-office/FoCellTool.cpp -> tools/koabstraction/KoCellTool.cpp
> #       renamed:    tools/f-office/FoCellTool.h -> tools/koabstraction/KoCellTool.h
> #       renamed:    tools/f-office/FoCellToolFactory.cpp -> tools/koabstraction/KoCellToolFactory.cpp
> #       renamed:    tools/f-office/FoCellToolFactory.h -> tools/koabstraction/KoCellToolFactory.h
> #       new file:   tools/koabstraction/KoExternalEditorInterface.h
> #       renamed:    tools/f-office/RemoveSheet.cpp -> tools/koabstraction/RemoveSheetCommand.cpp
> #       renamed:    tools/f-office/RemoveSheet.h -> tools/koabstraction/RemoveSheetCommand.h
> #       new file:   tools/koabstraction/koabstraction_export.h
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt d58e152 
>   tools/CMakeLists.txt a10cd08 
>   tools/f-office/CMakeLists.txt cd2c6bd 
>   tools/f-office/Common.h 30b31f6 
>   tools/f-office/FoCellEditor.h c50fcbd 
>   tools/f-office/FoCellEditor.cpp 13038d4 
>   tools/f-office/FoCellTool.h 3128050 
>   tools/f-office/FoCellTool.cpp 63ce11f 
>   tools/f-office/FoCellToolFactory.h d29de43 
>   tools/f-office/FoCellToolFactory.cpp da4339b 
>   tools/f-office/FoExternalEditor.h cab8a87 
>   tools/f-office/FoExternalEditor.cpp bfc232f 
>   tools/f-office/FoImageSelectionWidget.h dddc61b 
>   tools/f-office/FoImageSelectionWidget.cpp 0da2880 
>   tools/f-office/KoAbstractApplication.h 47c7f08 
>   tools/f-office/KoAbstractApplicationController.h 7293b4e 
>   tools/f-office/KoAbstractApplicationController.cpp d3aa2bd 
>   tools/f-office/KoAbstractApplicationImpl.h 469430e 
>   tools/f-office/KoAbstractApplicationImpl.cpp 01014f2 
>   tools/f-office/Main.cpp ca88305 
>   tools/f-office/MainWindow.h 937a16a 
>   tools/f-office/MainWindow.cpp 3110056 
>   tools/f-office/NotifyDialog.h c51fee7 
>   tools/f-office/NotifyDialog.cpp e9ecab2 
>   tools/f-office/RemoveSheet.h d44351c 
>   tools/f-office/RemoveSheet.cpp 9f617b9 
>   tools/koabstraction/CMakeLists.txt PRE-CREATION 
>   tools/koabstraction/FoCellEditor.h PRE-CREATION 
>   tools/koabstraction/FoCellEditor.cpp PRE-CREATION 
>   tools/koabstraction/KoAbstractApplication.h PRE-CREATION 
>   tools/koabstraction/KoAbstractApplicationController.h PRE-CREATION 
>   tools/koabstraction/KoAbstractApplicationController.cpp PRE-CREATION 
>   tools/koabstraction/KoAbstractApplicationImpl.h PRE-CREATION 
>   tools/koabstraction/KoCellTool.h PRE-CREATION 
>   tools/koabstraction/KoCellTool.cpp PRE-CREATION 
>   tools/koabstraction/KoCellToolFactory.h PRE-CREATION 
>   tools/koabstraction/KoCellToolFactory.cpp PRE-CREATION 
>   tools/koabstraction/KoExternalEditorInterface.h PRE-CREATION 
>   tools/koabstraction/RemoveSheetCommand.h PRE-CREATION 
>   tools/koabstraction/RemoveSheetCommand.cpp PRE-CREATION 
>   tools/koabstraction/koabstraction_export.h PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/100304/diff
> 
> 
> Testing
> -------
> 
> FreOffice builds and runs properly for any of the three document types. Did some testing on standalone minimal test application which implements the interfaces - documents are opened and displayed properly.
> 
> 
> Thanks,
> 
> Jarosław
> 
>

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

More information about the calligra-devel mailing list