Details

Reviewers

staniek

Commits

R14:1b554d8e7547: Improve ORODocument

Summary

Port OROPage

Port OROSection

Port and improve OROPrimitive

Port OROTextBox

Port OROLine

Port OROImage

Port OROPicture

Port OPORect and OROEllipse

Port OROCheck

Clone using methods not accessing the d pointer directtly

Test Plan

run kreportexample and ensure rendering as expected

Diff Detail

Repository

R14 KReport

Branch

dptr_oro

Lint

No Linters Available

Unit

No Unit Test Coverage

There are a very large number of changes, so older changes are hidden. Show Older Changes

staniek added inline comments.Nov 29 2016, 12:46 AM

src/common/KReportRenderObjects.cpp
57	clear is not really needed
60	same here
71	Second arg not needed
76	Second arg not needed, default constructed value is returned here, which for Qt is nullptr.
90–95	Second arg not needed
100	Second arg not needed
143	For symmetry don't we want to also delete page?
148	For symmetry don't we want to also delete section?
183	not really needed
199	not really needed
218	Second arg not needed
223	Second arg not needed
267	For symmetry don't we want to also delete primitive?
287	KReport::recordCount() is qint64 so how about using qint64 here too?
293	Let's initialize all members here because it's possible.
321	Second arg not needed
326	Second arg not needed
421	If we add `delete` to OROPage::removePrimitive() this line won't be correct. Either: Add void OROPage::takePrimitive(OROPrimitive* primitive) and use it here, or call d->page->d->primitives.removeOne(this)
470	TODO: rename to KReportTextStyleData, add initialization of KReportTextStyleData::alignment and KReportTextStyleData::backgroundOpacity.
484–492	Let's initialize all this in Private::Private(). Cleaner and less Coverity warnings.
533–583	can be `clone() const`
595	Missing copying: wordWrap canGrow requiresPostProcessing
645	can be `clone() const`
699–719	can be `clone() const`
751	can be `clone() const`
806–821	can be `clone() const`
876–881	can be `clone() const`
958	can be `clone() const`
src/common/KReportRenderObjects.h
56–57	+title param.
60	While we're looking here. Add docs informing that ownership is transferred.
62	Add docs informing that the page object will be removed and deleted.
67	Add docs informing that ownership is transferred.
68	Setion -> Section
68	Add docs informing that the section object will be removed and deleted.
200–201	How about keeping X and Y by using Qt::Orientation and removing Sort?
205	+const
274–275	Not done...
317	let's add arg name
319–320	let's add arg name
322–323	let's add arg name
342–343	let's add arg name
345–346	let's add arg name
397–398	let's add arg name
417–418	let's add arg name
419–420	let's add arg name
420–421	let's add arg name

This revision now requires changes to proceed.Nov 29 2016, 12:47 AM

piggz marked 39 inline comments as done.Nov 30 2016, 9:20 PM

Implement review comments

Not done TODO as that will be done in another class

piggz marked 2 inline comments as done.Nov 30 2016, 10:08 PM

2 missed comments

Almost there!

src/common/KReportRenderObjects.cpp
595	TODO
967	TODO
src/common/KReportRenderObjects.h
56–57	TODO
68	TODO
216–217	+param name
274–275	TODO
src/renderer/KReportHTMLCSSRenderer_p.cpp
96	TODO
src/renderer/KReportKSpreadRenderer.cpp
49	typo

This revision now requires changes to proceed.Nov 30 2016, 11:13 PM

Implement remaining TODOs

Almost there just one thing - xLessThan!

src/common/KReportRenderObjects.h
224–227	I mean xLessThan

This revision now requires changes to proceed.Dec 4 2016, 12:10 AM

Yes, i meant to ask a question about that.

What do you mean 'move to cpp'?

Do you mean, it will remain a static method of OROSection, OROSection::Private or just a static function that is not a member of anything?

In D3423#66959, @piggz wrote:

Yes, i meant to ask a question about that.

What do you mean 'move to cpp'?

Do you mean, it will remain a static method of OROSection, OROSection::Private or just a static function that is not a member of anything?

Sorry Adam, I mean a static function in cpp, one that is not a member of anything.

Move function to cpp

all done!

staniek requested changes to this revision.Dec 4 2016, 10:14 PM

staniek edited edge metadata.

staniek added inline comments.

src/common/KReportRenderObjects.cpp
23	Let's have `static` or use `namespace { }`

This revision now requires changes to proceed.Dec 4 2016, 10:14 PM

