Re: Review Request: Better integration of pencil tool into Krita

2012-12-29 Thread Boudewijn Rempt 

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

Ship it!


Cool work!

- Boudewijn Rempt


On Dec. 29, 2012, 2:23 a.m., Sven Langkamp wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107996/
> ---
> 
> (Updated Dec. 29, 2012, 2:23 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> ---
> 
> The change moves the pencil tool to basicflakes. It used in Krita like the 
> path tool to allow the path to be painted with the current brush. Code is 
> shared between Krita pencil and path tools.
> 
> 
> Diffs
> -
> 
>   karbon/plugins/tools/CMakeLists.txt 78ef2f3 
>   karbon/plugins/tools/CalligraphyTool/KarbonCalligraphicShape.cpp b2870e8 
>   karbon/plugins/tools/CalligraphyTool/KarbonCalligraphyTool.cpp c97158e 
>   karbon/plugins/tools/CalligraphyTool/KarbonSimplifyPath.cpp d292508 
>   karbon/plugins/tools/KarbonCurveFit.h 10e5b20 
>   karbon/plugins/tools/KarbonCurveFit.cpp bf92995 
>   karbon/plugins/tools/KarbonPencilTool.h 2455876 
>   karbon/plugins/tools/KarbonPencilTool.cpp 0812272 
>   karbon/plugins/tools/KarbonPencilToolFactory.h f88ec27 
>   karbon/plugins/tools/KarbonPencilToolFactory.cpp 7d28f77 
>   karbon/plugins/tools/KarbonToolsPlugin.cpp 6ddb06f 
>   krita/data/kritarc 3d96fc9 
>   krita/plugins/tools/defaulttools/CMakeLists.txt 24ec519 
>   krita/plugins/tools/defaulttools/default_tools.cc 8ff87a0 
>   krita/plugins/tools/defaulttools/kis_tool_path.h f32ee61 
>   krita/plugins/tools/defaulttools/kis_tool_path.cc a464af4 
>   krita/plugins/tools/defaulttools/kis_tool_pencil.h PRE-CREATION 
>   krita/plugins/tools/defaulttools/kis_tool_pencil.cc PRE-CREATION 
>   krita/ui/tool/kis_tool_shape.h 6cdfbb8 
>   krita/ui/tool/kis_tool_shape.cc 046e0d5 
>   libs/basicflakes/CMakeLists.txt 48dcdee 
>   libs/basicflakes/plugin/CMakeLists.txt e58bb53 
>   libs/basicflakes/plugin/KoPencilToolFactory.h PRE-CREATION 
>   libs/basicflakes/plugin/KoPencilToolFactory.cpp PRE-CREATION 
>   libs/basicflakes/plugin/Plugin.cpp 54672a9 
>   libs/basicflakes/tools/KoPencilTool.h PRE-CREATION 
>   libs/basicflakes/tools/KoPencilTool.cpp PRE-CREATION 
>   libs/flake/CMakeLists.txt 11de8f4 
>   libs/flake/KoCurveFit.h PRE-CREATION 
>   libs/flake/KoCurveFit.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/107996/diff/
> 
> 
> Testing
> ---
> 
> Tested, works.
> 
> 
> Thanks,
> 
> Sven Langkamp
> 
>

___
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

Re: Review Request: Better integration of pencil tool into Krita

2012-12-29 Thread Commit Hook 

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


This review has been submitted with commit 
a8b3f36b39bc486fe5b7e92e0f0cee4c5c17b541 by Sven Langkamp to branch master.

- Commit Hook


On Dec. 29, 2012, 2:23 a.m., Sven Langkamp wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107996/
> ---
> 
> (Updated Dec. 29, 2012, 2:23 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> ---
> 
> The change moves the pencil tool to basicflakes. It used in Krita like the 
> path tool to allow the path to be painted with the current brush. Code is 
> shared between Krita pencil and path tools.
> 
> 
> Diffs
> -
> 
>   karbon/plugins/tools/CMakeLists.txt 78ef2f3 
>   karbon/plugins/tools/CalligraphyTool/KarbonCalligraphicShape.cpp b2870e8 
>   karbon/plugins/tools/CalligraphyTool/KarbonCalligraphyTool.cpp c97158e 
>   karbon/plugins/tools/CalligraphyTool/KarbonSimplifyPath.cpp d292508 
>   karbon/plugins/tools/KarbonCurveFit.h 10e5b20 
>   karbon/plugins/tools/KarbonCurveFit.cpp bf92995 
>   karbon/plugins/tools/KarbonPencilTool.h 2455876 
>   karbon/plugins/tools/KarbonPencilTool.cpp 0812272 
>   karbon/plugins/tools/KarbonPencilToolFactory.h f88ec27 
>   karbon/plugins/tools/KarbonPencilToolFactory.cpp 7d28f77 
>   karbon/plugins/tools/KarbonToolsPlugin.cpp 6ddb06f 
>   krita/data/kritarc 3d96fc9 
>   krita/plugins/tools/defaulttools/CMakeLists.txt 24ec519 
>   krita/plugins/tools/defaulttools/default_tools.cc 8ff87a0 
>   krita/plugins/tools/defaulttools/kis_tool_path.h f32ee61 
>   krita/plugins/tools/defaulttools/kis_tool_path.cc a464af4 
>   krita/plugins/tools/defaulttools/kis_tool_pencil.h PRE-CREATION 
>   krita/plugins/tools/defaulttools/kis_tool_pencil.cc PRE-CREATION 
>   krita/ui/tool/kis_tool_shape.h 6cdfbb8 
>   krita/ui/tool/kis_tool_shape.cc 046e0d5 
>   libs/basicflakes/CMakeLists.txt 48dcdee 
>   libs/basicflakes/plugin/CMakeLists.txt e58bb53 
>   libs/basicflakes/plugin/KoPencilToolFactory.h PRE-CREATION 
>   libs/basicflakes/plugin/KoPencilToolFactory.cpp PRE-CREATION 
>   libs/basicflakes/plugin/Plugin.cpp 54672a9 
>   libs/basicflakes/tools/KoPencilTool.h PRE-CREATION 
>   libs/basicflakes/tools/KoPencilTool.cpp PRE-CREATION 
>   libs/flake/CMakeLists.txt 11de8f4 
>   libs/flake/KoCurveFit.h PRE-CREATION 
>   libs/flake/KoCurveFit.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/107996/diff/
> 
> 
> Testing
> ---
> 
> Tested, works.
> 
> 
> Thanks,
> 
> Sven Langkamp
> 
>

___
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

RR for removing 5 unused files from filters/sheets/csv

2012-12-29 Thread Friedrich W. H. Kossebau 
Hi,

grepping for Q3 I found that these 5 files are no longer used. Seems it was 
forgotten to remove them in the commit 
038515dfc447d0a3e2a6cfdd872efc67fcc1076d
Filters CSV Import.
Switch to the shared KoCsvImportDialog.


Anyone objecting to remove these files?

filters/sheets/csv/csvdialog.cpp
filters/sheets/csv/csvdialog.h
filters/sheets/csv/dialogui.ui
filters/sheets/csv/xmltree.cc
filters/sheets/csv/xmltree.h

Cheers
Friedrich
___
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

Re: Review Request: Make the epub filter handle math formulas

2012-12-29 Thread Inge Wallin 


> On Dec. 28, 2012, 2:45 p.m., C. Boemann wrote:
> > there is a few places where you have:
> > 
> > if ()
> > {
> > 
> > and one place where there is no {} after if
> > 
> > other than that I've not looked yet - I assume you want moji to review

The 

if ()
{

construct is only used where the test is >1 line.  If I put the { at the end of 
the line, the first statement inside the brackets will be perfectly aligned 
with the test. I find that a bit difficult to read, hence this way of getting 
around it.


- Inge


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


On Dec. 28, 2012, 2:48 a.m., Inge Wallin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107969/
> ---
> 
> (Updated Dec. 28, 2012, 2:48 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> ---
> 
> This patch implements support for math formulas in the EPUB filter. This is 
> the first one of the EPUB3 features that we want to add to Calligra 2.7.
> 
> This version only supports math formulas saved as an embedded document, like 
> LibreOffice and the OpenOffice variants save it. Calligra saves math formulas 
> as inline mathML in the frame, which is not supported by this version. I 
> thought that I could get some initial feedback while implementing support for 
> the Calligra way too.
> 
> 
> Diffs
> -
> 
>   filters/words/epub/OdfParser.cpp 6069b89 
>   filters/words/epub/OdtHtmlConverter.h 68aaffa 
>   filters/words/epub/OdtHtmlConverter.cpp d4b7199 
>   filters/words/epub/exportepub2.cpp 84d8a90 
>   filters/words/epub/exporthtml.cpp bdd33e7 
> 
> Diff: http://git.reviewboard.kde.org/r/107969/diff/
> 
> 
> Testing
> ---
> 
> Created one simple odt using OOo which has a formula and some text.
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

___
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

Re: Review Request: Make the epub filter handle math formulas

2012-12-29 Thread Inge Wallin 

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

(Updated Dec. 30, 2012, 4:44 a.m.)


Review request for Calligra.


Changes
---

Second version:
 - Fix all issues pointed out by reviewers of version 1
 - Implement support for inline math formulas

I consider this code finished now.


Description
---

This patch implements support for math formulas in the EPUB filter. This is the 
first one of the EPUB3 features that we want to add to Calligra 2.7.

This version only supports math formulas saved as an embedded document, like 
LibreOffice and the OpenOffice variants save it. Calligra saves math formulas 
as inline mathML in the frame, which is not supported by this version. I 
thought that I could get some initial feedback while implementing support for 
the Calligra way too.


Diffs (updated)
-

  filters/words/epub/OdfParser.cpp 6069b89 
  filters/words/epub/OdtHtmlConverter.h 68aaffa 
  filters/words/epub/OdtHtmlConverter.cpp e5e0edc 
  filters/words/epub/TODO e634a05 
  filters/words/epub/exportepub2.cpp cfd50c3 
  filters/words/epub/exporthtml.cpp 5bb44aa 

Diff: http://git.reviewboard.kde.org/r/107969/diff/


Testing
---

Created one simple odt using OOo which has a formula and some text.


Thanks,

Inge Wallin

___
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

Re: Review Request: Make the epub filter handle math formulas

2012-12-29 Thread C. Boemann 


> On Dec. 28, 2012, 2:45 p.m., C. Boemann wrote:
> > there is a few places where you have:
> > 
> > if ()
> > {
> > 
> > and one place where there is no {} after if
> > 
> > other than that I've not looked yet - I assume you want moji to review
> 
> Inge Wallin wrote:
> The 
> 
> if ()
> {
> 
> construct is only used where the test is >1 line.  If I put the { at the 
> end of the line, the first statement inside the brackets will be perfectly 
> aligned with the test. I find that a bit difficult to read, hence this way of 
> getting around it.
>

I'm not arguing against the merrits of it. I have personally always preferred 
this way of placing braces. However I also think we should follow the hacking 
style at all times. This way may be easier for you to read but the point of a 
acking style is that anyone with a minimum of trouble can come in and work on 
the code. After all we are supposed to be an open community.

For case like you desribe I personally write like this (knowing full well that 
this is my personal style as well (though not prohibited by the official style):

if (bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla
bla bla bla bla bla) {
foo;
bar;
}

another way not against the hacking style would be:

if (bla bla bla bla bla bla bla bla bla
bla bla bla bla bla bla bla
bla bla bla bla bla) {
foo;
bar;
}

In the end I don't care all that much about this issue, but will close by 
saying that the day we do sweeping hacking style cleanups your carefully 
crafted (and special cased) exceptions would be probably be gone. My variations 
are more likely not to be touched.


- C.


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


On Dec. 30, 2012, 4:44 a.m., Inge Wallin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107969/
> ---
> 
> (Updated Dec. 30, 2012, 4:44 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> ---
> 
> This patch implements support for math formulas in the EPUB filter. This is 
> the first one of the EPUB3 features that we want to add to Calligra 2.7.
> 
> This version only supports math formulas saved as an embedded document, like 
> LibreOffice and the OpenOffice variants save it. Calligra saves math formulas 
> as inline mathML in the frame, which is not supported by this version. I 
> thought that I could get some initial feedback while implementing support for 
> the Calligra way too.
> 
> 
> Diffs
> -
> 
>   filters/words/epub/OdfParser.cpp 6069b89 
>   filters/words/epub/OdtHtmlConverter.h 68aaffa 
>   filters/words/epub/OdtHtmlConverter.cpp e5e0edc 
>   filters/words/epub/TODO e634a05 
>   filters/words/epub/exportepub2.cpp cfd50c3 
>   filters/words/epub/exporthtml.cpp 5bb44aa 
> 
> Diff: http://git.reviewboard.kde.org/r/107969/diff/
> 
> 
> Testing
> ---
> 
> Created one simple odt using OOo which has a formula and some text.
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

___
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

Review Request: Make a real dynamic library out of kdgantt

2012-12-29 Thread Inge Wallin 

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

Review request for Calligra and Stuart Dickson.


Description
---

This is an adaption of the same patch for 2.6.

This patch needs to be tested on Windows, which I hope stuart can do.


Diffs
-

  3rdparty/kdgantt/CMakeLists.txt 4a75084 
  3rdparty/kdgantt/kdganttconstraintmodel.h bc1ed5c 
  3rdparty/kdgantt/kdganttview.cpp a09edf1 
  CMakeLists.txt 09d98be 
  plan/libs/ui/CMakeLists.txt c80f891 
  plan/workpackage/CMakeLists.txt 7ebedce 

Diff: http://git.reviewboard.kde.org/r/108019/diff/


Testing
---


Thanks,

Inge Wallin

___
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

Re: Review Request: Make a real dynamic library out of kdgantt

2012-12-29 Thread C. Boemann 

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


I think the code looks nice enough, so stuart are free to approve as far as i'm 
concerned.

I have one little question/nitpick


3rdparty/kdgantt/CMakeLists.txt


uhm i don't get why we can't remove it already.



- C. Boemann


On Dec. 30, 2012, 5:21 a.m., Inge Wallin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108019/
> ---
> 
> (Updated Dec. 30, 2012, 5:21 a.m.)
> 
> 
> Review request for Calligra and Stuart Dickson.
> 
> 
> Description
> ---
> 
> This is an adaption of the same patch for 2.6.
> 
> This patch needs to be tested on Windows, which I hope stuart can do.
> 
> 
> Diffs
> -
> 
>   3rdparty/kdgantt/CMakeLists.txt 4a75084 
>   3rdparty/kdgantt/kdganttconstraintmodel.h bc1ed5c 
>   3rdparty/kdgantt/kdganttview.cpp a09edf1 
>   CMakeLists.txt 09d98be 
>   plan/libs/ui/CMakeLists.txt c80f891 
>   plan/workpackage/CMakeLists.txt 7ebedce 
> 
> Diff: http://git.reviewboard.kde.org/r/108019/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Inge Wallin
> 
>

___
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel