D24761: Asign correct column width when importing XLS file

[David Llewellyn-Jones]

davidllewellynjones created this revision.
davidllewellynjones added a project: Calligra: 3.0.
Herald added a subscriber: Calligra-Devel-list.
davidllewellynjones requested review of this revision.

REVISION SUMMARY
  When importing an Excel 97-2003 .xls file (using the excel importer) the 
column widths don't match those from the file exported from Excel (or 
LibreOffice). Moreover, the widths of the columns depend on the dpi of the 
device in use, even though this doesn't affect the contents of the cells.
  
  For any screen that is higher than 96 dpi the columns will be too narrow and 
cell contents are likely to be truncated.
  
  This patch fixes the issue so that the correct column widths are assigned. As 
far as I can tell, this is the only situation where QWidget.physicalDpiX() is 
being explicitly used in the code.
  
  http://www.flypig.co.uk/dnload/dnload/other/calligra-column-widths.zip

TEST PLAN
  1. Download the following archive and extract the test-widths.xls and 
test-widths.xlsx files.
  
  http://www.flypig.co.uk/dnload/dnload/other/calligra-column-widths.zip
  
  2. Open the files. The column widths should be the same.
  
  The archive also contains screenshots of the patched and unpatched versions 
of Callibra using two different spi screens.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D24761

AFFECTED FILES
  filters/sheets/excel/sidewinder/sheet.cpp

To: davidllewellynjones
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D24761: Asign correct column width when importing XLS file

[David Llewellyn-Jones]

davidllewellynjones added reviewers: Calligra: 3.0, pvuorela.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D24761

To: davidllewellynjones, #calligra:_3.0, pvuorela
Cc: davidllewellynjones, Calligra-Devel-list, dcaliste, cochise, vandenoever

D24761: Asign correct column width when importing XLS file

[David Llewellyn-Jones]

davidllewellynjones edited the test plan for this revision.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D24761

To: davidllewellynjones, #calligra:_3.0, pvuorela
Cc: davidllewellynjones, Calligra-Devel-list, dcaliste, cochise, vandenoever

D24943: Better charset, unicode and image support for RTF files

[David Llewellyn-Jones]

davidllewellynjones accepted this revision.
davidllewellynjones added a comment.
This revision is now accepted and ready to land.


  I've made some suggestions and have a couple of queries, but they're all very 
minor.

INLINE COMMENTS

