<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/100304/">http://git.reviewboard.kde.org/r/100304/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 11th, 2011, 6:42 a.m., <b>Mani Chandrasekar</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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. </pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
<br />
<p>- Jarosław</p>
<br />
<p>On January 5th, 2011, 12:22 a.m., Jarosław Staniek wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Calligra.</div>
<div>By Jarosław Staniek.</div>
<p style="color: grey;"><i>Updated Jan. 5, 2011, 12:22 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(d58e152)</span></li>
<li>tools/CMakeLists.txt <span style="color: grey">(a10cd08)</span></li>
<li>tools/f-office/CMakeLists.txt <span style="color: grey">(cd2c6bd)</span></li>
<li>tools/f-office/Common.h <span style="color: grey">(30b31f6)</span></li>
<li>tools/f-office/FoCellEditor.h <span style="color: grey">(c50fcbd)</span></li>
<li>tools/f-office/FoCellEditor.cpp <span style="color: grey">(13038d4)</span></li>
<li>tools/f-office/FoCellTool.h <span style="color: grey">(3128050)</span></li>
<li>tools/f-office/FoCellTool.cpp <span style="color: grey">(63ce11f)</span></li>
<li>tools/f-office/FoCellToolFactory.h <span style="color: grey">(d29de43)</span></li>
<li>tools/f-office/FoCellToolFactory.cpp <span style="color: grey">(da4339b)</span></li>
<li>tools/f-office/FoExternalEditor.h <span style="color: grey">(cab8a87)</span></li>
<li>tools/f-office/FoExternalEditor.cpp <span style="color: grey">(bfc232f)</span></li>
<li>tools/f-office/FoImageSelectionWidget.h <span style="color: grey">(dddc61b)</span></li>
<li>tools/f-office/FoImageSelectionWidget.cpp <span style="color: grey">(0da2880)</span></li>
<li>tools/f-office/KoAbstractApplication.h <span style="color: grey">(47c7f08)</span></li>
<li>tools/f-office/KoAbstractApplicationController.h <span style="color: grey">(7293b4e)</span></li>
<li>tools/f-office/KoAbstractApplicationController.cpp <span style="color: grey">(d3aa2bd)</span></li>
<li>tools/f-office/KoAbstractApplicationImpl.h <span style="color: grey">(469430e)</span></li>
<li>tools/f-office/KoAbstractApplicationImpl.cpp <span style="color: grey">(01014f2)</span></li>
<li>tools/f-office/Main.cpp <span style="color: grey">(ca88305)</span></li>
<li>tools/f-office/MainWindow.h <span style="color: grey">(937a16a)</span></li>
<li>tools/f-office/MainWindow.cpp <span style="color: grey">(3110056)</span></li>
<li>tools/f-office/NotifyDialog.h <span style="color: grey">(c51fee7)</span></li>
<li>tools/f-office/NotifyDialog.cpp <span style="color: grey">(e9ecab2)</span></li>
<li>tools/f-office/RemoveSheet.h <span style="color: grey">(d44351c)</span></li>
<li>tools/f-office/RemoveSheet.cpp <span style="color: grey">(9f617b9)</span></li>
<li>tools/koabstraction/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/FoCellEditor.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/FoCellEditor.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/KoAbstractApplication.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/KoAbstractApplicationController.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/KoAbstractApplicationController.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/KoAbstractApplicationImpl.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/KoCellTool.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/KoCellTool.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/KoCellToolFactory.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/KoCellToolFactory.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/KoExternalEditorInterface.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/RemoveSheetCommand.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/RemoveSheetCommand.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tools/koabstraction/koabstraction_export.h <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100304/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>