Review Request 129420: [calligra] Remove obsolete typedefs, deprecated since Qt 5.7

[Anthony Fieroni]

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

Review request for Calligra and Friedrich W. H. Kossebau.


Repository: calligra


Description
---

^^


Diffs
-

  braindump/plugins/stateshape/CategorizedItemDelegate.cpp ad661c2 
  libs/main/KoAutoSaveRecoveryDialog.cpp e543eaa 
  libs/main/KoDocumentSectionDelegate.h ad4b9f1 
  libs/main/KoDocumentSectionDelegate.cpp bcd7628 
  libs/widgets/KoDockWidgetTitleBar.cpp 4d0d1aa 
  plugins/textshape/dialogs/StylesCombo.cpp 9ca044b 
  plugins/textshape/dialogs/StylesDelegate.cpp faa90fb 

Diff: https://git.reviewboard.kde.org/r/129420/diff/


Testing
---

Build with Karbon.


Thanks,

Anthony Fieroni

Re: Review Request 129420: [calligra] Remove obsolete typedefs, deprecated since Qt 5.7

[Anthony Fieroni]

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

(Updated Nov. 17, 2016, 7:34 p.m.)


Status
--

This change has been marked as submitted.


Review request for Calligra and Friedrich W. H. Kossebau.


Changes
---

Submitted with commit 821f1048d5ff74d996dbb7cdcfca5405f5ab5b57 by Anthony 
Fieroni to branch master.


Repository: calligra


Description
---

^^


Diffs
-

  braindump/plugins/stateshape/CategorizedItemDelegate.cpp ad661c2 
  libs/main/KoAutoSaveRecoveryDialog.cpp e543eaa 
  libs/main/KoDocumentSectionDelegate.h ad4b9f1 
  libs/main/KoDocumentSectionDelegate.cpp bcd7628 
  libs/widgets/KoDockWidgetTitleBar.cpp 4d0d1aa 
  plugins/textshape/dialogs/StylesCombo.cpp 9ca044b 
  plugins/textshape/dialogs/StylesDelegate.cpp faa90fb 

Diff: https://git.reviewboard.kde.org/r/129420/diff/


Testing
---

Build with Karbon.


Thanks,

Anthony Fieroni

Review Request 129421: [karbon] Returning of Karbon as maintained product

[Anthony Fieroni]

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

Review request for Calligra, Camilla Boemann and Friedrich W. H. Kossebau.


Repository: calligra


Description
---

As https://community.kde.org/Calligra/Maintainers Karbon has a maintainer, so i 
can be Co-Maintainer, i'm missing experience with graphics nor vector graphics 
software but i will try to help and fixing bugs.


Diffs
-

  CalligraProducts.cmake 965e9d8 
  karbon/CMakeLists.txt ba775ad 
  karbon/main.cpp 2fa3f4d 
  karbon/ui/KarbonAboutData.h aa96ee1 

Diff: https://git.reviewboard.kde.org/r/129421/diff/


Testing
---

Builds.


Thanks,

Anthony Fieroni

Review Request 129435: [shapefiltereffects] Update ConvolveMatrixEffect kernel accordingly to order

[Anthony Fieroni]

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

Review request for Calligra, Camilla Boemann, Friedrich W. H. Kossebau, and Yue 
Liu.


Bugs: 353816
http://bugs.kde.org/show_bug.cgi?id=353816


Repository: calligra


Description
---

Added ability to save kernel changes by adding OK | Cancel buttons


Diffs
-

  plugins/shapefiltereffects/ConvolveMatrixEffectConfigWidget.cpp ef5b410 

Diff: https://git.reviewboard.kde.org/r/129435/diff/


Testing
---

Build and run with Karbon


Thanks,

Anthony Fieroni

Re: Review Request 129421: [karbon] Returning of Karbon as maintained product

[Anthony Fieroni]

> On Ноев. 18, 2016, 12:09 преди обяд, Camilla Boemann wrote:
> > It's great that you want to co maintain - i think you could even apply for 
> > maintaining without the "co-", but i also think it's a bit premature to 
> > remove that message. Maintain (be ambassador, fix bugs etc) for a month or 
> > so and if you are still warm about the idea we can remove the message.I 
> > mean the spot is yours if you want but make yourself a maintainer by deed 
> > before we make you one by name. Deal?
> > 
> > And just to emphazise it would be really great to see you work on karbon - 
> > it deserves to come to full glory!

So you want to return warn message on non-Release builds?


- Anthony


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


On Ноев. 17, 2016, 10:09 след обяд, Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129421/
> ---
> 
> (Updated Ноев. 17, 2016, 10:09 след обяд)
> 
> 
> Review request for Calligra, Camilla Boemann and Friedrich W. H. Kossebau.
> 
> 
> Repository: calligra
> 
> 
> Description
> ---
> 
> As https://community.kde.org/Calligra/Maintainers Karbon has a maintainer, so 
> i can be Co-Maintainer, i'm missing experience with graphics nor vector 
> graphics software but i will try to help and fixing bugs.
> 
> 
> Diffs
> -
> 
>   CalligraProducts.cmake 965e9d8 
>   karbon/CMakeLists.txt ba775ad 
>   karbon/main.cpp 2fa3f4d 
>   karbon/ui/KarbonAboutData.h aa96ee1 
> 
> Diff: https://git.reviewboard.kde.org/r/129421/diff/
> 
> 
> Testing
> ---
> 
> Builds.
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129421: [karbon] Returning of Karbon as maintained product

[Anthony Fieroni]

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

(Updated Ноев. 19, 2016, 9:38 след обяд)


Review request for Calligra, Camilla Boemann and Friedrich W. H. Kossebau.


Repository: calligra


Description
---

As https://community.kde.org/Calligra/Maintainers Karbon has a maintainer, so i 
can be Co-Maintainer, i'm missing experience with graphics nor vector graphics 
software but i will try to help and fixing bugs.


Diffs (updated)
-

  CalligraProducts.cmake 965e9d8 

Diff: https://git.reviewboard.kde.org/r/129421/diff/


Testing
---

Builds.


Thanks,

Anthony Fieroni

Re: Review Request 129421: [karbon] Returning of Karbon as maintained product

[Anthony Fieroni]

> On Ноев. 18, 2016, 12:09 преди обяд, Camilla Boemann wrote:
> > It's great that you want to co maintain - i think you could even apply for 
> > maintaining without the "co-", but i also think it's a bit premature to 
> > remove that message. Maintain (be ambassador, fix bugs etc) for a month or 
> > so and if you are still warm about the idea we can remove the message.I 
> > mean the spot is yours if you want but make yourself a maintainer by deed 
> > before we make you one by name. Deal?
> > 
> > And just to emphazise it would be really great to see you work on karbon - 
> > it deserves to come to full glory!
> 
> Anthony Fieroni wrote:
> So you want to return warn message on non-Release builds?
> 
> Camilla Boemann wrote:
> I'm sorry I don't understand what you mean? Also karbon doesn't have a 
> maintainter. That webpage is wrong/out of date. You are very welcome to 
> become the new maintainer, but let's not remove that message until we have 
> seen that you are staying with us

