Re: Review Request 129512: [stencil] Follow system color for stencilsdocker icons

[Dag Andersen]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129512/#review101106
---


Ship it!




Ship It!

- Dag Andersen


On Nov. 21, 2016, 4:54 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129512/
> ---
> 
> (Updated Nov. 21, 2016, 4:54 a.m.)
> 
> 
> Review request for Calligra and Camilla Boemann.
> 
> 
> Repository: calligra
> 
> 
> Description
> ---
> 
> Still has icons who are drown incorrect, i blame factory, but need more 
> invetigates.
> 
> 
> Diffs
> -
> 
>   plugins/stencilsdocker/CMakeLists.txt 43a4e3c 
>   plugins/stencilsdocker/StencilBoxDocker.cpp 3e4da8c 
> 
> Diff: https://git.reviewboard.kde.org/r/129512/diff/
> 
> 
> Testing
> ---
> 
> Build and run with Karbon.
> PS: Email me if you notice any other downsides :)
> 
> 
> File Attachments
> 
> 
> White theme
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/11/20/74f03c72-491f-478e-a082-9c5e3e072999__Screenshot_20161120_213518.png
> Black theme
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/11/20/b96331ae-db38-455f-a715-50101c30a3d0__Screenshot_20161120_213613.png
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129800: [Mac] : prepare for "linuxy" vs. standalone app bundle builds

[Dag Andersen]

> On Jan. 10, 2017, 8:28 p.m., Anthony Fieroni wrote:
> > So when Karbon works, i'm +1, please try words / sheets are they worked as 
> > well as Karbon?
> 
> René J.V. Bertin wrote:
> I cannot tell yet, but I'm not expecting any issues because of this 
> patch. As you can see it only touches common/shared code at the moment, and I 
> am NOT building with `-DAPPLE_STANDALONE_BUILD`.
> 
> I *think* I should have most external dependencies installed for Words 
> and Sheets, so I'll be looking at building them in the near future, and 
> update this ticket accordingly.
> 
> I'm my point of view that's icing on the cake that can come after we've 
> reached feature parity with Linux. I can imagine others think different 
> (that's what Macs are for ;)). Hence this patch, it'll make it easier to 
> follow both approaches simultaneously.
> 
> René J.V. Bertin wrote:
> BTW: I do hope for some collaborative brainstorming w.r.t. using info 
> from KoResourcePaths.h more widely, instead of using QStandardPaths types 
> directly. That's a rather delicate intervention which I'd prefer not to take 
> the sole responsibility for.
> 
> René J.V. Bertin wrote:
> I can confirm Words works too with this patch.
> 
> (I did have to fix a small issue in the words-odf filter, I've taken the 
> liberty to push the fix: 
> https://cgit.kde.org/calligra.git/commit/?id=a8fd10d8b0a24e581eeb4754b458ba98ddbf0167)
> 
> René J.V. Bertin wrote:
> Sheets works nicely too!
> 
> Anthony Fieroni wrote:
> I response for only Karbon, wait for Camilla and Dag Andersen.
> 
> René J.V. Bertin wrote:
> Evidently.
> 
> Plan also works as far as I can tell (I never used it before).
> 
> There is an issue with `calligraplanwork` however, but given the error I 
> think it has nothing to do with my changes. If anything, I may have missed a 
> location where a change is required, for instance to install required DBus 
> service files.

Afaics this should be ok.
The defines in KoResourcePaths.h doesn't seem to be used?
Can't comment on apple stuff, never seen one close up ;)


- Dag


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129800/#review101926
---


On Jan. 10, 2017, 4:34 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129800/
> ---
> 
> (Updated Jan. 10, 2017, 4:34 p.m.)
> 
> 
> Review request for Calligra and Camilla Boemann.
> 
> 
> Repository: calligra
> 
> 
> Description
> ---
> 
> This proposal is an initial implementation of things discussed on the ML a 
> short while back.
> 
> Building KF5 software on Mac as if on any Unix variant (with "Cocoa" instead 
> of X11) is possible and is what you get without specific changes to the build 
> system. With a few tweaks to Qt's QStandardPaths (provided by MacPorts) this 
> kind of build works flawlessly and with an identical feature set as on Linux 
> c.s.
> 
> NB: this build also puts applications in an app bundle "wrapper", but one 
> that contains just the minimal resources (executable, app icon and 
> Info.plist).
>  
> One can also tweak the build so that a relocatable and standalone app bundle 
> results which contains the application and all its 3rd-party dependencies 
> (Qt5, KF5 frameworks, etc.). This works with a stock Qt5 build but still 
> requires patches throughout KF5 code and build systems. Several projects 
> already provide "official" builds of this type for Mac: Kate, KDevelop, 
> Marble and Krita to name a few.
> 
> The current patch prepares for allowing a choice for either a standalone app 
> bundle build or a more traditional build via a CMake option 
> `APPLE_STANDALONE_BUNDLE` and preprocessor token of the same name. This makes 
> it easy to dissociate the Apple build types from general Apple build 
> requirements. Testing for build flavour is done by checking 
> `APPLE_STANDALONE_BUNDLE`, testing for build platform by checking `APPLE` 
> (CMake) or `Q_OS_MACOS` (Qt/C++) (or `__APPLE__` in code not using Qt).
> 
> In addition to the introduction of the CMake option, the patch
> 
> - updates `KoApplication::start()`. Judging from Kate's approach it shouldnt' 
> be necessary on Mac to set `XDG_DATA_DIRS`, which isn't used anywhere in code 
> (e

Re: Review Request 129800: [Mac] : prepare for "linuxy" vs. standalone app bundle builds

[Dag Andersen]

> On Jan. 10, 2017, 8:28 p.m., Anthony Fieroni wrote:
> > So when Karbon works, i'm +1, please try words / sheets are they worked as 
> > well as Karbon?
> 
> René J.V. Bertin wrote:
> I cannot tell yet, but I'm not expecting any issues because of this 
> patch. As you can see it only touches common/shared code at the moment, and I 
> am NOT building with `-DAPPLE_STANDALONE_BUILD`.
> 
> I *think* I should have most external dependencies installed for Words 
> and Sheets, so I'll be looking at building them in the near future, and 
> update this ticket accordingly.
> 
> I'm my point of view that's icing on the cake that can come after we've 
> reached feature parity with Linux. I can imagine others think different 
> (that's what Macs are for ;)). Hence this patch, it'll make it easier to 
> follow both approaches simultaneously.
> 
> René J.V. Bertin wrote:
> BTW: I do hope for some collaborative brainstorming w.r.t. using info 
> from KoResourcePaths.h more widely, instead of using QStandardPaths types 
> directly. That's a rather delicate intervention which I'd prefer not to take 
> the sole responsibility for.
> 
> René J.V. Bertin wrote:
> I can confirm Words works too with this patch.
> 
> (I did have to fix a small issue in the words-odf filter, I've taken the 
> liberty to push the fix: 
> https://cgit.kde.org/calligra.git/commit/?id=a8fd10d8b0a24e581eeb4754b458ba98ddbf0167)
> 
> René J.V. Bertin wrote:
> Sheets works nicely too!
> 
> Anthony Fieroni wrote:
> I response for only Karbon, wait for Camilla and Dag Andersen.
> 
> René J.V. Bertin wrote:
> Evidently.
> 
> Plan also works as far as I can tell (I never used it before).
> 
> There is an issue with `calligraplanwork` however, but given the error I 
> think it has nothing to do with my changes. If anything, I may have missed a 
> location where a change is required, for instance to install required DBus 
> service files.
> 
> Dag Andersen wrote:
> Afaics this should be ok.
> The defines in KoResourcePaths.h doesn't seem to be used?
> Can't comment on apple stuff, never seen one close up ;)
> 
> René J.V. Bertin wrote:
> > The defines in KoResourcePaths.h doesn't seem to be used?
> 
> No indeed. As I tried to explain in the description, they're more a 
> proposal:
> 
> > the change to KoResourcePathsImpl::mapTypeToQStandardPaths() only has a 
> real interest if it's used throughout the code to replace the explicit use of 
> QStandardPaths locations.
> > For now I have set this up through build-type specific preprocessor 
> macros in KoResourcePaths.h [...]
> 
> I don't want to start changing a huge amount of unknown code to use those 
> macros because there are lots of places that would have to be changed, and I 
> don't think it can be done by a few simple search-and-replace runs in an 
> editor. Above all, I want to be sure that you agree about this approach.
> 
> And let me repeat: this isn't necessary for the kind of build that 
> interests me on Mac, so there's no hurry as far as I'm concerned ;)

Afaik KoResourcePaths is used in all calligra.
There are issues and I have a patch waiting (https://phabricator.kde.org/D2577) 
that didn't get into 3.0.
It's fairly big so I'm waiting for 3.1 to look at it again.

Different thing: in KoApplication line 242 you added a comment:
// add the paths that Krita5 sets in XDG_DATA_DIRS
Krita is not using any calligra libs anymore, so this could possibly be removed 
but I don't know if any other app depends on this, so...


- Dag


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129800/#review101926
---


On Jan. 10, 2017, 4:34 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129800/
> ---
> 
> (Updated Jan. 10, 2017, 4:34 p.m.)
> 
> 
> Review request for Calligra and Camilla Boemann.
> 
> 
> Repository: calligra
> 
> 
> Description
> ---
> 
> This proposal is an initial implementation of things discussed on the ML a 
> short while back.
> 
> Building KF5 software on Mac as if on any Unix variant (with "Cocoa" instead 
> of X11) is possible and is what you get without specific changes to the build 

D8310: Port Calligra Gemini for 3.x

[Dag Andersen]

danders added a comment.


  I'm clueless on qtquick, but unless somebody else has comments I'd say ship 
it.

REPOSITORY
  R8 Calligra

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

To: leinir, Calligra-Devel-list
Cc: danders, vandenoever

D8310: Port Calligra Gemini for 3.x

[Dag Andersen]

danders added a comment.


  Dan Leinir Turthra Jensen skrev den 2017-10-30 12:06:
  I hope ppl have just taken an extended summer vacation ;)

REPOSITORY
  R8 Calligra

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

To: leinir, Calligra-Devel-list
Cc: danders, vandenoever

D2577: KoResourcePaths: Fix handling of wildcards in directories and files

[Dag Andersen]

danders added a comment.


  The background was it failed for one case in Plan, and I found a possible 
failure in KoMainWindow (I think it was), but that was a "if all else fails" 
case and I doubt the code will ever be executed.
  Also, I have seen that not all places uses KoResourcePaths so a calligra wide 
cleanup should be done anyway.
  
  So over time, I have come to think that we should abandon KoResourcePaths and 
convert to QStandardPaths instead.

REPOSITORY
  R8 Calligra

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

To: danders, #calligra:_3.0, staniek, Calligra-Devel-list
Cc: anthonyfieroni, staniek, vandenoever

D9253: Make Plan build on Windows

[Dag Andersen]

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


  Surprised it was *that* easy...

REPOSITORY
  R8 Calligra

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

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

D11505: Plan: fix build with Qt 5.6

[Dag Andersen]

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


  Hmm, it should have been i18n'ed also. I'll fix that later, just land it.

REPOSITORY
  R8 Calligra

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

To: wbauer, danders
Cc: Calligra-Devel-list, cochise, vandenoever

D9537: [kotextlayoutarea] Make percentage line height relative to the default height

[Dag Andersen]

danders added a comment.


  Yeah :)
  Seems a unit test needs adjustments, could you have a look?
  
https://build.kde.org/view/Calligra/job/Calligra%20calligra%20kf5-qt5%20FreeBSDQt5.10/lastCompletedBuild/testReport/(root)/TestSuite/libs_kotextlayout_TestBlockLayout/

REPOSITORY
  R8 Calligra

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

To: anthonyfieroni, danders, mecir, boemann
Cc: Calligra-Devel-list, cochise, vandenoever

D14901: Fix blocklayout unittest after percentage lineheight was fixed in D9537

[Dag Andersen]

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

REVISION SUMMARY
  There is a couple of failing tests now marked as XFAIL.
  I am afraid these are new bugs, but I may have missed something (I hope)

REPOSITORY
  R8 Calligra

BRANCH
  blocklayout_unittest_danders

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

AFFECTED FILES
  libs/textlayout/tests/TestBlockLayout.cpp

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, cochise, vandenoever

D14901: Fix blocklayout unittest after percentage lineheight was fixed in D9537

[Dag Andersen]

danders marked an inline comment as done.
danders added a comment.


  What to change depends on what the number means ;)
  Well in general all numbers that depends on fontsize should be changed, the 
rest should stay the same, but yes it is a puzzle.

REPOSITORY
  R8 Calligra

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

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, cochise, vandenoever

D14916: [ArtisticTextShape] Merge text ranges with same style

[Dag Andersen]

danders added a comment.


  The change in appendText(const QString &text):
  The original appended to an existing range while you add a new range. 
  I would think you could just leave the old code untouched, no?

REPOSITORY
  R8 Calligra

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

To: anthonyfieroni, boemann, danders, #calligra:_3.0
Cc: Calligra-Devel-list, cochise, vandenoever

D14916: [ArtisticTextShape] Merge text ranges with same style

[Dag Andersen]

danders added a comment.


  Hmm, I assumed it only got in here when appending a text to
  a) an empty shape (no need for merging),
  b) at the end of an existing text, in which case using the the existing 