> DocumentDestination.cpp:125
> + m_charactersToSkip = m_unicodeSkip;
> + } else if (controlWord == "uc" && hasValue) {
> + m_unicodeSkip = value;

A minor issue, but the spacing around the brackets here doesn't match the 
surrounding code.

> PictDestination.cpp:46
> + m_format = "png";
> + } else if ( controlWord == "dibitmap" ) {
> + qCDebug(lcRtf) << "BMP";

The spec mentions `\wbitmap` is also a Windows device-dependent bitmap. Could 
that be sent through this branch too?

(see: http://latex2rtf.sourceforge.net/rtfspec_7.html#rtfspec_24)

> PictDestination.cpp:54
>   } else if ( controlWord == "pich" ) {
>  qCDebug(lcRtf) << "pict height: " << value;
>   } else if ( controlWord == "picscalex" ) {

The `\picw` and `\pich` keywords are now ignored; is this intentional? It looks 
like  they're mandatory, whereas the `\picwgoal`, `\pichgoal`, `\picscalex` and 
`\picscaley` which seem to be used now instead, are optional.

(see: http://latex2rtf.sourceforge.net/rtfspec_7.html#rtfspec_24)

> PictDestination.cpp:110
> +qCWarning(lcRtf) << "Embedded picture in unknown format";
> +}
>  }

Lines 87-110 seem to use different spacing around the brackets than elsewhere.

> TextDocumentRtfOutput.cpp:68
> +{
> + m_cursor->insertText(str);
>  }

The tabs/spaces are all over the place in this file already, but it probably 
makes sense to stick to one or the other nevertheless (spaces by the looks of 
it).

REPOSITORY
  R8 Calligra

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D24943

To: pvuorela, davidllewellynjones
Cc: davidllewellynjones, Calligra-Devel-list, dcaliste, cochise, vandenoever

D25008: Add XLSX spreadsheets import optimisations for small/readonly devices

[David Llewellyn-Jones]

davidllewellynjones created this revision.
davidllewellynjones added reviewers: Calligra: 3.0, pvuorela, dcaliste.
davidllewellynjones added a project: Calligra: 3.0.
Herald added a subscriber: Calligra-Devel-list.
davidllewellynjones requested review of this revision.

REVISION SUMMARY
  Loading large XLSX files on resource-constrained devices can be very slow, 
and the problem is made worse by the fact that spreadsheet applications make it 
easy to add styles to large numbers of rows and columns, even though the number 
of cells with actual content may be quite small (e.g. select a single entire 
column/row and apply a style). In this case it's much faster to only convert 
the cells inside the boundary containing values, and ignore the cells beyond 
this which only have styles applied.
  
  This patch adds a number of compile-time defines that can be used to optimise 
loading of XLSX spreadsheet documents on restricted devices.
  
  For the desktop version I expect none of these flags will be particularly 
interesting, so if they're not provided to cmake, the current behaviour is left 
unchanged.
  
  `MSOOXML_MAX_SPREADSHEET_COLS=`
  
  This controls the maximum number of columns that the importer will load. Any 
columns outside the range will be ignored. The default value is 0x7FFF, the 
maximum number of columns supported by Calligra.
  
  `MSOOXML_MAX_SPREADSHEET_ROWS=`
  
  This controls the maximum number of rows that the importer will load. Any 
rows outside the range will be ignored. The default value is 0xF, the 
maximum number of rows supported by Calligra.
  
  `MSOOXML_SPREADSHEET_CONTENT_BORDER=`
  
  If this value is set to a positive value, the importer will calculate the 
smallest spreadsheet size that can accommodate all cells containing values or 
formulae. It adds a border of the given number of cells and uses that for the 
maximum bounds of the imported spreadsheet. This can be useful because it's 
easy to create spreadsheets where large numbers of cells have style attributes 
set, but which in practice only have content in a smaller area of the 
spreadsheet. Setting this value therefore provides an optimised way to import 
the spreadsheet, but with style data lost outside of the bounded region.
  
  If this value is left unset, or is negative, the full spreadsheet will be 
imported.
  
  `MSOOXML_IMPORT_READ_ONLY=`
  
  Formula cells in XLSX documents store both the formula and the calculated 
value at the time the document was saved. When importing a file for read-only 
viewing, the value is more useful than the cell. By setting this flag, the 
importer can optimise by using the values rather than the formulae.

TEST PLAN
  The following example spreadsheet has data in columns IR (252) and IW (257):
  
  http://www.flypig.co.uk/dnload/dnload/other/calligra-optimise01.zip
  
  1. Load the file and notice there's data in columns IR and IW.
  
  2. Following the standard instructions 
, rebuild 
Calligra using the additional flags:
  
cd ~/kde/src/calligra
cmake -DCMAKE_INSTALL_PREFIX=$HOME/kde/inst5 $HOME/kde/src/calligra 
-DCMAKE_BUILD_TYPE=RelWithDebInfo -DPRODUCTSET=SHEETS 
-DMSOOXML_MAX_SPREADSHEET_COLS=0xFF -DMSOOXML_MAX_SPREADSHEET_ROWS=0x1400 
-DMSOOXML_SPREADSHEET_CONTENT_BORDER=5 -DMSOOXML_IMPORT_READ_ONLY=true
make -j6
make install -j6
  
  
  
  3. Open the file with the newly build version of Calligra and notice that the 
less data has been loaded.
  
  Playing around with the file and different values for the flags should give a 
mixture of results.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25008

AFFECTED FILES
  CMakeLists.txt
  filters/libmsooxml/MsooXmlGlobal.cpp
  filters/libmsooxml/MsooXmlGlobal.h
  filters/sheets/xlsx/XlsxXmlWorksheetReader.cpp
  filters/sheets/xlsx/XlsxXmlWorksheetReader_p.h

To: davidllewellynjones, #calligra:_3.0, pvuorela, dcaliste
Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, cochise, vandenoever

D25008: Add XLSX spreadsheets import optimisations for small/readonly devices

[David Llewellyn-Jones]

davidllewellynjones updated this revision to Diff 68954.
davidllewellynjones added a comment.


  Thanks @dcaliste, this is an excellent suggestion and much better naming. 
I've updated the diff to make the changes you suggested.
  
  - MSOOXML_IMPORT_READ_ONLY -> MSOOXML_IMPORT_BY_VALUES
  - MSOOXML::readonlySpreadsheet() -> MSOOXML::byValuesSpreadsheet()

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25008?vs=68885&id=68954

REVISION DETAIL
  https://phabricator.kde.org/D25008

AFFECTED FILES
  CMakeLists.txt
  filters/libmsooxml/MsooXmlGlobal.cpp
  filters/libmsooxml/MsooXmlGlobal.h
  filters/sheets/xlsx/XlsxXmlWorksheetReader.cpp
  filters/sheets/xlsx/XlsxXmlWorksheetReader_p.h

To: davidllewellynjones, #calligra:_3.0, pvuorela, dcaliste
Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, cochise, vandenoever

D24761: Asign correct column width when importing XLS file

[David Llewellyn-Jones]

davidllewellynjones updated this revision to Diff 68959.
davidllewellynjones added a comment.


  Thanks here also @dcaliste. This is all a bit confusing I agree and deserves 
some explanation. I've added in something along the lines you suggested, which 
I think helps (your insight that the scaling has to match the font DPI really 
makes it clear).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24761?vs=68246&id=68959

REVISION DETAIL
  https://phabricator.kde.org/D24761

AFFECTED FILES
  filters/sheets/excel/sidewinder/sheet.cpp

To: davidllewellynjones, #calligra:_3.0, pvuorela
Cc: dcaliste, davidllewellynjones, Calligra-Devel-list, cochise, vandenoever

D24943: Better charset, unicode and image support for RTF files

[David Llewellyn-Jones]

davidllewellynjones added inline comments.

INLINE COMMENTS

> PictDestination.cpp:101
> +}
> +}
> +