Ok. I will be the maintainer. I will add you to reviewer of my patches or other 
guy?


- Anthony


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


On Ноев. 17, 2016, 10:09 след обяд, Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129421/
> ---
> 
> (Updated Ноев. 17, 2016, 10:09 след обяд)
> 
> 
> Review request for Calligra, Camilla Boemann and Friedrich W. H. Kossebau.
> 
> 
> Repository: calligra
> 
> 
> Description
> ---
> 
> As https://community.kde.org/Calligra/Maintainers Karbon has a maintainer, so 
> i can be Co-Maintainer, i'm missing experience with graphics nor vector 
> graphics software but i will try to help and fixing bugs.
> 
> 
> Diffs
> -
> 
>   CalligraProducts.cmake 965e9d8 
>   karbon/CMakeLists.txt ba775ad 
>   karbon/main.cpp 2fa3f4d 
>   karbon/ui/KarbonAboutData.h aa96ee1 
> 
> Diff: https://git.reviewboard.kde.org/r/129421/diff/
> 
> 
> Testing
> ---
> 
> Builds.
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129421: [karbon] Returning of Karbon as maintained product

[Anthony Fieroni]

> On Ноев. 18, 2016, 12:09 преди обяд, Camilla Boemann wrote:
> > It's great that you want to co maintain - i think you could even apply for 
> > maintaining without the "co-", but i also think it's a bit premature to 
> > remove that message. Maintain (be ambassador, fix bugs etc) for a month or 
> > so and if you are still warm about the idea we can remove the message.I 
> > mean the spot is yours if you want but make yourself a maintainer by deed 
> > before we make you one by name. Deal?
> > 
> > And just to emphazise it would be really great to see you work on karbon - 
> > it deserves to come to full glory!
> 
> Anthony Fieroni wrote:
> So you want to return warn message on non-Release builds?
> 
> Camilla Boemann wrote:
> I'm sorry I don't understand what you mean? Also karbon doesn't have a 
> maintainter. That webpage is wrong/out of date. You are very welcome to 
> become the new maintainer, but let's not remove that message until we have 
> seen that you are staying with us
> 
> Anthony Fieroni wrote:
> Ok. I will be the maintainer. I will add you to reviewer of my patches or 
> other guy?
> 
> Camilla Boemann wrote:
> put calligra as reviewer, however we are not really using this 
> reviewboard anymore, but rather phabricator instead

So should i put this and other one patch in phabricator ?


- Anthony


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


On Ноев. 19, 2016, 9:38 след обяд, Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129421/
> ---
> 
> (Updated Ноев. 19, 2016, 9:38 след обяд)
> 
> 
> Review request for Calligra, Camilla Boemann and Friedrich W. H. Kossebau.
> 
> 
> Repository: calligra
> 
> 
> Description
> ---
> 
> As https://community.kde.org/Calligra/Maintainers Karbon has a maintainer, so 
> i can be Co-Maintainer, i'm missing experience with graphics nor vector 
> graphics software but i will try to help and fixing bugs.
> 
> 
> Diffs
> -
> 
>   CalligraProducts.cmake 965e9d8 
> 
> Diff: https://git.reviewboard.kde.org/r/129421/diff/
> 
> 
> Testing
> ---
> 
> Builds.
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129435: [shapefiltereffects] Update ConvolveMatrixEffect kernel accordingly to order

[Anthony Fieroni]

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

(Updated Nov. 20, 2016, 6:15 a.m.)


Status
--

This change has been marked as submitted.


Review request for Calligra, Camilla Boemann, Friedrich W. H. Kossebau, and Yue 
Liu.


Changes
---

Submitted with commit 9d8146e05b96ba8245816a06e087f1cc88af2a2e by Anthony 
Fieroni to branch master.


Bugs: 353816
http://bugs.kde.org/show_bug.cgi?id=353816


Repository: calligra


Description
---

Added ability to save kernel changes by adding OK | Cancel buttons


Diffs
-

  plugins/shapefiltereffects/ConvolveMatrixEffectConfigWidget.cpp ef5b410 

Diff: https://git.reviewboard.kde.org/r/129435/diff/


Testing
---

Build and run with Karbon


Thanks,

Anthony Fieroni

Review Request 129437: [widgets] Make sure destination xml file path exist

[Anthony Fieroni]

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

Review request for Calligra and Camilla Boemann.


Repository: calligra


Description
---

Otherwise it shown warning message ""Cannot write meta information to..."


Diffs
-

  libs/widgets/KoResourceServer.h 884e8ba 
  libs/widgets/KoResourceTagStore.cpp 3ef0f4f 

Diff: https://git.reviewboard.kde.org/r/129437/diff/


Testing
---

Build and run with Karbon.


Thanks,

Anthony Fieroni

Review Request 129438: [karbon] Correct usage of KPageDialog in KarbonConfigureDialog.cpp

[Anthony Fieroni]

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

Review request for Calligra and Camilla Boemann.


Repository: calligra


Description
---

Now config dialog saves changes correct, it's a porting script bug


Diffs
-

  karbon/ui/dialogs/KarbonConfigureDialog.cpp 0ba166d 

Diff: https://git.reviewboard.kde.org/r/129438/diff/


Testing
---

Build and run with Karbon.


Thanks,

Anthony Fieroni

Re: Review Request 129421: [karbon] Returning of Karbon as maintained product

[Anthony Fieroni]

> On Ноев. 18, 2016, 12:09 преди обяд, Camilla Boemann wrote:
> > It's great that you want to co maintain - i think you could even apply for 
> > maintaining without the "co-", but i also think it's a bit premature to 
> > remove that message. Maintain (be ambassador, fix bugs etc) for a month or 
> > so and if you are still warm about the idea we can remove the message.I 
> > mean the spot is yours if you want but make yourself a maintainer by deed 
> > before we make you one by name. Deal?
> > 
> > And just to emphazise it would be really great to see you work on karbon - 
> > it deserves to come to full glory!
> 
> Anthony Fieroni wrote:
> So you want to return warn message on non-Release builds?
> 
> Camilla Boemann wrote:
> I'm sorry I don't understand what you mean? Also karbon doesn't have a 
> maintainter. That webpage is wrong/out of date. You are very welcome to 
> become the new maintainer, but let's not remove that message until we have 
> seen that you are staying with us
> 
> Anthony Fieroni wrote:
> Ok. I will be the maintainer. I will add you to reviewer of my patches or 
> other guy?
> 
> Camilla Boemann wrote:
> put calligra as reviewer, however we are not really using this 
> reviewboard anymore, but rather phabricator instead
> 
> Anthony Fieroni wrote:
> So should i put this and other one patch in phabricator ?
> 
> Camilla Boemann wrote:
> no that is not needed - we allow many types of review - also in person or 
> via a mail - even in some cases self review