formatting of the last range should ok. I just can't see why using a 
defaultfont in this case is correct.
  
  What have I missed?

REPOSITORY
  R8 Calligra

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

To: anthonyfieroni, boemann, danders, #calligra:_3.0
Cc: Calligra-Devel-list, cochise, vandenoever

D14916: [ArtisticTextShape] Merge text ranges with same style

[Dag Andersen]

danders accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R8 Calligra

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

To: anthonyfieroni, boemann, danders, #calligra:_3.0
Cc: Calligra-Devel-list, cochise, vandenoever

D14975: Don't use obsolete QRectF and QAbstractItemModel functions

[Dag Andersen]

danders added a comment.


  I'm not 100% sure of stackChanged() change.
  Probably ok, but the original would call both begin and end before calling 
the selection model.
  To get exactly the same behaviour you could do the same.
  It depends on how the selction model handles reset and setCurrentIndex() I 
guess.
  
  Hmm, I think I would call both begin and end before setCurrentIndex() as 
setCurrentIndex()  usually emits signals that may prompt consumers to access 
the model.

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

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

D14975: Don't use obsolete QRectF and QAbstractItemModel functions

[Dag Andersen]

danders accepted this revision.
This revision is now accepted and ready to land.

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

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

D15056: [EnhancedPathShape] Keep correct aspect ratio of shapes in odf files

[Dag Andersen]

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


  Haven't tested, but sounds sane :)

REPOSITORY
  R8 Calligra

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

To: anthonyfieroni, #calligra:_3.0, danders, boemann
Cc: Calligra-Devel-list, cochise, vandenoever

D15102: Update deprecated QAbstractItemModel::reset() to use {begin, end}ResetModel() instead

[Dag Andersen]

danders added a reviewer: leinir.

REPOSITORY
  R8 Calligra

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

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

D14901: Fix blocklayout unittest after percentage lineheight was fixed in D9537

[Dag Andersen]

danders added a comment.


  Ping?