The logic is confusing me here I'm afraid. If there's no `\picwgoal` control 
word, then `m_goalWidth` will default to 0 and the code above will be skipped. 
The image width will then be set to 0 on line 104: `m_imageFormat.setWidth( 0 
);`. Similarly for height. Is this correct? It feels like it might make more 
sense for the conditions to be reversed.

> denexter wrote in PictDestination.cpp:54
> I think picw and pich are source sizes and somewhat meaningless since that 
> information is encoded in the image. The goal size and scale combine to give 
> the display size.

Thanks for the explanation, that makes sense. Probably these control words are 
more important for WMFs and the like.

> pvuorela wrote in PictDestination.cpp:110
> Yea, could change these too. Though have been pondering of just feeding the 
> whole rtf-qt through astyle to get consistent formatting and whitespace.

I think it needs it, but I appreciate that's not really related to this change.

REPOSITORY
  R8 Calligra

BRANCH
  unicode

REVISION DETAIL
  https://phabricator.kde.org/D24943

To: pvuorela, davidllewellynjones
Cc: denexter, davidllewellynjones, Calligra-Devel-list, dcaliste, cochise, 
vandenoever

D24943: Better charset, unicode and image support for RTF files

[David Llewellyn-Jones]

davidllewellynjones accepted this revision.
davidllewellynjones added a comment.


  This looks like a nice and useful collection of changes to me.

REPOSITORY
  R8 Calligra

BRANCH
  unicode

REVISION DETAIL
  https://phabricator.kde.org/D24943

To: pvuorela, davidllewellynjones
Cc: denexter, davidllewellynjones, Calligra-Devel-list, dcaliste, cochise, 
vandenoever