Can we ship this, Arch (removed Karbon from git build), KaOS (removed it 
entirely from repo)?


- Anthony


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


On Ноев. 19, 2016, 9:38 след обяд, Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129421/
> ---
> 
> (Updated Ноев. 19, 2016, 9:38 след обяд)
> 
> 
> Review request for Calligra, Camilla Boemann and Friedrich W. H. Kossebau.
> 
> 
> Repository: calligra
> 
> 
> Description
> ---
> 
> As https://community.kde.org/Calligra/Maintainers Karbon has a maintainer, so 
> i can be Co-Maintainer, i'm missing experience with graphics nor vector 
> graphics software but i will try to help and fixing bugs.
> 
> 
> Diffs
> -
> 
>   CalligraProducts.cmake 965e9d8 
> 
> Diff: https://git.reviewboard.kde.org/r/129421/diff/
> 
> 
> Testing
> ---
> 
> Builds.
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129438: [karbon] Correct usage of KPageDialog in KarbonConfigureDialog.cpp

[Anthony Fieroni]

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

(Updated Nov. 20, 2016, 5:18 p.m.)


Status
--

This change has been marked as submitted.


Review request for Calligra and Camilla Boemann.


Changes
---

Submitted with commit 64c64378f1eb2cfaf6ba51c18387af099583396d by Anthony 
Fieroni to branch master.


Repository: calligra


Description
---

Now config dialog saves changes correct, it's a porting script bug


Diffs
-

  karbon/ui/dialogs/KarbonConfigureDialog.cpp 0ba166d 

Diff: https://git.reviewboard.kde.org/r/129438/diff/


Testing
---

Build and run with Karbon.


Thanks,

Anthony Fieroni

Re: Review Request 129437: [widgets] Make sure destination xml file path exist

[Anthony Fieroni]

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

(Updated Nov. 20, 2016, 5:22 p.m.)


Status
--

This change has been marked as submitted.


Review request for Calligra and Camilla Boemann.


Changes
---

Submitted with commit 6657dac8e5f14e89586790e2f51a5c858432b05b by Anthony 
Fieroni to branch master.


Repository: calligra


Description
---

Otherwise it shown warning message ""Cannot write meta information to..."


Diffs
-

  libs/widgets/KoResourceServer.h 884e8ba 
  libs/widgets/KoResourceTagStore.cpp 3ef0f4f 

Diff: https://git.reviewboard.kde.org/r/129437/diff/


Testing
---

Build and run with Karbon.


Thanks,

Anthony Fieroni

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

[Anthony Fieroni]

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

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 129512: [stencil] Follow system color for stencilsdocker icons

[Anthony Fieroni]

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

(Updated Ноев. 21, 2016, 6:54 преди обяд)


Review request for Calligra and Camilla Boemann.


Changes
---

It looks better


Repository: calligra


Description
---

Still has icons who are drown incorrect, i blame factory, but need more 
invetigates.


Diffs (updated)
-

  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

Review Request 129514: [connectionTool] Check dynamic_cast return type

[Anthony Fieroni]

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

Review request for Calligra and Camilla Boemann.


Bugs: 326258
http://bugs.kde.org/show_bug.cgi?id=326258


Repository: calligra


Description
---

I can't reproduce these days, but it's safe to check. Correct is based on 
backtrace in bug report.


Diffs
-

  plugins/defaultTools/connectionTool/ConnectionTool.cpp 9e3dec6 

Diff: https://git.reviewboard.kde.org/r/129514/diff/


Testing
---

PS: I manage to work on 2 wishlist, Px (pixels) if can be enabled from settings 
(i still don't know why it's disabled) and export arrow line in SVG. Other bus 
looks really outdated.


Thanks,

Anthony Fieroni

Review Request 129531: [flake] Serialize / deserialize svg canonicalGradient

[Anthony Fieroni]

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

Review request for Calligra and Camilla Boemann.


Repository: calligra


Description
---

Enable write / parse canonicalGradient


Diffs
-

  libs/flake/svg/SvgParser.cpp 943c448 
  libs/flake/svg/SvgStyleWriter.cpp b949dd5 

Diff: https://git.reviewboard.kde.org/r/129531/diff/


Testing
---

Build and run with Karbon


Thanks,

Anthony Fieroni

Re: Review Request 129531: [flake] Serialize / deserialize svg canonicalGradient

[Anthony Fieroni]

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

(Updated Ноев. 22, 2016, 8:53 след обяд)


Review request for Calligra and Camilla Boemann.


Changes
---

spreadMethod


Repository: calligra


Description
---

Enable write / parse canonicalGradient


Diffs (updated)
-

  libs/flake/svg/SvgParser.cpp 943c448 
  libs/flake/svg/SvgStyleWriter.cpp b949dd5 

Diff: https://git.reviewboard.kde.org/r/129531/diff/


Testing
---

Build and run with Karbon


Thanks,

Anthony Fieroni

Review Request 129532: [shortcuts] Undo / Redo shortkeys

[Anthony Fieroni]

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

Review request for Calligra and Camilla Boemann.


Repository: calligra


Description
---

+ fullscreen action provide warning on runtime "Use 
actionCollection->setDefaultShortcut instead of QAction->setShortcut


Diffs
-

  libs/main/KoMainWindow.cpp 828d789 
  libs/main/KoView.cpp f3b6fd9 

Diff: https://git.reviewboard.kde.org/r/129532/diff/


Testing
---

Build and run with Karbon


Thanks,

Anthony Fieroni

Re: Review Request 129532: [shortcuts] Undo / Redo shortkeys

[Anthony Fieroni]

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

(Updated Ноев. 22, 2016, 10:30 след обяд)


Review request for Calligra and Camilla Boemann.


Changes
---

Use standard keysequence instead of hardcoded


Repository: calligra


Description
---

+ fullscreen action provide warning on runtime "Use 
actionCollection->setDefaultShortcut instead of QAction->setShortcut


Diffs (updated)
-

  libs/main/KoMainWindow.cpp 828d789 
  libs/main/KoView.cpp f3b6fd9 

Diff: https://git.reviewboard.kde.org/r/129532/diff/


Testing
---

Build and run with Karbon


Thanks,

Anthony Fieroni

Re: Review Request 129531: [flake] Serialize / deserialize svg conicalGradient

[Anthony Fieroni]

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

(Updated Ноев. 22, 2016, 11:07 след обяд)


Review request for Calligra and Camilla Boemann.