REPOSITORY
  R8 Calligra

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

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, cochise, vandenoever

D14901: Fix blocklayout unittest after percentage lineheight was fixed in D9537

[Dag Andersen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R8:705d0ae50923: Fix blocklayout unittest after percentage 
lineheight was fixed in D9537 (authored by danders).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D14901?vs=39919&id=40803#toc

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14901?vs=39919&id=40803

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

AFFECTED FILES
  libs/textlayout/tests/TestBlockLayout.cpp

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, cochise, vandenoever

D15403: {Style] Default sheets to white background

[Dag Andersen]

danders added a comment.


  Hmm, imo styling odf apps content area should be considdered a bug :=)
  Because: All info pertaining to presentation of content is stored in odf file 
and is controllable by user, so styling it would  often create strange 
resullts. (And it would not be much wysiwyg about it)
  Of course everything else like tools, dialogs, window decorations can be 
styled.

REPOSITORY
  R8 Calligra

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

To: anthonyfieroni, danders, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D15403: {Style] Default sheets to white background

[Dag Andersen]

danders accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R8 Calligra

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

To: anthonyfieroni, danders, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D15412: [filters] Extend table lifetime

[Dag Andersen]

danders added a comment.


  Sorry, don't have any qualified opinons, I don't quite understand the 
original code either, so...
  Imo if you think it is good, land it.

REPOSITORY
  R8 Calligra

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

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

D15549: Words: Save anchor info also for page-anchored shapes

[Dag Andersen]

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

REVISION SUMMARY
  LO needs anchor info to position shapes correctly.

REPOSITORY
  R8 Calligra

BRANCH
  danders_words_anchor

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

AFFECTED FILES
  words/part/KWOdfWriter.cpp

To: danders, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D15549: Words: Save anchor info also for page-anchored shapes

[Dag Andersen]

danders added a comment.


  After reading the fine print, see: 
http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part1.html#__RefHeading__1420028_253892949
  it seems to me a shape can be anchored to a page in different ways. Note this:
  "If the attribute value is not from-left and not from-inside, the svg:x 
attribute associated with the frame element is ignored for text documents."
  This would explain that LO seems to discard x,y when anchor info is not saved.
  Afaics there is no defined default values for anchor values.

REPOSITORY
  R8 Calligra

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

To: danders, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D15567: Words: Do not popup contextmenu twice

[Dag Andersen]

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

REVISION SUMMARY
  Popup menu was called from mousePressEvent.
  No need as it is handled in contextMenuEvent.

REPOSITORY
  R8 Calligra

BRANCH
  danders_words_contextmenu

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

AFFECTED FILES
  words/part/KWCanvas.cpp

To: danders, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D15549: Words: Save anchor info also for page-anchored shapes