D25008: Add XLSX spreadsheets import optimisations for small/readonly devices

[David Llewellyn-Jones]

davidllewellynjones added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in CMakeLists.txt:124-129
> Why 2 times?

That's a very good question; nicely noticed. I'll update the patch to remove 
one.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25008

To: davidllewellynjones, #calligra:_3.0, pvuorela, dcaliste
Cc: anthonyfieroni, Calligra-Devel-list, davidllewellynjones, dcaliste, 
cochise, vandenoever

D25008: Add XLSX spreadsheets import optimisations for small/readonly devices

[David Llewellyn-Jones]

davidllewellynjones updated this revision to Diff 69054.
davidllewellynjones added a comment.


  Remove duplicated cmake definition.

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25008?vs=68954&id=69054

REVISION DETAIL
  https://phabricator.kde.org/D25008

AFFECTED FILES
  CMakeLists.txt
  filters/libmsooxml/MsooXmlGlobal.cpp
  filters/libmsooxml/MsooXmlGlobal.h
  filters/sheets/xlsx/XlsxXmlWorksheetReader.cpp
  filters/sheets/xlsx/XlsxXmlWorksheetReader_p.h

To: davidllewellynjones, #calligra:_3.0, pvuorela, dcaliste
Cc: anthonyfieroni, Calligra-Devel-list, davidllewellynjones, dcaliste, 
cochise, vandenoever

D25008: Add XLSX spreadsheets import optimisations for small/readonly devices

[David Llewellyn-Jones]

davidllewellynjones marked 2 inline comments as done.
davidllewellynjones added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in CMakeLists.txt:124-129
> Why 2 times?

Thanks again, I've removed the duplication now.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25008

To: davidllewellynjones, #calligra:_3.0, pvuorela, dcaliste
Cc: anthonyfieroni, Calligra-Devel-list, davidllewellynjones, dcaliste, 
cochise, vandenoever

D25256: Allow non-conforming LibreOffice PPT files to be imported

[David Llewellyn-Jones]

davidllewellynjones created this revision.
davidllewellynjones added reviewers: pvuorela, dcaliste.
davidllewellynjones added a project: Calligra: 3.0.
Herald added a subscriber: Calligra-Devel-list.
davidllewellynjones requested review of this revision.

REVISION SUMMARY
  An apparent bug in the LibreOffice PPT exporter makes it output files which 
technically don't conform to the PPT specification. Calligra refuses to load 
these files, which although technically may be the correct behaviour, is 
extremely annoying for the user. LibreOffice's deviation from the PPT spec is 
pretty minor, and a slight weakening of Calligra's validation allows the files 
to be imported successfully.
  
  In more detail, when loading a drawing each text paragraph in the drawing has 