Changes
---

Fix typos.


Summary (updated)
-

[flake] Serialize / deserialize svg conicalGradient


Repository: calligra


Description (updated)
---

Enable write / parse conicalGradient


Diffs (updated)
-

  libs/flake/svg/SvgParser.cpp 943c448 
  libs/flake/svg/SvgStyleWriter.cpp b949dd5 

Diff: https://git.reviewboard.kde.org/r/129531/diff/


Testing
---

Build and run with Karbon


Thanks,

Anthony Fieroni

Re: Review Request 129532: [shortcuts] Undo / Redo shortkeys

[Anthony Fieroni]

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

(Updated Nov. 23, 2016, 5:36 a.m.)


Status
--

This change has been marked as submitted.


Review request for Calligra and Camilla Boemann.


Changes
---

Submitted with commit 4fb2711afc1125efdcc009d9e20ca05a4405b735 by Anthony 
Fieroni to branch master.


Repository: calligra


Description
---

+ fullscreen action provide warning on runtime "Use 
actionCollection->setDefaultShortcut instead of QAction->setShortcut


Diffs
-

  libs/main/KoMainWindow.cpp 828d789 
  libs/main/KoView.cpp f3b6fd9 

Diff: https://git.reviewboard.kde.org/r/129532/diff/


Testing
---

Build and run with Karbon


Thanks,

Anthony Fieroni

Re: Review Request 129531: [flake] Serialize / deserialize svg conicalGradient

[Anthony Fieroni]

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

(Updated Nov. 24, 2016, 7:57 a.m.)


Status
--

This change has been marked as submitted.


Review request for Calligra and Camilla Boemann.


Changes
---

Submitted with commit 823fed356b2ca0052428a5aa98240f318e586268 by Anthony 
Fieroni to branch master.


Repository: calligra


Description
---

Enable write / parse conicalGradient


Diffs
-

  libs/flake/svg/SvgParser.cpp 943c448 
  libs/flake/svg/SvgStyleWriter.cpp b949dd5 

Diff: https://git.reviewboard.kde.org/r/129531/diff/


Testing
---

Build and run with Karbon


Thanks,

Anthony Fieroni

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

[Anthony Fieroni]

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



Ping, is it has a problem?

- Anthony Fieroni


On Ноев. 21, 2016, 6:54 преди обяд, Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129512/
> ---
> 
> (Updated Ноев. 21, 2016, 6:54 преди обяд)
> 
> 
> 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 129514: [connectionTool] Check dynamic_cast return type

[Anthony Fieroni]

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



This can go, just in any case, no?

- Anthony Fieroni


On Ноев. 21, 2016, 7:42 преди обяд, Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129514/
> ---
> 
> (Updated Ноев. 21, 2016, 7:42 преди обяд)
> 
> 
> Review request for Calligra and Camilla Boemann.
> 
> 
> Bugs: 326258
> http://bugs.kde.org/show_bug.cgi?id=326258
> 
> 
> Repository: calligra
> 
> 
> Description
> ---
> 
> I can't reproduce these days, but it's safe to check. Correct is based on 
> backtrace in bug report.
> 
> 
> Diffs
> -
> 
>   plugins/defaultTools/connectionTool/ConnectionTool.cpp 9e3dec6 
> 
> Diff: https://git.reviewboard.kde.org/r/129514/diff/
> 
> 
> Testing
> ---
> 
> PS: I manage to work on 2 wishlist, Px (pixels) if can be enabled from 
> settings (i still don't know why it's disabled) and export arrow line in SVG. 
> Other bus looks really outdated.
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

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

[Anthony Fieroni]

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

(Updated Nov. 26, 2016, 5:50 a.m.)


Status
--

This change has been marked as submitted.


Review request for Calligra and Camilla Boemann.


Changes
---

Submitted with commit d5c8f3d4690f4e7223b11442bf1e4ef3b1f0d10a by Anthony 
Fieroni to branch master.


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 129514: [connectionTool] Check dynamic_cast return type

[Anthony Fieroni]

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

(Updated Nov. 26, 2016, 12:50 a.m.)


Status
--

This change has been marked as submitted.


Review request for Calligra and Camilla Boemann.


Changes
---

Submitted with commit f3f26b78c7b792e9f5de480d2fd00c98a48d8ba2 by Anthony 
Fieroni to branch master.


Bugs: 326258
http://bugs.kde.org/show_bug.cgi?id=326258


Repository: calligra


Description
---

I can't reproduce these days, but it's safe to check. Correct is based on 
backtrace in bug report.


Diffs
-

  plugins/defaultTools/connectionTool/ConnectionTool.cpp 9e3dec6 

Diff: https://git.reviewboard.kde.org/r/129514/diff/


Testing
---

PS: I manage to work on 2 wishlist, Px (pixels) if can be enabled from settings 
(i still don't know why it's disabled) and export arrow line in SVG. Other bus 
looks really outdated.


Thanks,

Anthony Fieroni

Re: Review Request 129421: [karbon] Returning of Karbon as maintained product

[Anthony Fieroni]

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

(Updated Ноев. 26, 2016, 8:12 преди обяд)


Status
--

This change has been marked as submitted.


Review request for Calligra, Camilla Boemann and Friedrich W. H. Kossebau.


Repository: calligra


Description
---

As https://community.kde.org/Calligra/Maintainers Karbon has a maintainer, so i 
can be Co-Maintainer, i'm missing experience with graphics nor vector graphics 
software but i will try to help and fixing bugs.


Diffs
-

  CalligraProducts.cmake 965e9d8 

Diff: https://git.reviewboard.kde.org/r/129421/diff/


Testing
---

Builds.


Thanks,

Anthony Fieroni

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

[Anthony Fieroni]

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




libs/main/KoApplication.cpp (line 252)
<https://git.reviewboard.kde.org/r/129800/#comment68176>

Correct is #elif
http://en.cppreference.com/w/cpp/preprocessor


- Anthony Fieroni