[Dag Andersen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R8:02f34b05723a: Words: Save anchor info also for 
page-anchored shapes (authored by danders).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D15549?vs=41737&id=41811#toc

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15549?vs=41737&id=41811

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

AFFECTED FILES
  words/part/KWOdfWriter.cpp

To: danders, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D15622: Upgrade intersect() to intersected() calls

[Dag Andersen]

danders accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R8 Calligra

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

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

D15620: Remove deprecated use of QDate::setYMD() and QFlag instead of QWindowFlag

[Dag Andersen]

danders accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R8 Calligra

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

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

D15619: Remove use of kglobal.h in sheets

[Dag Andersen]

danders accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R8 Calligra

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

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

D15567: Words: Do not popup contextmenu twice

[Dag Andersen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R8:7bd9b921826e: Words: Do not popup contextmenu twice 
(authored by danders).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15567?vs=41797&id=42052

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

AFFECTED FILES
  words/part/KWCanvas.cpp

To: danders, boemann, leinir
Cc: leinir, Calligra-Devel-list, dcaliste, cochise, vandenoever

D15775: Make the item background color and page cache properties available from View component

[Dag Andersen]

danders added inline comments.

INLINE COMMENTS

> View.h:118
> + */
> +bool pageCache() const;
> +/**

Could you rename pageCache() to pageCacheEnabled() and d:o setPageChache()

REPOSITORY
  R8 Calligra

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

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

D15053: Consistently use qreal in libs/pigment

[Dag Andersen]

danders accepted this revision as: danders.
danders added a comment.
This revision is now accepted and ready to land.


  Fine with me.

REPOSITORY
  R8 Calligra

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

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

D16127: Annotation: Block adding comment to an annotation shape

[Dag Andersen]

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

REVISION SUMMARY
  To reproduce crash:
  
  1. Add a text shape
  2. Add an annotation shape
  3. Activate the annotation shape
  4. Select the Shape Handling tool
  5. Select both shapes; first annotation shape then the text shape
  6. Select the text tool: The caret should now be in the annotation tool
  7. Hit CTRL-SHIFT C to add a comment -> crash
  
  TODO: Find out why disabling insert_annotation action is not working
  
  Closes T4902 

REPOSITORY
  R8 Calligra

BRANCH
  danders_annotation_crash

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

AFFECTED FILES
  plugins/textshape/TextTool.cpp

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16127: Annotation: Block adding comment to an annotation shape

[Dag Andersen]

danders marked an inline comment as done.
danders added a comment.


  No, it should not be possible to activate a tool without a shape.

REPOSITORY
  R8 Calligra

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

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16127: Annotation: Block adding comment to an annotation shape

[Dag Andersen]

danders updated this revision to Diff 43721.
danders added a comment.


  - DefaultTool: Do not considder parent flags when checking Move-/ResizeAllowd
  - DefaultTool: MoveStrategy: Check if shape is movable
  - DefaultTool: ResizeStrategy: Check if shape is resizeable
  - DefaultTool: ShareStrategy: Check if shape is shearable
  - DefaultTool: RotateStrategy: Check if shape rotation is allowed
  - Fix undo/redo of anchored shapes

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16127?vs=43382&id=43721

BRANCH
  shapemove_danders

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

AFFECTED FILES
  libs/flake/commands/KoShapeMoveCommand.cpp
  libs/flake/commands/KoShapeMoveCommand.h
  plugins/defaultTools/defaulttool/DefaultTool.cpp
  plugins/defaultTools/defaulttool/ShapeMoveStrategy.cpp
  plugins/defaultTools/defaulttool/ShapeMoveStrategy.h
  plugins/defaultTools/defaulttool/ShapeResizeStrategy.cpp
  plugins/defaultTools/defaulttool/ShapeRotateStrategy.cpp
  plugins/defaultTools/defaulttool/ShapeShearStrategy.cpp
  plugins/textshape/TextTool.cpp

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16127: Annotation: Block adding comment to an annotation shape

[Dag Andersen]

danders added a comment.


  Arghh, accidentally propt a new diff on top of this, howto undo this?
  (arc is fine, but do not do anything wrong)

REPOSITORY
  R8 Calligra

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

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16127: Annotation: Block adding comment to an annotation shape

[Dag Andersen]

danders abandoned this revision.

REPOSITORY
  R8 Calligra

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

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16127: Annotation: Block adding comment to an annotation shape

[Dag Andersen]

danders updated this revision to Diff 43724.
danders added a comment.


  - If no textshape is editied, select the first found text shape

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16127?vs=43721&id=43724

BRANCH
  danders_annotation_crash

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

AFFECTED FILES
  plugins/textshape/TextTool.cpp

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16127: Annotation: Block adding comment to an annotation shape

[Dag Andersen]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R8:d4744090e8c6: Annotation: Block adding comment to an 
annotation shape (authored by danders).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16127?vs=43724&id=43728

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

AFFECTED FILES
  plugins/textshape/TextTool.cpp

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16289: Karbon: Add Anthony as maintainer and remove unmaintained

[Dag Andersen]

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

REPOSITORY
  R8 Calligra

BRANCH
  karbon_danders

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

AFFECTED FILES
  karbon/ui/KarbonAboutData.h

To: danders, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16289: Karbon: Add Anthony as maintainer and remove unmaintained

[Dag Andersen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R8:99527b565f59: Karbon: Add Anthony as maintainer and remove 
unmaintained (authored by danders).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16289?vs=43840&id=43844

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

AFFECTED FILES
  karbon/ui/KarbonAboutData.h

To: danders, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16367: EnhancedPathShape: Shape is moved/resized on save/load

[Dag Andersen]

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

REVISION SUMMARY
  Remove all code that uses viewbox to affect position and size.
  
  Oasis spec:
  The svg:viewBox attribute establishes a user coordinate system inside the 
coordinate system
  of the shape specified by the position and size attributes.
  
  I interpret this to mean that viewbox shall *never* affect the shapes 
position or size.

TEST PLAN
  To reproduce: Add a database stencil in flow and save/load.

REPOSITORY
  R8 Calligra

BRANCH
  enhancedpath_danders

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

AFFECTED FILES
  plugins/pathshapes/enhancedpath/EnhancedPathShape.cpp
  plugins/pathshapes/enhancedpath/EnhancedPathShape.h

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16367: EnhancedPathShape: Shape is moved/resized on save/load

[Dag Andersen]

danders added a comment.


  I have tested with LO:
  Create with flow,
  Open in LO,
  Save in LO,
  Open in flow.
  And it seems to work, so there must be some special case?
  The only workaround I've seen is for ellipses.
  No chance for a testdoc I guess...

REPOSITORY
  R8 Calligra

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

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16367: EnhancedPathShape: Shape is moved/resized on save/load

[Dag Andersen]

danders added a comment.


  Yes, found it: https://bugs.kde.org/show_bug.cgi?id=289646
  Afaics this affects only callout shapes where OO/LO breaks the intention of 
the odf spec
  by using the shape size and position as text area.
  This is ok (sort of) when text area and shape area overlap but not when part 
of the path goes outside
  the shape area as with callouts.
  The current code fixes this but we also get false positives as demonstrated.
  I'll try to find a more specific way to handle this.
  I doubt LO will fix this, ever, so we have to live with it, but I *hate* 
working around bugs in other apps.

REPOSITORY
  R8 Calligra

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

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16367: EnhancedPathShape: Shape is moved/resized on save/load

[Dag Andersen]

danders abandoned this revision.
danders added a comment.


  Ok, after some more investigation, I've found a number of problems around 
these things.
  This was really just a bug in the stencil database.odg file.
  The callout stuff is the big one, because it abuses odf so needs special 
treatment.
  I'll just call this patch a false start and abondone it.

REPOSITORY
  R8 Calligra

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

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16679: Words: Fix interop problem with LO/OO

[Dag Andersen]

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

REVISION SUMMARY
  Words creates pagestyles with name "Standard" and displayname "Default".
  Afaics the displayname is not used anywhere, so remove it.
  
  There is a bug in LO/OO that creates a "Default" style where all words
  style attributes go. The style is never used by LO/OO, effectivly
  discarding all attributes.

REPOSITORY
  R8 Calligra

BRANCH
  words_pagestyle_danders

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

AFFECTED FILES
  words/part/KWPageManager.cpp

To: danders, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16679: Words: Fix interop problem with LO/OO

[Dag Andersen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R8:caffa566d8d2: Words: Fix interop problem with LO/OO 
(authored by danders).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16679?vs=44892&id=44896

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

AFFECTED FILES
  words/part/KWPageManager.cpp

To: danders, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16756: CalloutShape: Add a new shape to handle callouts

[Dag Andersen]

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

REVISION SUMMARY
  This is the start of a dedicated callout shape.
  It uses draw:type to identify callouts, same as LO/OO.
  Unfortunatwly it abuses odf, but if we shall be compatible
  with LO/OO I see no other option.
  
  Roundtrips via LO works both ways and LO's ui also works with
  callout produced with stage.
  
  TODO:
  
  - Shearing and rotation
  - Needs new tool for resize/move of 'bubble' part
  
  Known problem:
  Rounding (?) problem when calculating position/size offsets,
  but this will be changed anyway when shearing/rotation is implemented.

REPOSITORY
  R8 Calligra

BRANCH
  callout_danders

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

AFFECTED FILES
  plugins/pathshapes/CMakeLists.txt
  plugins/pathshapes/PathShapesPlugin.cpp
  plugins/pathshapes/enhancedpath/CalloutShape.cpp
  plugins/pathshapes/enhancedpath/CalloutShape.h
  plugins/pathshapes/enhancedpath/CalloutShapeFactory.cpp
  plugins/pathshapes/enhancedpath/CalloutShapeFactory.h
  plugins/pathshapes/enhancedpath/EnhancedPathShape.cpp
  plugins/pathshapes/enhancedpath/EnhancedPathShape.h
  plugins/pathshapes/enhancedpath/EnhancedPathShapeFactory.cpp
  plugins/pathshapes/enhancedpath/EnhancedPathShapeFactory.h

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D16756: CalloutShape: Add a new shape to handle callouts

[Dag Andersen]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R8:2e1aaae999f3: CalloutShape: Add a new shape to handle 
callouts (authored by danders).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16756?vs=45112&id=45447

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

AFFECTED FILES
  plugins/pathshapes/CMakeLists.txt
  plugins/pathshapes/PathShapesPlugin.cpp
  plugins/pathshapes/enhancedpath/CalloutShape.cpp
  plugins/pathshapes/enhancedpath/CalloutShape.h
  plugins/pathshapes/enhancedpath/CalloutShapeFactory.cpp
  plugins/pathshapes/enhancedpath/CalloutShapeFactory.h
  plugins/pathshapes/enhancedpath/EnhancedPathShape.cpp
  plugins/pathshapes/enhancedpath/EnhancedPathShape.h
  plugins/pathshapes/enhancedpath/EnhancedPathShapeFactory.cpp
  plugins/pathshapes/enhancedpath/EnhancedPathShapeFactory.h

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D15428: [textlayout] Don't enter infinite loop when table is misfit

[Dag Andersen]

danders added a comment.


  Would it be possible to make a unit test?

REPOSITORY
  R8 Calligra

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

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

D15428: [textlayout] Don't enter infinite loop when table is misfit

[Dag Andersen]

danders added a comment.


  Had a closer look at this. Afaics we get an infinit loop when a table is 
'totally misfit',
  Can't say I understand the table layout logic, but my assumption is that if a 
row is a total misfit it can't just be ignored,
  so breaking off the row layout loop in this case seems to work:
  
  F6473096: x.diff 

REPOSITORY
  R8 Calligra

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

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

D15428: [textlayout] Don't enter infinite loop when table is misfit

[Dag Andersen]

danders added a comment.


  Ok, think I'm on to something.
  Testing with the 1.doc in bug 381341. It seems it fails on the table in 
approx page 222 (open in LO)
  with text in 0,0: Экономический субъект
  Stepping through the KoTextLayoutTableArea::layoutRow(), it seems *all* 
columns in row 0 (at least) spans rows and hence (I think) layouting does not 
work.
  
  I probably don't have more time to spend on this before next week, so please 
have a look :)

REPOSITORY
  R8 Calligra

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

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

D15428: [textlayout] Don't enter infinite loop when table is misfit

[Dag Andersen]

danders added a comment.


  I ended up in the same spot as you:
  Since all columns in row 0 spans rows, totalMisFit will always be set to true 
and the whole table is layed out on next page, and next page again and again ...
  
  I'm not 100% certain just adding noCellsFitted to the condition covers all 
bases:
  We try to layout all headerrows plus 1, but afaics we have to considder 
spanned rows when first nonheaderrow spans rows.
  See attached path for a suggested solution.
  (Maybe also if last headerrow spans rows, but well, probably not a common 
case ;)
  
  Also, if this minimum does not fit on a virgin page, I think we should just 
lay it out and hope for the best.
  I added some code for this in the attached patch, but I'm not sure it is the 
right way, or complete.
  F6482937: y.diff 

REPOSITORY
  R8 Calligra

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

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

D15428: [textlayout] Don't enter infinite loop when table is misfit

[Dag Andersen]

danders added a comment.


  In D15428#378420 , @anthonyfieroni 
wrote:
  
  > Yep, it's look like same approach to mine, did you try mine or it's not 
correct in all cases?
  
  
  Mine tries to take into account the case where the first row after header is 
spanning rows, I don't think yours do?

REPOSITORY
  R8 Calligra

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

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

D15428: [textlayout] Don't enter infinite loop when table is misfit

[Dag Andersen]

danders added a comment.


  In D15428#378422 , @boemann wrote:
  
  > Dan I like your diff better - I don't think it's completely there but it's 
a better starting point
  >
  > On a more conceptual level, what should happen if the design of table is 
such that headers can't fit on a virgin page? What should we do.? One one hand 
it should be there but on the other hand we will never get to show the real 
part of the table ever.
  >  What is the solution we should aim for?
  
  
  I thought I had covered that case by not setting totalMisFit  (but I'm not 
certain). I thought that rows then would be layed out regrdless, but I may have 
missed a trick.

REPOSITORY
  R8 Calligra

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

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

D15428: [textlayout] Don't enter infinite loop when table is misfit

[Dag Andersen]

danders added a comment.


  In D15428#378432 , @boemann wrote:
  
  > No you misunderstand. I wasn't talking about you diff - I want to know what 
it is we are trying to accomplish. in spoken words
  
  
  Ahhh. It's a guard against loop/crash in extreme cases.
  
  In real life this should (almost) never happen, maybe when changing page size 
or if we do not have the correct font.
  I think the only thing to do is to just layout the header rows over multiple 
pages (as we do with non-header rows).

REPOSITORY
  R8 Calligra

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

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

D15428: [textlayout] Don't enter infinite loop when table is misfit

[Dag Andersen]

danders added a comment.


  Fidled a little more with this, and found several problems when table with 
merged cells is split over pages,
  like unmerged cell painted on both pages (empty on first page), caret not 
shown in selected cell and sometimes shown in prev cell.
  
  Tried to recreate problem with words, which is easy, both with Anthonies 
(D15428 ) patch and with master:
  
  - Create a 2x1 table then merge the two rows:
  
  master: Table disappears (expected, as layout code is skipped)
  D15428 : Table is displayed as it should 
be.
  
  - Create a table larger than a page, then merge the first 2 rows:
  
  master: Loops
  D15428 : Table is displayed as it should.
  
  I encountered an assert in KoTextLayoutTableArea::selectionBoundingBox() line 
342,
  but this is not a new problem and unrelated to D15428 
.
  To recreate: Add a table, merge two rows, add a row below the merged rows -> 
assert.
  
  I also checked out how LO handles this. In imvvho they don't do a great job, 
but they don't freeze nor crash ;)
  As you aptly point out Camilla, we need to define in more detail what to do 
in different cases, but as always
  LO is probably the standard even when they are not good.
  
  Based on all this, I retract all my suggestions/proposals/patches.
  Imvvho Anthonies patch adresses two concrete problems (a loop and a crash) 
and afaics it does not introduce new problems,
  so I propose it is accepted.

REPOSITORY
  R8 Calligra

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

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

D18095: Add new table layout unit tests

[Dag Andersen]

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

REVISION SUMMARY
  Adds multiple areas capabilities and tests with merged cells

REPOSITORY
  R8 Calligra

BRANCH
  table_unittest_danders

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

AFFECTED FILES
  libs/textlayout/tests/MockRootAreaProvider.cpp
  libs/textlayout/tests/MockRootAreaProvider.h
  libs/textlayout/tests/TestDocumentLayout.cpp
  libs/textlayout/tests/TestTableLayout.cpp
  libs/textlayout/tests/TestTableLayout.h

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D18095: Add new table layout unit tests

[Dag Andersen]

danders added a comment.


  Not trivial (for me) to get data out of layout so it is hard to create robust 
tests.
  I don't think I have been able to create a test to simulate the loop excactly.
  Also, uncertain wether the MockRootAreaProvider is sufficient.
  Any ideas/comments wellcome.

REPOSITORY
  R8 Calligra

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

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D18095: Add new table layout unit tests

[Dag Andersen]

danders added a comment.


  No, all do not pass, generally when the first row is merged, layout fails.
  (I am trying to provoke problems similar to bug 381341 that we have tried to 
fix in D15428 .)

REPOSITORY
  R8 Calligra

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

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D18095: Add new table layout unit tests

[Dag Andersen]

danders updated this revision to Diff 49141.
danders added a comment.


  Add a test that trigger a loop in the table layout

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18095?vs=48957&id=49141

BRANCH
  table_unittest_danders

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

AFFECTED FILES
  libs/textlayout/tests/MockRootAreaProvider.cpp
  libs/textlayout/tests/MockRootAreaProvider.h
  libs/textlayout/tests/TestDocumentLayout.cpp
  libs/textlayout/tests/TestTableLayout.cpp
  libs/textlayout/tests/TestTableLayout.h

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D18095: Add new table layout unit tests

[Dag Andersen]

danders added a comment.


  Even if they do not all pass, I think they should be commited if the 
tests/methods are ok and the reason for not passing are genuin bugs, no?

REPOSITORY
  R8 Calligra

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

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D18095: Add new table layout unit tests

[Dag Andersen]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R8:666ae5b484cc: Add new table layout unit tests (authored by 
danders).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18095?vs=49141&id=49511

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

AFFECTED FILES
  libs/textlayout/tests/MockRootAreaProvider.cpp
  libs/textlayout/tests/MockRootAreaProvider.h
  libs/textlayout/tests/TestDocumentLayout.cpp
  libs/textlayout/tests/TestTableLayout.cpp
  libs/textlayout/tests/TestTableLayout.h

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D15428: [textlayout] Don't enter infinite loop when table is misfit

[Dag Andersen]

danders added a comment.


  Ran this patch against new unit tests and it fixes all the expected ones, 
namely those where all cells in first row is merged with cells in second row.
  Test results without patch: 
https://build.kde.org/job/Calligra/job/calligra/job/kf5-qt5%20SUSEQt5.10/lastCompletedBuild/testReport/projectroot.libs.textlayout/tests/libs_kotextlayout_TestTableLayout
  See uploaded file for result with patch.F6552719: LastTest.log 


REPOSITORY
  R8 Calligra

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

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

D18448: Replaced "distraction free" with "fullscreen" - BUG:378527

[Dag Andersen]

danders added a reviewer: boemann.

REPOSITORY
  R8 Calligra

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

To: niccolove, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D18466: Fixed calligra crashing when opening remote document

[Dag Andersen]

danders added a comment.


  I think the problem is that slotFilePrint() is called (in 
KoApplication::start()) before the document is actually loaded.
  Probably we need to start printing in a slot connected to 
KoMainWindow::loadCompleted() instead.
  And to be safe, we also need to handle loadCanceled().
  And the same for exportToPdf, I think.

REPOSITORY
  R8 Calligra

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

To: niccolove, danders
Cc: boemann, Calligra-Devel-list, dcaliste, cochise, vandenoever

D18466: Fixed calligra crashing when opening remote document

[Dag Andersen]

danders added a comment.


  In D18466#400507 , @niccolove 
wrote:
  
  > I'm not sure on how to do that, do you have any tip?
  >  openDocumentInternal is called by openDocument, which is called in 
KoApplication just before checking the arguments. When it sees the print 
argument, it calls slotFilePrint. openDocumentInternal downloads the document 
in async. Should it be made not async, or is there another way to make it wait 
until the other process ends? Also, "slotLoadCanceled" is already implemented, 
or were you talking about something else?
  
  
  Well niccolove, you have really put your hand into a bee-hive :)
  The answer from anthony can be used as a test to see if the the princip 
works, but does not address all situations.
  
  The current implementation needs quite a lot of change to work. These are the 
conditions as I see them:
  
  1. openDocument() initiates loading and returns true if it finds the file, 
just return false if the file do not exist.
  2. The actual loading is asynchronous if the file is remote.
  3. It is possible to specify multiple files to load. (Each file will be shown 
in it's own main window.)
  4. For writing to pdf, a file name can be specified on the command line.
  
  These are the steps I think is needed:
  
  1. Create slot(s) in KoApplication to handle the different  resullts; print, 
pdf and cancelled.
  2. The signal connections must be done before openDocument() is called. (So 
that local files also works.)
  3. Add KoDocument as an arg to the KoMainWindow::loadCompleted() signal, so 
KoApllication knows which dcoument has been loaded.
  4. For pdf, find a way to handle filename.

REPOSITORY
  R8 Calligra

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

To: niccolove, danders
Cc: anthonyfieroni, boemann, Calligra-Devel-list, dcaliste, cochise, vandenoever

D15428: [textlayout] Don't enter infinite loop when table is misfit

[Dag Andersen]

danders added a comment.


  Well, unit tests has been added that fails without this patch and passes with 
this patch.
  If there are more test cases needed, please add them so that we can get this 
patch committed soon...

REPOSITORY
  R8 Calligra

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

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

D18466: Fixed calligra crashing when opening remote document

[Dag Andersen]

danders added a comment.


  Regarding the completed slot, the doc says:
  --print  Only print and exit
  --export-pdf  Only export to PDF and exit
  
  To exit, you need to know when all jobs have finished, so if some failed you 
need to track that.
  Also, the connection to job in slotExportToPdf() is wrong (not your fault) as 
it will just quit while there may be other jobs running.

REPOSITORY
  R8 Calligra

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

To: niccolove, danders
Cc: anthonyfieroni, boemann, Calligra-Devel-list, dcaliste, cochise, vandenoever

D18466: Fixed calligra crashing when opening remote document

[Dag Andersen]

danders added a comment.


  Yeah, well there are some half done designs in here :(
  
  The benchmark just does not work with remote files. Don't know if it should 
or if it should be blocked. Let's leave it for now.
  
  If you do not specify a pdf filename, it will use the same name as in the 
loaded file just changing the extension to .pdf, so it is possible to export 
multiple files.
  I have sort of assumed that print and exportToPdf commands was meant to be 
hands free, to be able to print a number of files wo a lot of mouse work.
  Popping up dialogs (print dialog or error dialogs) defies that goal, so imho 
these things should be handled differently.
  I recognize that this fix has balooned and we are getting into difficult 
areas, so I propose:
  Let's do without exit atm, just remove the job connection in 
slotExportToPdf() and add a TODO to implement exit later.

REPOSITORY
  R8 Calligra

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

To: niccolove, danders
Cc: anthonyfieroni, boemann, Calligra-Devel-list, dcaliste, cochise, vandenoever

D18466: Fixed calligra crashing when opening remote document

[Dag Andersen]

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


  Yes, close.
  When export to pdf, the mainwindow is not shown and since we don't exit, the 
app will hang.
  You need to invert the check in KoApplication line 460 so the window is shown.
  
  Also please fix the trailing spaces.
  
  And then: ok to land!

REPOSITORY
  R8 Calligra

BRANCH
  print-remote-files (branched from master)

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

To: niccolove, danders
Cc: anthonyfieroni, boemann, Calligra-Devel-list, dcaliste, cochise, vandenoever

D18466: Fixed calligra crashing when opening remote document

[Dag Andersen]

danders added a comment.


  Ok, it's accepted so please commit/push/land

REPOSITORY
  R8 Calligra

BRANCH
  print-remote-files (branched from master)

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

To: niccolove, danders
Cc: anthonyfieroni, boemann, Calligra-Devel-list, dcaliste, cochise, vandenoever

D18841: StencilBoxDocker: Make the docker collapsed by default

[Dag Andersen]

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

REVISION SUMMARY
  WIP

REPOSITORY
  R8 Calligra

BRANCH
  activate_stencils_danders

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

AFFECTED FILES
  plugins/stencilsdocker/StencilBoxDockerFactory.h
  stage/part/KPrFactory.cpp
  words/part/KWFactory.cpp

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

D18841: StencilBoxDocker: Make the docker collapsed by default

[Dag Andersen]

danders abandoned this revision.
danders added a comment.


  mistake

REPOSITORY
  R8 Calligra

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

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

D18843: Activate stencils docker in all apps

[Dag Andersen]

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

REVISION SUMMARY
  The docker now loads in separate thread and is hidden by default,
  so does not have any performance impact and is non-intrusive.
  
  From a user perspective (me):
  Why words: My weapon of choice to create a flow chart is words,
  because of things like headers/footers and even TOC.
  
  Why sheets: Just for consistency.

TEST PLAN
  Open words, immediately start typing: words is responsive.

REPOSITORY
  R8 Calligra

BRANCH
  activate_stencils_danders

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

AFFECTED FILES
  stage/part/KPrFactory.cpp
  words/part/KWFactory.cpp

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D18866: Use check_symbol_exists instead of check_function_exists

[Dag Andersen]

danders added a comment.


  What about the other ​check_function_exists, should they also be changed?
  Also, do you know when ​check_symbol_exists was introduced?
  We still use cmake 2.8.12.

REPOSITORY
  R8 Calligra

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

To: heikobecker
Cc: danders, Calligra-Devel-list, dcaliste, cochise, vandenoever

D18866: Use check_symbol_exists instead of check_function_exists

[Dag Andersen]

danders accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R8 Calligra

BRANCH
  master

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

To: heikobecker, danders
Cc: pino, danders, Calligra-Devel-list, dcaliste, cochise, vandenoever

D18466: Fixed calligra crashing when opening remote document

[Dag Andersen]

danders added a comment.


  In D18466#411248 , @boemann wrote:
  
  > Dan can you please reply - I'm fine with the account but I can't reply to 
Ben as I am not able to send email
  
  
  Yes, I have, and just answered again in case something went wrong first time.
  Let's see if something happens soon.

REPOSITORY
  R8 Calligra

BRANCH
  print-remote-files (branched from master)

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

To: niccolove, danders
Cc: anthonyfieroni, boemann, Calligra-Devel-list, dcaliste, cochise, vandenoever

D18963: Improve KoModeBox display in horizontal Mode

[Dag Andersen]

danders added a comment.


  Please remove trailing whitespace.

REPOSITORY
  R8 Calligra

BRANCH
  calligra/3.1

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

To: ognarb, #calligra:_3.0, #vdg, boemann
Cc: danders, boemann, anthonyfieroni, abetts, ngraham, Calligra-Devel-list, 
dcaliste, cochise, vandenoever

D18843: Activate stencils docker in all apps

[Dag Andersen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R8:d11e63457aab: Activate stencils docker in all apps 
(authored by danders).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18843?vs=51158&id=51575

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

AFFECTED FILES
  stage/part/KPrFactory.cpp
  words/part/KWFactory.cpp

To: danders, boemann, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D18466: Fixed calligra crashing when opening remote document

[Dag Andersen]

danders added a comment.


  Thanks, I'm checking it out now...
  Just a couple of things:
  Keywords needs to be on a separate line to have effect, see 
https://community.kde.org/Infrastructure/Git/Hooks
  Also FIXED-IN is nice to add.
  
  I've updated and closed the bug, so no problem.

REPOSITORY
  R8 Calligra

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

To: niccolove, danders
Cc: anthonyfieroni, boemann, Calligra-Devel-list, dcaliste, cochise, vandenoever

D19132: Update libs/widgets to c++11

[Dag Andersen]

danders added a reviewer: leinir.
danders added a comment.


  Hmmm. wasn't it somethng with ms windows and for loops, leinir?
  Also, I think there may be detachment issues with them, clazy can tell.
  There is a qAsConst() solution but that is not supported in the qt version we 
need to support.
  If there are issues, maybe drop the for loops for now.

REPOSITORY
  R8 Calligra

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

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

D19132: Update libs/widgets to c++11

[Dag Andersen]

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


  Ok, I can't say I have scrutinized every change, but...
  Could you give a heads up when you continue with other parts or else there 
may be merge problems.
  I'm working on karbon and pageapp atm, so please keep off :)

REPOSITORY
  R8 Calligra

BRANCH
  master

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

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

D19132: Update libs/widgets to c++11

[Dag Andersen]

danders added a comment.


  > Thanks for the review, I will work next on 
libs/{pigment,version,widgetutils}.
  
  Hmm, maybe wait with pigment, according to rempt, only krita actually used it.
  I haven't looked into it, but if that is the case probably we should dump it.

REPOSITORY
  R8 Calligra

BRANCH
  master

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

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

D19216: Karbon: Enable multi page capability

[Dag Andersen]

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

REVISION SUMMARY
  Since odg spec supports multiple pages, I feel karbon also needs to support 
it.
  
  Ported to use pageapp classes.
  
  Karbon has config group "Interface" that is not precent in other apps.
  Some of the options have been disabled atm. Imho they should be 
  harmonized with other apps and/or moved to View menu.
  
  A lot of code was duplicated between pageapp and karbon
  and has been removed from karbon:
  
  - Save/load
  - Layers docker and all layer operations
  - Grid, guides, rulers and zoom
  - Event handlers
  - Printing
  - Show page margins has been moved to pageapp
  
  In general, import/export needs review to determine how to
  handle multiple pages when e.g. exporting to a format that
  does not support pages.
  
  Known bugs:
  
  - "Separate paths" command:
- Execute command, the shape disappears.
- Undo crashes. Note: Afaics this code is not touched so probably a libs 
bug.
  
  - Snap to grid does not work

TEST PLAN
  I am not an avid user of karbon, so would be nice if some that where could 
test.
  Also, do not have all types of different format docs for filter testing.
  Some that work are svg, jpg and karbon files.

REPOSITORY
  R8 Calligra

BRANCH
  karbon_multipage_danders

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

AFFECTED FILES
  filters/karbon/CMakeLists.txt
  filters/karbon/image/CMakeLists.txt
  filters/karbon/image/ImageExport.cpp
  filters/karbon/svg/CMakeLists.txt
  filters/karbon/svg/SvgExport.cpp
  filters/karbon/svg/SvgImport.cpp
  filters/karbon/wmf/CMakeLists.txt
  filters/karbon/wmf/WmfExport.cpp
  karbon/CMakeLists.txt
  karbon/data/karbon.rc
  karbon/ui/CMakeLists.txt
  karbon/ui/KarbonDocument.cpp
  karbon/ui/KarbonDocument.h
  karbon/ui/KarbonDocumentMergeCommand.cpp
  karbon/ui/KarbonDocumentMergeCommand.h
  karbon/ui/KarbonPart.cpp
  karbon/ui/KarbonPart.h
  karbon/ui/KarbonPrintJob.cpp
  karbon/ui/KarbonPrintJob.h
  karbon/ui/KarbonView.cpp
  karbon/ui/KarbonView.h
  karbon/ui/ProxyView.cpp
  karbon/ui/ProxyView.h
  karbon/ui/dockers/KarbonLayerDocker.cpp
  karbon/ui/dockers/KarbonLayerDocker.h
  karbon/ui/dockers/KarbonLayerModel.cpp
  karbon/ui/dockers/KarbonLayerModel.h
  karbon/ui/dockers/KarbonLayerSortingModel.cpp
  karbon/ui/dockers/KarbonLayerSortingModel.h
  karbon/ui/widgets/KarbonCanvas.cpp
  karbon/ui/widgets/KarbonCanvas.h
  karbon/ui/widgets/KarbonConfigInterfacePage.cpp
  karbon/ui/widgets/KarbonSmallStylePreview.h
  libs/pageapp/KoPAView.cpp
  libs/pageapp/KoPAView.h

To: danders, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D19216: Karbon: Enable multi page capability

[Dag Andersen]

danders added a comment.


  In D19216#417288 , @rjvbb wrote:
  
  > This would indeed be great to have; even a page selector when importing a 
multi-page document would be an improvement (the Adobe Illustrator version I've 
use had that; IIRC it would just leave all other pages of the document alone).
  >
  > You should also test with PDF documents; in my experience Karbon 3.1 works 
well enough with them.
  
  
  Yes, pdf docs work, it is almost 100% handled by poppler.
  
  > ...
  > 
  >> A lot of code was duplicated between pageapp and karbon
  >>  and has been removed from karbon:
  > 
  > Shouldn't that be a separate change - or does multi-page support come 
automatically with this change?
  
  Well, my initial thought, but there was going to be quite a few intermidiate 
solutions mainly because
  the KoPACanvas cannot be subclassed, so I decided to do everything in one go.

REPOSITORY
  R8 Calligra

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

To: danders, anthonyfieroni
Cc: rjvbb, Calligra-Devel-list, dcaliste, cochise, vandenoever

D19216: Karbon: Enable multi page capability

[Dag Andersen]

danders added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in KarbonConfigInterfacePage.cpp:73-90
> I want to discuss comments in the review, i don't want to have in. why we 
> remove this?

Recent file & docker font:
Why does karbon need these, none of the other apps have it.
Personally I would remove it, alternativly implent for all apps.
Canvas color:
I don't quite see what it is for. You can set a background color for the canvas 
but it is only for the views, it is not printed.
Also, if you have multiple views, it sets it in all views.

REPOSITORY
  R8 Calligra

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

To: danders, anthonyfieroni
Cc: rjvbb, Calligra-Devel-list, dcaliste, cochise, vandenoever

D19216: Karbon: Enable multi page capability

[Dag Andersen]

danders added a comment.


  In D19216#417502 , @rjvbb wrote:
  
  > > Canvas color:
  > > I don't quite see what it is for. You can set a background color for the 
canvas but it is only for the views, it is not printed.
  >
  > A custom canvas colour feature doesn't strike me as odd, nor that it isn't 
printed (printing it WITHOUT setting a dedicated option would seem wrong to me).
  >
  > What about when you use an inversed theme, isn't a custom canvas colour 
required then if you want to see your black line art on a light (white) canvas?
  
  
  No, the canvas is part of the document and must never be themed. The canvas 
background is as much part of your drawing as any line you put on it.

REPOSITORY
  R8 Calligra

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

To: danders, anthonyfieroni
Cc: rjvbb, Calligra-Devel-list, dcaliste, cochise, vandenoever

D19216: Karbon: Enable multi page capability

[Dag Andersen]

danders added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in KarbonConfigInterfacePage.cpp:73-90
> We can remove "number of recent files" since other apps does not have it. but 
> for other 2 i don't see why we remove them, someone can found useful 
> (somehow). About me, i want them back, recent file complete remove not 
> commented.

I reconsiddered this, sheets actually also implements this so I'll leave it in.
(It does not work properly neither for karbon nor sheets, but that for 
different rainy day)

> danders wrote in KarbonConfigInterfacePage.cpp:73-90
> Recent file & docker font:
> Why does karbon need these, none of the other apps have it.
> Personally I would remove it, alternativly implent for all apps.
> Canvas color:
> I don't quite see what it is for. You can set a background color for the 
> canvas but it is only for the views, it is not printed.
> Also, if you have multiple views, it sets it in all views.

I also reconsidder docker font setting. It is useful and (partially) 
implemented in libs. You have to restart to get the new setting.
This should be impleneted for all apps, but that's for a different patch.

But, when it comes to canvas background, the more a look into it the more I 
dislike it.
Afaics it goes against both odf spec, wysiwyg and pageapp implementation.
Unfortunatly it was implemented so long ago that there is not mail archive any 
longer and
the git log doesn't give any reason.
Unless I get a *very* good reason to implement this I will not include it.

REPOSITORY
  R8 Calligra

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

To: danders, anthonyfieroni
Cc: rjvbb, Calligra-Devel-list, dcaliste, cochise, vandenoever

D19216: Karbon: Enable multi page capability

[Dag Andersen]

danders marked an inline comment as done.

REPOSITORY
  R8 Calligra

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

To: danders, anthonyfieroni
Cc: boemann, rjvbb, Calligra-Devel-list, dcaliste, cochise, vandenoever

D15428: [textlayout] Don't enter infinite loop when table is misfit

[Dag Andersen]

danders added a comment.


  Can we get a conclussion to this?
  @Camilla Have you come up with any more unit tests?

REPOSITORY
  R8 Calligra

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

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

D19327: Karbon: Enable multi page capability

[Dag Andersen]

danders created this revision.
danders added a reviewer: anthonyfieroni.
danders added a project: Calligra: 3.0.
danders requested review of this revision.

REVISION SUMMARY
  Since odg spec supports multiple pages, I feel karbon also needs to support 
it.
  
  Ported to use pageapp classes.
  
  Karbon has config group "Interface" that is not precent in other apps.
  Some of the options have been disabled atm. Imho they should be
  harmonized with other apps and/or moved to View menu.
  
  A lot of code was duplicated between pageapp and karbon
  and has been removed from karbon:
  
  - Save/load
  - Layers docker and all layer operations
  - Grid, guides, rulers and zoom
  - Event handlers
  - Printing
  - Show page margins has been moved to pageapp
  
  In general, import/export needs review to determine how to
  handle multiple pages when e.g. exporting to a format that
  does not support pages.
  
  Known bugs:
  
  - "Separate paths" command:
- Execute command, the shape disappears.
- Undo crashes. Note: Afaics this code is not touched so probably a libs 
bug.
  
  - Snap to grid does not work
  
  - Number of recent files does not work properly

TEST PLAN
  I am not an avid user of karbon, so would be nice if some that where could 
test.
  Also, do not have all types of different format docs for filter testing.
  Some that work are pdf, svg, jpg and karbon files.

REPOSITORY
  R8 Calligra

BRANCH
  karbon_multipage_danders

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

AFFECTED FILES
  filters/karbon/CMakeLists.txt
  filters/karbon/image/CMakeLists.txt
  filters/karbon/image/ImageExport.cpp
  filters/karbon/svg/CMakeLists.txt
  filters/karbon/svg/SvgExport.cpp
  filters/karbon/svg/SvgImport.cpp
  filters/karbon/wmf/CMakeLists.txt
  filters/karbon/wmf/WmfExport.cpp
  karbon/CMakeLists.txt
  karbon/data/karbon.rc
  karbon/ui/CMakeLists.txt
  karbon/ui/KarbonDocument.cpp
  karbon/ui/KarbonDocument.h
  karbon/ui/KarbonDocumentMergeCommand.cpp
  karbon/ui/KarbonDocumentMergeCommand.h
  karbon/ui/KarbonPart.cpp
  karbon/ui/KarbonPart.h
  karbon/ui/KarbonPrintJob.cpp
  karbon/ui/KarbonPrintJob.h
  karbon/ui/KarbonView.cpp
  karbon/ui/KarbonView.h
  karbon/ui/ProxyView.cpp
  karbon/ui/ProxyView.h
  karbon/ui/dockers/KarbonLayerDocker.cpp
  karbon/ui/dockers/KarbonLayerDocker.h
  karbon/ui/dockers/KarbonLayerModel.cpp
  karbon/ui/dockers/KarbonLayerModel.h
  karbon/ui/dockers/KarbonLayerSortingModel.cpp
  karbon/ui/dockers/KarbonLayerSortingModel.h
  karbon/ui/widgets/KarbonCanvas.cpp
  karbon/ui/widgets/KarbonCanvas.h
  karbon/ui/widgets/KarbonConfigInterfacePage.cpp
  karbon/ui/widgets/KarbonSmallStylePreview.h
  libs/pageapp/KoPAView.cpp
  libs/pageapp/KoPAView.h

To: danders, anthonyfieroni
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever

D19216: Karbon: Enable multi page capability

[Dag Andersen]

danders added a comment.


  In D19216#421689 , @rjvbb wrote:
  
  > >   That is actual a valid point, although in say Krita with transparent 
pixels a checkerboard is shown.
  >
  > Heh, I've had the opportunity to work closely with professional 
infographists so I picked up a thing or two about using applications like 
Illustrator and InDesign :)
  >
  > How to indicate a transparent area of an object is yet another subject, 
with a related but not identical purpose.
  >
  > >   That said it might make more sense for this "testing background" to be 
part of the document and not a general setting - if the use case is as you say 
then the testing background would also be different for different projects
  >
  > True. I was reacting to *re*moving a setting completely, not to moving it 
to a more appropriate context.
  
  
  Ok, after another rethink...
  It starts to make sence if karbon is used not as an odg editor but as an svg 
editor.
  Let's say you create an icon that should be shown on certain background 
color(s), it would be a help if you could check your design aginst those colors.
  In his case you are not concerned with wysiwyg or printing  as you would be 
with an odg document.

REPOSITORY
  R8 Calligra

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

To: danders, anthonyfieroni
Cc: boemann, rjvbb, Calligra-Devel-list, dcaliste, cochise, vandenoever

D19761: Remove obsolete syncX() workaround

[Dag Andersen]

danders accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R8 Calligra

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

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

D19760: Support Qt <5.7 on KoShape flag usage

[Dag Andersen]

danders accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R8 Calligra

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

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

D19759: Remove Qt::WA_InputMethodEnabled attribute from QGraphicsWidgets

[Dag Andersen]

danders added a reviewer: boemann.

REPOSITORY
  R8 Calligra

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

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

D19885: Implement logging category for excel plugins

[Dag Andersen]

danders accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R8 Calligra

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

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

D19884: Avoid deprecated QAbstractItemModel::reset()

[Dag Andersen]

danders added inline comments.

INLINE COMMENTS

> dcaliste wrote in TextDocumentStructureModel.cpp:308
> I did the same changes in https://phabricator.kde.org/D15102 at another place 
> and I put the begin and end at the start and end of the routine, like you 
> did. But I remember that I got review asking not to risk to change signal 
> order emission, in case, and put the begin/end just around the m_textDocument 
> assignment.
> 
> I'm not completely sure that I remember correctly and I cannot find it in 
> Phabricator anymore...

Hmmm, cannot find the diff either, but I *think* it was about an emit, wasn' it?

REPOSITORY
  R8 Calligra

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

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

D19912: Implement logging category for rtf-qt

[Dag Andersen]

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


  Strange indents, could you fix it?
  Otherwise ok.

REPOSITORY
  R8 Calligra

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

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