Having read the differences between static and using a namespace I will use static. Hopefully we can close this later this evening :)

Final comment!

Good job :)

This revision is now accepted and ready to land.Dec 5 2016, 10:09 PM

Closed by commit R14:1b554d8e7547: Improve ORODocument (authored by piggz). · Explain WhyDec 5 2016, 10:13 PM

This revision was automatically updated to reflect the committed changes.

staniek added inline comments.Dec 9 2016, 5:15 PM

src/plugins/CMakeLists.txt
1	Do you remember the reason?

Yes, it was becuase we un-exported the classes needed by plugins. We can re-enable it, and I will also create a task to create a new barcode plugin based on some other library.

			Path	Packages
M			src/common/KReportItemLine.cpp (2 lines)
M			src/common/KReportRenderObjects.h (513 lines)
M			src/common/KReportRenderObjects.cpp (796 lines)
M			src/items/check/KReportItemCheck.cpp (16 lines)
M			src/items/field/KReportItemField.cpp (4 lines)
M			src/items/image/KReportItemImage.cpp (2 lines)
M			src/items/label/KReportItemLabel.cpp (2 lines)
M			src/items/text/KReportItemText.cpp (4 lines)
M			src/plugins/CMakeLists.txt (2 lines)
M			src/plugins/barcode/3of9.cpp (2 lines)
M			src/plugins/barcode/code128.cpp (4 lines)
M			src/plugins/barcode/codeean.cpp (84 lines)
M			src/plugins/barcode/i2of5.cpp (2 lines)
M			src/plugins/maps/KReportItemMaps.cpp (2 lines)
M			src/plugins/web/KReportItemWeb.cpp (2 lines)
M			src/renderer/KReportHTMLCSSRenderer_p.cpp (22 lines)
M			src/renderer/KReportHTMLTableRenderer_p.cpp (20 lines)
M			src/renderer/KReportKSpreadRenderer.cpp (8 lines)
M			src/renderer/KReportPreRenderer.cpp (12 lines)
M			src/renderer/KReportPrintRenderer_p.cpp (40 lines)
M			src/renderer/KReportScreenRenderer_p.cpp (24 lines)
M			src/renderer/KReportView.cpp (2 lines)
M			src/renderer/odtframe/KoOdtFrameReportCheckBox.h (6 lines)
M			src/renderer/odtframe/KoOdtFrameReportCheckBox.cpp (8 lines)
M			src/renderer/odtframe/KoOdtFrameReportPrimitive.cpp (2 lines)
M			src/renderer/scripting/KReportScriptDraw.cpp (8 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	8319	0ee5572		Nov 19 2016, 12:05 AM	★	★
Diff 2	8333	0ee5572	- Implement comments	Nov 20 2016, 8:40 PM	★	★
Diff 3	8420	0ee5572	- Fix order of primitives	Nov 22 2016, 7:21 PM	★	★
Diff 4	8649	0ee5572	- Implement review comments	Nov 30 2016, 10:00 PM	★	★
Diff 5	8650	0ee5572	- 2 missed comments	Nov 30 2016, 10:09 PM	★	★
Diff 6	8712	0ee5572	- Implement remaining TODOs	Dec 2 2016, 10:57 PM	★	★
Diff 7	8751	0ee5572	- Move function to cpp	Dec 4 2016, 10:11 PM	★	★
Diff 8	8794	0ee5572	- Final comment!	Dec 5 2016, 9:39 PM	★	★
Diff 9	8795	1cc2c69	R14:1b554d8e75471dda9b5580c37deddfef69a5994b	Dec 5 2016, 10:13 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
8f7e6edab521	c99a7e418099	48e2dc2144ae	Adam Pigg	Implement remaining TODOs (Show More…)	Dec 2 2016, 10:57 PM
48e2dc2144ae	aa1b7a23c2bc	3f23279bf3a0	Adam Pigg	2 missed comments	Nov 30 2016, 10:08 PM
3f23279bf3a0	d7b9102824ba	c092d6d16230	Adam Pigg	Implement review comments	Nov 30 2016, 9:58 PM
c092d6d16230	f800e88b19fd	298addc9ff6a	Adam Pigg	Fix order of primitives	Nov 22 2016, 7:20 PM
298addc9ff6a	91a1b5935c59	7d175a638152	Adam Pigg	Implement comments (Show More…)	Nov 20 2016, 8:38 PM
7d175a638152	163f2bff5e64	312d76832e14	Adam Pigg	Improve ORODocument (Show More…)	Nov 19 2016, 12:03 AM
312d76832e14	340e731f8a41	160f2c74531b	Adam Pigg	Port OROCheck	Nov 18 2016, 11:39 PM
160f2c74531b	5c3288b98d36	8aaecd4df794	Adam Pigg	Port OPORect and OROEllipse	Nov 18 2016, 11:13 PM
8aaecd4df794	84b88052918c	633aaa8446dc	Adam Pigg	Port OROPicture	Nov 18 2016, 10:59 PM
633aaa8446dc	b5d5ec6e0a6d	33cf87871792	Adam Pigg	Port OROImage	Nov 18 2016, 10:54 PM
33cf87871792	019f8c72ebe0	61a1ddb3acfd	Adam Pigg	Port OROLine	Nov 18 2016, 10:43 PM
61a1ddb3acfd	2455777d46b1	e456e52ee703	Adam Pigg	Port OROTextBox	Nov 18 2016, 10:38 PM
e456e52ee703	4f48087f9bec	a89cfdac17a7	Adam Pigg	Port and improve OROPrimitive	Nov 18 2016, 10:05 PM
a89cfdac17a7	31168c0750fb	de70e31fddb6	Adam Pigg	Port OROSection	Nov 18 2016, 9:31 PM
de70e31fddb6	55b4b5710541	e2db36c12e0b	Adam Pigg	Port OROPage	Nov 18 2016, 9:14 PM
e2db36c12e0b	1644e2bde52b	0ee55729c26e	Adam Pigg	Improve ORODocument	Nov 18 2016, 8:23 PM

Improve ORODocument
ClosedPublic
Actions

Details

Diff Detail

Revision Contents
Changeset List

Diff 8712

src/common/KReportItemLine.cpp

src/common/KReportRenderObjects.h

src/common/KReportRenderObjects.cpp

src/items/check/KReportItemCheck.cpp

src/items/field/KReportItemField.cpp

src/items/image/KReportItemImage.cpp

src/items/label/KReportItemLabel.cpp

src/items/text/KReportItemText.cpp

src/plugins/CMakeLists.txt

src/plugins/barcode/3of9.cpp

src/plugins/barcode/code128.cpp

src/plugins/barcode/codeean.cpp

src/plugins/barcode/i2of5.cpp

src/plugins/maps/KReportItemMaps.cpp

src/plugins/web/KReportItemWeb.cpp

src/renderer/KReportHTMLCSSRenderer_p.cpp

src/renderer/KReportHTMLTableRenderer_p.cpp

src/renderer/KReportKSpreadRenderer.cpp

src/renderer/KReportPreRenderer.cpp

src/renderer/KReportPrintRenderer_p.cpp

src/renderer/KReportScreenRenderer_p.cpp

src/renderer/KReportView.cpp

src/renderer/odtframe/KoOdtFrameReportCheckBox.h

src/renderer/odtframe/KoOdtFrameReportCheckBox.cpp

src/renderer/odtframe/KoOdtFrameReportPrimitive.cpp

src/renderer/scripting/KReportScriptDraw.cpp

Improve ORODocumentClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 8712

src/common/KReportItemLine.cpp

src/common/KReportRenderObjects.h

src/common/KReportRenderObjects.cpp

src/items/check/KReportItemCheck.cpp

src/items/field/KReportItemField.cpp

src/items/image/KReportItemImage.cpp

src/items/label/KReportItemLabel.cpp

src/items/text/KReportItemText.cpp

src/plugins/CMakeLists.txt

src/plugins/barcode/3of9.cpp

src/plugins/barcode/code128.cpp

src/plugins/barcode/codeean.cpp

src/plugins/barcode/i2of5.cpp

src/plugins/maps/KReportItemMaps.cpp

src/plugins/web/KReportItemWeb.cpp

src/renderer/KReportHTMLCSSRenderer_p.cpp

src/renderer/KReportHTMLTableRenderer_p.cpp

src/renderer/KReportKSpreadRenderer.cpp

src/renderer/KReportPreRenderer.cpp

src/renderer/KReportPrintRenderer_p.cpp

src/renderer/KReportScreenRenderer_p.cpp

src/renderer/KReportView.cpp

src/renderer/odtframe/KoOdtFrameReportCheckBox.h

src/renderer/odtframe/KoOdtFrameReportCheckBox.cpp

src/renderer/odtframe/KoOdtFrameReportPrimitive.cpp

src/renderer/scripting/KReportScriptDraw.cpp

Improve ORODocument
ClosedPublic
Actions

Revision Contents
Changeset List