On Ян. 10, 2017, 6:34 след обяд, René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129800/
> ---
> 
> (Updated Ян. 10, 2017, 6:34 след обяд)
> 
> 
> 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 
> (except in MacPorts tweaked QStandardPaths!). The `PATH` env. variable also 
> shouldn't be *re*set and only needs changing (potentially!) in a standalone 
> app bundle build. I don't have a MS Windows dev. system so I've merged 
> Krita's way of setting `XDG_DATA_DIRS` with Calligra's current code. Note 
> that Qt/Win also doesn't seem to use that variable in `QStandardPaths`.
> - rewrites `KoResourcePathsImpl::mapTypeToQStandardPaths()` to use a much 
> more efficient static `QHash` table. *A priori* it should be possible to use 
> `QStandardPaths::AppDataLocation` to obtain the location of the app bundle 
> resources directory; it could be populated with symlinks into 
> `/path/to/foo.app/Contents/share` so resources can be found with minimum 
> changes to the build system (install locations). This will need to be 
> established going forward (knowing that my own main interest is with "linuxy 
> builds").
> - the change to `KoResourcePathsImpl::mapTypeToQStandardPaths()` only has a 
> real interest if it's used throughout the code instead of explicit use of 
> QStandardPaths locations. For now I have set this up through build-type 
> specific preprocessor macros in KoResourcePaths.h (because an enum would 
> probably have to be cast to work with the QSP methods). I haven't changed any 
> code to use those macros.
> 
> Not yet incorporated: tweaks to the `ecm_add_app_icon` calls to use its new 
> capability to generate an app icon from an SVG file (currently tested with 
> Karbon).
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 13ac88f 
>   libs/main/KoApplication.cpp 7b23f8d 
>   libs/widgets/KoResourcePaths.h 8830a5a 
>   libs/widgets/KoResourcePaths.cpp 7df9dc6 
> 
> Diff: https://git.reviewboard.kde.org/r/129800/diff/
> 
> 
> Testing
> ---
> 
> Karbon works as expected with this patch on Mac OS X 10.9.5 (and Linux) with 
> Qt 5.7.1 and KF5 5.29.0 installed under /opt/local . 
> 
> Without the patch Karbon crashes or aborts immediately on Mac because it 
> doesn't find a single resource in the locations indicated by the 
> inappropriate `XDG_DATA_DIRS` value.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

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

[Anthony Fieroni]

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



So when Karbon works, i'm +1, please try words / sheets are they worked as well 
as Karbon?

- Anthony Fieroni


On Ян. 10, 2017, 6:34 след обяд, René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129800/
> ---
> 
> (Updated Ян. 10, 2017, 6:34 след обяд)
> 
> 
> 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 
> (except in MacPorts tweaked QStandardPaths!). The `PATH` env. variable also 
> shouldn't be *re*set and only needs changing (potentially!) in a standalone 
> app bundle build. I don't have a MS Windows dev. system so I've merged 
> Krita's way of setting `XDG_DATA_DIRS` with Calligra's current code. Note 
> that Qt/Win also doesn't seem to use that variable in `QStandardPaths`.
> - rewrites `KoResourcePathsImpl::mapTypeToQStandardPaths()` to use a much 
> more efficient static `QHash` table. *A priori* it should be possible to use 
> `QStandardPaths::AppDataLocation` to obtain the location of the app bundle 
> resources directory; it could be populated with symlinks into 
> `/path/to/foo.app/Contents/share` so resources can be found with minimum 
> changes to the build system (install locations). This will need to be 
> established going forward (knowing that my own main interest is with "linuxy 
> builds").
> - the change to `KoResourcePathsImpl::mapTypeToQStandardPaths()` only has a 
> real interest if it's used throughout the code instead of explicit use of 
> QStandardPaths locations. For now I have set this up through build-type 
> specific preprocessor macros in KoResourcePaths.h (because an enum would 
> probably have to be cast to work with the QSP methods). I haven't changed any 
> code to use those macros.
> 
> Not yet incorporated: tweaks to the `ecm_add_app_icon` calls to use its new 
> capability to generate an app icon from an SVG file (currently tested with 
> Karbon).
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 13ac88f 
>   libs/main/KoApplication.cpp 7b23f8d 
>   libs/widgets/KoResourcePaths.h 8830a5a 
>   libs/widgets/KoResourcePaths.cpp 7df9dc6 
> 
> Diff: https://git.reviewboard.kde.org/r/129800/diff/
> 
> 
> Testing
> ---
> 
> Karbon works as expected with this patch on Mac OS X 10.9.5 (and Linux) with 
> Qt 5.7.1 and KF5 5.29.0 installed under /opt/local . 
> 
> Without the patch Karbon crashes or aborts immediately on Mac because it 
> doesn't find a single resource in the locations indicated by the 
> inappropriate `XDG_DATA_DIRS` value.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 129804: [Mac] enable app icon generation from the SVG icon (potentially for MS Windows too)

[Anthony Fieroni]

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




karbon/CMakeLists.txt (line 55)
<https://git.reviewboard.kde.org/r/129804/#comment68188>

Why only for Apple, it's needed some other changes ?


- Anthony Fieroni


On Jan. 11, 2017, 5:30 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129804/
> ---
> 
> (Updated Jan. 11, 2017, 5:30 p.m.)
> 
> 
> Review request for Calligra and KDE Software on Mac OS X.
> 
> 
> Repository: calligra
> 
> 
> Description
> ---
> 
> Hinted at in my previous RR: this patch concern the generation of app icons 
> on Mac and potentially on MS Windows.
> 
> Several points:
> 
> - The `iconutils` utility is limited or incorrectly invoked by 
> `ecm_add_app_icon`, leading to errors on OS X 10.9 when using PNG files 
> larger than 256x256px . 
> - `ecm_add_app_icon` takes care of copying the icon (.icns) file into the app 
> bundle; no need to install it explicitly
> - a recent change I committed to the ECM allows `ecm_add_app_icon` to accept 
> an SVG (or SVGz) file, and will create the app icon using that file if the 
> `ksvg2icns` utility is available (provided by KIconThemes so it should always 
> be available).
> 
> I hope that this modification will be released with ECM 5.30.0 . The ECM 
> being what they are there should be no issue requiring its latest version, 
> but the changes in the present patch will be transparent with older ECM 
> versions (except for warnings about ignoring unsupported icon source files).
> 
> There is an equivalent `svg2ico` utility in the KDEWin project which also 
> provides a `png2ico` implementation. I plan to propose an updated version for 
> inclusion with KIconThemes. That would allow feature parity for 
> `ecm_add_app_icon` on MSWin, but I would need someone else to test it (I 
> don't have a MSWin dev system).
> 
> 
> Diffs
> -
> 
>   braindump/src/CMakeLists.txt 95e0772 
>   flow/part/CMakeLists.txt f5d0893 
>   gemini/CMakeLists.txt c8713d6 
>   karbon/CMakeLists.txt ba775ad 
>   plan/CMakeLists.txt 25427da 
>   sheets/CMakeLists.txt 0256de5 
>   stage/app/CMakeLists.txt b5b4464 
>   words/app/CMakeLists.txt 8cf0157 
> 
> Diff: https://git.reviewboard.kde.org/r/129804/diff/
> 
> 
> Testing
> ---
> 
> tested on Mac and Linux with Qt 5.7.1 and FWs 5.29.0 , currently only for 
> Karbon and Words.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

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

[Anthony Fieroni]

> On Ян. 10, 2017, 10:28 след обяд, 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!

I response for only Karbon, wait for Camilla and Dag Andersen.


- Anthony


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


On Ян. 10, 2017, 6:34 след обяд, René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129800/
> ---
> 
> (Updated Ян. 10, 2017, 6:34 след обяд)
> 
> 
> 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 
> (except in MacPorts tweaked QStandardPaths!). The `PATH` env. variable also 
> shouldn't be *re*set and only needs changing (potentially!) in a standalone 
> app bundle build. I don't have a MS Windows dev. system so I've merged 
> Krita's way of setting `XDG_DATA_DIRS` with Calligra's current code. Note 
> that Qt/Win also doesn't seem to use that variable in `QStandardPaths`.
> - rewrites `KoResourcePathsImpl::mapTypeToQStandardPaths()` to use a much 
> more efficient static `QHash` table. *A priori* it should be possibl

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

[Anthony Fieroni]

anthonyfieroni added a comment.
This revision now requires changes to proceed.


  This review is more than an years old, is it still related?

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.
Restricted Application added a subscriber: Calligra-Devel-list.


  Ping?

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  I investigate couple of days, last year, in rendering and i think problem 
isn't there. So probably table, but i don't get suspicious code, so text layout 
can be.

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  Test documents -> 
https://europass.cedefop.europa.eu/documents/curriculum-vitae/templates-instructions

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 35423.

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9537?vs=24438&id=35423

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

AFFECTED FILES
  libs/textlayout/KoTextLayoutArea.cpp

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  In D9537#270932 , @boemann wrote:
  
  > And finally the code you already wrote can be simplyfied by by not having 
an else but always multiplying
  
  
  You mean to not percentage and multiply always 1.16?

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  I don't understand what you mean.

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  Ping, let put in this fix, what should be done?

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  This -> 
https://phabricator.kde.org/source/calligra/browse/master/plugins/textshape/dialogs/ParagraphIndentSpacing.cpp$124-132
 ?

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 38205.

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9537?vs=35423&id=38205

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

AFFECTED FILES
  libs/textlayout/KoTextLayoutArea.cpp
  plugins/textshape/dialogs/ParagraphIndentSpacing.cpp

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

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

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 38209.

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9537?vs=38205&id=38209

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

AFFECTED FILES
  libs/textlayout/KoTextLayoutArea.cpp
  plugins/textshape/dialogs/ParagraphIndentSpacing.cpp

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

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

[Anthony Fieroni]

anthonyfieroni closed this revision.
anthonyfieroni added a comment.


  
https://cgit.kde.org/calligra.git/commit/?id=805df22766026fac22154ae89b64dad532a6cbb4

REPOSITORY
  R8 Calligra

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

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

D11971: Use same keyword separator for read and write

[Anthony Fieroni]

anthonyfieroni added a comment.
Restricted Application added a subscriber: Calligra-Devel-list.


  It looks good +1

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  Values like 14.4, 28.8, 28.0-12.0, should be correct, how are they results?

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  In D9537#300367 , @boemann wrote:
  
  > given that we no multiply with1.16 the values should be: 12*1.16, 24*1.16,  
no
  
  
  It looks like - no
  
  > Actual   (blockLayout->lineAt(1).y()): 120.875
  >  Expected (0.0 + 18.0 + 100.0): 118
  
  and other it does not multiply by 1.16

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  Thanks, i really don't know how to change predefined values like 28.0 etc. i 
see you changed them i still not clear to me what are the new vales :)

INLINE COMMENTS

> TestBlockLayout.cpp:497
>  int lastLineNum = block1Layout->lineCount() - 1;
> -const qreal lineSpacing = 12.0 * 1.2;
> +const qreal lineSpacing = 12.0 * 1.16;
>  KoTextDocument(m_doc).setParaTableSpacingAtStart(false);

Can you use macro again?

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

[Anthony Fieroni]

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

REPOSITORY
  R8 Calligra

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

AFFECTED FILES
  plugins/artistictextshape/ArtisticTextShape.cpp

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

D14916: [ArtisticTextShape] Merge text ranges with same style

[Anthony Fieroni]

anthonyfieroni added a comment.


  Basically we want to be unchanged, but when it merged, now, it does not have 
guaranteed that appending to last will be nor same behavior nor correct.

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  You are right, when i see docs
  
  > /// Appends plain text to the last text range
  >  void appendText(const QString &plainText);

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

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 40031.

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14916?vs=39955&id=40031

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

AFFECTED FILES
  plugins/artistictextshape/ArtisticTextShape.cpp

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

D14916: [ArtisticTextShape] Merge text ranges with same style

[Anthony Fieroni]

This revision was automatically updated to reflect the committed changes.
Closed by commit R8:f5a88ba9040e: [ArtisticTextShape] Merge text ranges with 
same style (authored by anthonyfieroni).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14916?vs=40031&id=40141

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

AFFECTED FILES
  plugins/artistictextshape/ArtisticTextShape.cpp

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

D14975: Don't use obsolete QRectF functions

[Anthony Fieroni]

anthonyfieroni added a comment.


  You can make them at once, if there is no functionality changes.

REPOSITORY
  R8 Calligra

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

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

D14975: Don't use obsolete QRectF functions

[Anthony Fieroni]

anthonyfieroni added reviewers: danders, Calligra: 3.0.

REPOSITORY
  R8 Calligra

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

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

D15053: Consistently use qreal in libs/pigment

[Anthony Fieroni]

anthonyfieroni added a comment.


  You use it on Pinebook or similar ARM based laptop?

REPOSITORY
  R8 Calligra

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

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

D15053: Consistently use qreal in libs/pigment

[Anthony Fieroni]

anthonyfieroni added a comment.


  I'm OK with the patch, can you try Karbon as well?

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

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

REVISION SUMMARY
  View bound has same aspect ratio as content size, view box is square, so when 
calculation is applied result rect loose it ratio and always became a square 
like describe in bug report.
  
  CCBUG: 334608

TEST PLAN
  Now it performed current size bounds to view box, which will result in 
cutting size to fix view box (if needed) then it scaled to result size by 
keeping aspect ratio.

REPOSITORY
  R8 Calligra

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

AFFECTED FILES
  plugins/pathshapes/enhancedpath/EnhancedPathShape.cpp

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

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

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 40363.
anthonyfieroni added a comment.


  Fix wrong diff

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15056?vs=40362&id=40363

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

AFFECTED FILES
  plugins/pathshapes/enhancedpath/EnhancedPathShape.cpp

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

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

[Anthony Fieroni]

anthonyfieroni retitled this revision from "[EnhancedPathShape] Keep correct 
aspect ratio of odf files" to "[EnhancedPathShape] Keep correct aspect ratio of 
shapes in odf files".
anthonyfieroni edited the test plan for this revision.

REPOSITORY
  R8 Calligra

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

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

D15053: Consistently use qreal in libs/pigment

[Anthony Fieroni]

anthonyfieroni added a comment.


  In D15053#314717 , @dcaliste wrote:
  
  > I cannot test easily on arm because karbon depends on KF5kdelibs4support, 
which is not available.
  
  
  I will investigate to remove it as depend.

REPOSITORY
  R8 Calligra

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  Any objections to this?

REPOSITORY
  R8 Calligra

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

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

D15111: [KoUnit] Let's show pixel units

[Anthony Fieroni]

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

REVISION SUMMARY
  CCBUG: 312739

REPOSITORY
  R8 Calligra

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

AFFECTED FILES
  libs/main/KoView_p.h
  libs/main/config/KoConfigMiscPage.cpp
  libs/odf/KoUnit.cpp
  libs/odf/KoUnit.h
  libs/widgets/KoPageLayoutWidget.cpp

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

D15111: [KoUnit] Let's show pixel units

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 40536.
anthonyfieroni added a comment.


  Fix Tests as well.
  Notable change is that i made to always use round functions toXXX, do we need 
precision as offer qreal ?

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15111?vs=40532&id=40536

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

AFFECTED FILES
  libs/main/KoView_p.h
  libs/main/config/KoConfigMiscPage.cpp
  libs/odf/KoUnit.cpp
  libs/odf/KoUnit.h
  libs/odf/tests/TestKoUnit.cpp
  libs/widgets/KoPageLayoutWidget.cpp

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

D15111: [KoUnit] Let's show pixel units

[Anthony Fieroni]

anthonyfieroni added a comment.


  That's purely abstract, i think to add DPI spinbox next to pixel setting. I'm 
not a designer, but it looks like they want to work on *real* pixels that 
should look on the target device, that's maybe DPI config will be relevant.

REPOSITORY
  R8 Calligra

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

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

D15111: [KoUnit] Let's show pixel units

[Anthony Fieroni]

anthonyfieroni added a comment.


  I agree with that, but in some situations like targeting embedded device with 
especially knowing resolution :) I really don't know what can they'll see in 
pixels, but defined resolution is possible case.

REPOSITORY
  R8 Calligra

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

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

D15111: [KoUnit] Let's show pixel units

[Anthony Fieroni]

anthonyfieroni added a comment.


  Agree, maybe that's why it's vector graphics not nixes graphics. I really 
don't know what and how other vector graphics software offers these pixels :)

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

This revision was automatically updated to reflect the committed changes.
Closed by commit R8:da40d525d2c0: [EnhancedPathShape] Keep correct aspect ratio 
of shapes in odf files (authored by anthonyfieroni).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15056?vs=40363&id=40640

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

AFFECTED FILES
  plugins/pathshapes/enhancedpath/EnhancedPathShape.cpp

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

D15111: [KoUnit] Let's show pixel units

[Anthony Fieroni]

anthonyfieroni added a comment.


  We'll rethink of being this option or i should discard as unwanted?

REPOSITORY
  R8 Calligra

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

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

D15111: [KoUnit] Let's show pixel units

[Anthony Fieroni]

anthonyfieroni added a comment.


  @staniek did you want a DPI config spinbox? Can try this patch over master to 
approve it can be useful in Karbon? I keep in mind to make markers for SVG to 
have ability to save arrows, we have a bug report for that too. Maybe a little 
help will be needed :)