a TextPFRun structure ("A structure that specifies the paragraph-level 
formatting of a run of text"). This starts with a mask, followed by a sequence 
of fields. Only unmasked fields are included in the sequence.
  
  According to Section 2.9.45 of the PPT specification version 6 
, the 
following fields must be masked out:
  
masks.leftMargin
masks.indent
masks.defaultTabSize
masks.tabStops
  
  In spite of this LibreOffice includes the `leftMargin` and `indent` fields 
(flags 0x100 and 0x400). I'm not familiar with the LibreOffice codebase, but it 
looks like this 

 is the problem code. From this same code it look like LibreOffice doesn't 
export the `defaultTabSize` or `tabStops` fields (which is correct).
  
  This patch loosens Calligra's validation to allow these flags to be set. I've 
tested this with a bunch of files which previously failed to load, including 
quite complex ones, and they all seem to load fine once the patch is applied.
  
  A couple of important notes.
  
  1. The validation code is generated by binschema 
 and I'll submit a separate patch there.
  2. The `calligra/filters/libmso/generated/mso.jar` file also needs to be 
updated to this version , 
but I couldn't see a way to include the binary file in with this patch.

TEST PLAN
  1. Save out a file from LibreOffice in PPT format, or download this archive 
 with a 
test file inside.
  2. Attempt to load the file into Calligra Stage.
  3. Note that it refuses to load with the error "Invalid file format".
  4. Apply the patch.
  5. Attempt to load the same file again.
  6. Note that it loads correctly. If you used my test file, witness my amazing 
presentation design.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25256

AFFECTED FILES
  filters/libmso/generated/simpleParser.cpp
  filters/libmso/generated/simpleParser.h

To: davidllewellynjones, pvuorela, dcaliste
Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, cochise, vandenoever

D25256: Allow non-conforming LibreOffice PPT files to be imported

[David Llewellyn-Jones]

davidllewellynjones added a comment.


  Here's the corresponding patch for binschema: 
https://github.com/KDE/binschema/pull/1

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25256

To: davidllewellynjones, pvuorela, dcaliste
Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, cochise, vandenoever

D25256: Relax TextPFRun validation to allow LibreOffice PPT import

[David Llewellyn-Jones]

davidllewellynjones updated this revision to Diff 70039.
davidllewellynjones retitled this revision from "Allow non-conforming 
LibreOffice PPT files to be imported" to "Relax TextPFRun validation to allow 
LibreOffice PPT import".
davidllewellynjones edited the summary of this revision.
davidllewellynjones edited the test plan for this revision.
davidllewellynjones added a comment.


  - SVN_SILENT made messages (.desktop file) - always resolve ours
  - SVN_SILENT made messages (.desktop file) - always resolve ours
  - SVN_SILENT made messages (.desktop file) - always resolve ours
  - Relax TextPFRun validation to allow LibreOffice PPT import
  
  1. Updating D25256 : Relax TextPFRun 
validation to allow LibreOffice PPT import #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create
  
  Align with binschema changes
  
  This updates the diff to align with the changes accepted to binschema 
updating mso.xml. These changes are then used to generate the files here. This 
change also now includes the mso.jar binary file.

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25256?vs=69581&id=70039

BRANCH
  allow-libreoffice-ppt-validation (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25256

AFFECTED FILES
  filters/libmso/generated/mso.jar
  filters/libmso/generated/simpleParser.cpp
  filters/libmso/generated/simpleParser.h
  filters/words/rtf/import/3rdparty/rtf-qt/src/controlword.cpp
  karbon/templates/basic/empty.desktop
  libs/flake/KoSnapStrategy.cpp
  plugins/stencilsdocker/stencils/BPMN/Text-Annotation.desktop
  sheets/data/templates/HomeFamily/MenuPlan.desktop

To: davidllewellynjones, pvuorela, dcaliste
Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, 
vandenoever

D25256: Relax TextPFRun validation to allow LibreOffice PPT import

[David Llewellyn-Jones]

davidllewellynjones updated this revision to Diff 70040.
davidllewellynjones added a comment.


  
  
  1. Updating D25256 : Relax TextPFRun 
validation to allow LibreOffice PPT import #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create
  
  Remove previous commits to master that got mixed in accidentally.

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25256?vs=70039&id=70040

BRANCH
  allow-libreoffice-ppt-validation (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25256

AFFECTED FILES
  filters/libmso/generated/mso.jar
  filters/libmso/generated/simpleParser.cpp
  filters/libmso/generated/simpleParser.h

To: davidllewellynjones, pvuorela, dcaliste
Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, 
vandenoever

D25256: Relax TextPFRun validation to allow LibreOffice PPT import

[David Llewellyn-Jones]

davidllewellynjones updated this revision to Diff 70041.
davidllewellynjones edited the summary of this revision.
davidllewellynjones added a comment.


  
  
  1. Updating D25256 : Relax TextPFRun 
validation to allow LibreOffice PPT import #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create
  
  Remove excess spaces from commit message.

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25256?vs=70040&id=70041

BRANCH
  allow-libreoffice-ppt-validation (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D25256

AFFECTED FILES
  filters/libmso/generated/mso.jar
  filters/libmso/generated/simpleParser.cpp
  filters/libmso/generated/simpleParser.h

To: davidllewellynjones, pvuorela, dcaliste
Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, 
vandenoever

D25715: Use RTF default color as default Qt format

[David Llewellyn-Jones]

davidllewellynjones added inline comments.

INLINE COMMENTS

> ColorTableDestination.cpp:44
> + handled = false;
>  qCDebug(lcRtf) << "unexpected control word in colortbl:" << 
> controlWord;
>   }

It looks like there may be a distinction to be made between an empty control 
word and an unhandled control word. In the case of an unhandled control word, 
clearing the colour seems sensible, but then in the case of an empty control 
word, the debug output isn't needed.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D25715

To: pvuorela, davidllewellynjones, dcaliste
Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, 
vandenoever

D25715: Use RTF default color as default Qt format

[David Llewellyn-Jones]

davidllewellynjones accepted this revision.
davidllewellynjones added a comment.
This revision is now accepted and ready to land.


  This looks good to me, and certainly gave better results for me using files 
with the 'auto' colour.

INLINE COMMENTS

> pvuorela wrote in ColorTableDestination.cpp:44
> Or well, the code might return such, but think the file in that case is 
> broken and as such ok to complain about on debug. Also a bit separate from 
> the changes here.

Yeah, this was my misunderstanding of the spec. I don't see a problem with what 
you're doing here.

REPOSITORY
  R8 Calligra

BRANCH
  rtf_default_color

REVISION DETAIL
  https://phabricator.kde.org/D25715

To: pvuorela, davidllewellynjones, dcaliste
Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, 
vandenoever

D27694: Don't recreate ImageDataItem texture unless needed

[David Llewellyn-Jones]

davidllewellynjones created this revision.
davidllewellynjones added reviewers: Calligra: 3.0, leinir, danders.
davidllewellynjones added a project: Calligra: 3.0.
Herald added a subscriber: Calligra-Devel-list.
davidllewellynjones requested review of this revision.

REVISION SUMMARY
  Submitting on behalf of Joona Petrell.
  
  This change improves performance by only updating the image texture if the 
image data changes.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D27694

AFFECTED FILES
  components/ImageDataItem.cpp

To: davidllewellynjones, #calligra:_3.0, leinir, danders
Cc: Calligra-Devel-list, davidllewellynjones, dcaliste, ognarb, cochise, 
vandenoever

T12815: Create Calligra Framework by separating out applications and libraries

[David Llewellyn-Jones]

davidllewellynjones added a comment.


  As a newcomer, and only occasional contributor, my experience of getting the 
build up and running is that it's already a daunting process, made tractable 
only because the instructions  
are very good. Anything that simplifies these instructions is good, anything 
that complicates them is bad.
  
  If I'm honest, having everything in one repository probably made things 
simpler. I'm sure separate repos could be made easy too (e.g. using 
submodules), but also introduces more scope for errors that are difficult for 
an occasional contributor (i.e. me) to solve.
  
  In T12815#223419 , @dcaliste wrote:
  
  > But what I like in the proposition of @leinir is the target to separate the 
"engine" from the UI (_i.e._ the widgetery). As an example (hopefully not 
outdated), the "find" classes, the one to search and replace, are deeply liked 
to the widgets that control them. It makes it impossible to reuse these class 
in a UI different from the original one.
  
  
  Having said all that above, as a developer for Sailfish OS (but speaking 
personally), I would completely second @dcaliste's point. Whatever the delivery 
structure, better decoupling between components would make Calligra more 
flexible and usable in other contexts. The split between the chrome and the 
document rendering is a very clear case, and if splitting into repos helps 
reinforce this, then I'd support it.

TASK DETAIL
  https://phabricator.kde.org/T12815

To: davidllewellynjones
Cc: davidllewellynjones, ndavis, jtamate, rempt, anthonyfieroni, dcaliste, 
boemann, pino, rjvbb, ngraham, ognarb, Calligra-Devel-list, #calligra:_3.0, 
leinir, cochise, vandenoever