REPOSITORY
  R8 Calligra

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

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

D15111: [KoUnit] Let's show pixel units

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 41190.
anthonyfieroni added a comment.


  Complete remove of hide pixel.
  
  This review can stay open till someone find out it as useful, then it can 
reviewed.

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15111?vs=40536&id=41190

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

AFFECTED FILES
  filters/karbon/image/ImageExportOptionsWidget.cpp
  libs/main/KoView_p.h
  libs/main/config/KoConfigMiscPage.cpp
  libs/odf/KoUnit.cpp
  libs/odf/KoUnit.h
  libs/odf/tests/TestKoUnit.cpp
  libs/widgets/KoPageLayoutWidget.cpp
  plan/src/libs/main/KoView_p.h
  plan/src/libs/odf/tests/TestKoUnit.cpp
  plan/src/libs/widgets/KoPageLayoutWidget.cpp
  sheets/part/Doc.cpp
  sheets/part/dialogs/PreferenceDialog.cpp

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

D15403: {Style] Default sheets to white background

[Anthony Fieroni]

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

REVISION SUMMARY
  Other applications does not respect palette for background but 
Calligrasheets. Make default to white background with black text, when starts 
ending cell background is palette' one, text too.

REPOSITORY
  R8 Calligra

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

AFFECTED FILES
  sheets/Style.cpp

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

D15403: {Style] Default sheets to white background

[Anthony Fieroni]

anthonyfieroni edited the summary of this revision.

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  It's inspired in forum discussion, it has predominately advice that other 
Calligra apps has a white background despite palette but sheets.

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

[Anthony Fieroni]

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

REVISION SUMMARY
  CCBUG: 379255

REPOSITORY
  R8 Calligra

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

AFFECTED FILES
  filters/libmsooxml/MsooXmlDrawingReaderTableImpl.h

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

D15412: [filters] Extend table lifetime

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 41377.
anthonyfieroni edited the test plan for this revision.
anthonyfieroni added a comment.


  Aggressive refactor, it makes m_table a shared pointer, presetTables is one 
time initialized.

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15412?vs=41359&id=41377

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

AFFECTED FILES
  filters/libmsooxml/MsooXmlDrawingReaderTableImpl.h
  filters/libmsooxml/MsooXmlDrawingReaderTableMethods.h
  filters/libmsooxml/PresetDrawingMLTables.h
  filters/libodf2/KoTable.h
  filters/stage/pptx/PptxXmlDocumentReader.h
  filters/stage/pptx/PptxXmlSlideReader.h
  filters/words/docx/import/DocxXmlDocumentReader.cpp
  filters/words/docx/import/DocxXmlDocumentReader.h

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

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

[Anthony Fieroni]

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

REVISION SUMMARY
  Also QTextLine can crash due to its validity, Qt implementation incorporate a 
pointer that isn't check exclusively but in isValid
  
  CCBUG: 381341

TEST PLAN
  Attached document is now open and pretty responsive due to its 243 pages :)

REPOSITORY
  R8 Calligra

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

AFFECTED FILES
  libs/textlayout/KoTextLayoutNoteArea.cpp
  libs/textlayout/KoTextLayoutTableArea.cpp

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

[Anthony Fieroni]

anthonyfieroni retitled this revision from "[textlayout] Don't enter infinite 
loop when table are misfit" to "[textlayout] Don't enter infinite loop when 
table is misfit".

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

[Anthony Fieroni]

anthonyfieroni edited the summary of this revision.

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

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 41418.
anthonyfieroni added a comment.


  Initialize labelYOffset even layout line is not valid

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15428?vs=41409&id=41418

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

AFFECTED FILES
  libs/textlayout/KoTextLayoutNoteArea.cpp
  libs/textlayout/KoTextLayoutTableArea.cpp

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

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> boemann wrote in KoTextLayoutTableArea.cpp:464
> nah this is too aggressive.
> There are definitely cases where setting to 0 is the correct thing to do.
> There might be some times we enter an infinite loop yes, but we need to catch 
> this is some other way

I should see how this would work, because if we reset it to 0 every time it 
takes same way to parse, as it can see cursor->row is force reset only here.

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  In D15428#324451 , @boemann wrote:
  
  > One reason why this might fail is if virginpage becomes false when we add 
the table headerrow (if we can fix this and that doesn't have other ill effect 
then I would prefer such a solution).
  >  So find out where we set virginpage to false, and make sure it doesn't go 
from true to false when adding the header row
  >
  > I hope this makes sense
  
  
  Only one i can see before 
https://phabricator.kde.org/source/calligra/browse/master/libs/textlayout/KoTextLayoutArea.cpp$540
 to setted to true every time what you think?

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  It does not work, as you can see it overriding here 
https://phabricator.kde.org/source/calligra/browse/master/libs/textlayout/KoTextLayoutArea.cpp$571
 so that's why i want to set it true before loop.

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  Something as
  
if (d->headerRows) {
cursor->row = 0;
}

REPOSITORY
  R8 Calligra

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

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

D15412: [filters] Extend table lifetime

[Anthony Fieroni]

anthonyfieroni added a comment.


  Ping on this, did you try 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

D15412: [filters] Extend table lifetime

[Anthony Fieroni]

anthonyfieroni added a comment.


  It can be allocated only in read_tbl but as you can see in bug report, 
use-after-free is very common to happen.

INLINE COMMENTS

> staniek wrote in MsooXmlDrawingReaderTableMethods.h:40
> This is declaration-only header, so better no, otherwise allocation of a new 
> KoTable is added to all places we include the header, e.g. also 
> PptxXmlSlideReader and in the future.

That isn't a problem, it's initialized at constructor time.

> staniek wrote in DocxXmlDocumentReader.cpp:6082
> Isn't this change in logic?

No

REPOSITORY
  R8 Calligra

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

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

D15412: [filters] Extend table lifetime

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 41552.

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15412?vs=41377&id=41552

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

AFFECTED FILES
  filters/libmsooxml/MsooXmlDrawingReaderTableImpl.h
  filters/libmsooxml/MsooXmlDrawingReaderTableMethods.h
  filters/libmsooxml/PresetDrawingMLTables.h
  filters/libodf2/KoTable.cpp
  filters/libodf2/KoTable.h
  filters/stage/pptx/PptxXmlSlideReader.h
  filters/words/docx/import/DocxXmlDocumentReader.cpp
  filters/words/docx/import/DocxXmlDocumentReader.h

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

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

[Anthony Fieroni]

anthonyfieroni updated this revision to Diff 41564.
anthonyfieroni added a comment.


  Another try:
  Don't mark table as misfit when it does not have header rows

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15428?vs=41418&id=41564

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

AFFECTED FILES
  libs/textlayout/KoTextLayoutNoteArea.cpp
  libs/textlayout/KoTextLayoutTableArea.cpp

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  Any other ideas for special handling of headerRows == 0?

REPOSITORY
  R8 Calligra

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

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

D12839: Get Calligra to compile with Qt 5.11

[Anthony Fieroni]

anthonyfieroni closed this revision.
anthonyfieroni added a comment.


  It was merged.

REPOSITORY
  R8 Calligra

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

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

D15403: {Style] Default sheets to white background

[Anthony Fieroni]

This revision was automatically updated to reflect the committed changes.
Closed by commit R8:e5fc394f4a54: [Style] Default sheets to white background 
(authored by anthonyfieroni).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15403?vs=41338&id=41840

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

AFFECTED FILES
  sheets/Style.cpp

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

D15412: [filters] Extend table lifetime

[Anthony Fieroni]

This revision was automatically updated to reflect the committed changes.
Closed by commit R8:cb7ff65d2e7c: Extend table lifetime (authored by 
anthonyfieroni).

REPOSITORY
  R8 Calligra

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15412?vs=41552&id=41841

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

AFFECTED FILES
  filters/libmsooxml/MsooXmlDrawingReaderTableImpl.h
  filters/libmsooxml/MsooXmlDrawingReaderTableMethods.h
  filters/libmsooxml/PresetDrawingMLTables.h
  filters/libodf2/KoTable.cpp
  filters/libodf2/KoTable.h
  filters/stage/pptx/PptxXmlSlideReader.h
  filters/words/docx/import/DocxXmlDocumentReader.cpp
  filters/words/docx/import/DocxXmlDocumentReader.h

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

D15617: Move away from kdebug and kdelibs4 in filter/sheets

[Anthony Fieroni]

anthonyfieroni added a comment.


  This is old practice, please make a logging category -> 
https://doc.qt.io/qt-5/qloggingcategory.html

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  Also can  you add parentheses over single line code and if you know better 
way to get impl() without duplicate calls will be great :)

INLINE COMMENTS

> View.cpp:160
>  canvas->setGeometry(0, 0, q->width(), q->height());
> +
> static_cast(document->impl())->setBackgroundColor(q->fillColor());
> +static_cast(document->impl())->setPageCache(m_cache);

You don't check against impl is not nullptr?

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

[Anthony Fieroni]

anthonyfieroni added a subscriber: boemann.
anthonyfieroni added a comment.


  I'm OK with it +1, @danders or @boemann should accept it.

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

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> TextTool.cpp:2036
> +}
> +m_textShape = textShape;
> +}

If it has more than one shape that not satisfy above condition  (2032) 
m_textShape will point to last one, not to first as current behavior. So maybe

  if (!m_textShape) {
  m_textShape = textShape;
  }

It will point to first when it does not have exactly match.

> TextTool.cpp:3121
> +// but just disabling the insert_annotation action somehow does not work.
> +if (m_textShape->shapeId() == AnnotationShape_SHAPEID) {
> +return;

Can m_textShape be a nullptr ?

REPOSITORY
  R8 Calligra

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

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

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

[Anthony Fieroni]

anthonyfieroni added a comment.


  Let's make some fix about that.

REPOSITORY
  R8 Calligra

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

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

D16127: Annotation: Block adding comment to an annotation shape

[Anthony Fieroni]

anthonyfieroni added a comment.


  Looks good to me +1

REPOSITORY
  R8 Calligra

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

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

D16289: Karbon: Add Anthony as maintainer and remove unmaintained

[Anthony Fieroni]

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

REPOSITORY
  R8 Calligra

BRANCH
  karbon_danders

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

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