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
63	clear is not really needed
66	same here
77	Second arg not needed
82	Second arg not needed, default constructed value is returned here, which for Qt is nullptr.
96–101	Second arg not needed
106	Second arg not needed
149	For symmetry don't we want to also delete page?
154	For symmetry don't we want to also delete section?
189	not really needed
205	not really needed
224	Second arg not needed
229	Second arg not needed
273	For symmetry don't we want to also delete primitive?
293	KReport::recordCount() is qint64 so how about using qint64 here too?
299	Let's initialize all members here because it's possible.
327	Second arg not needed
332	Second arg not needed
422	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)
471	TODO: rename to KReportTextStyleData, add initialization of KReportTextStyleData::alignment and KReportTextStyleData::backgroundOpacity.
485–493	Let's initialize all this in Private::Private(). Cleaner and less Coverity warnings.
559–584	can be `clone() const`
596	Missing copying: wordWrap canGrow requiresPostProcessing
646	can be `clone() const`
720	can be `clone() const`
752	can be `clone() const`
822	can be `clone() const`
877–882	can be `clone() const`
959	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
272–273	Not done...
315	let's add arg name
317–318	let's add arg name
320–321	let's add arg name
340–341	let's add arg name
342–343	let's add arg name
395–396	let's add arg name
415–416	let's add arg name
417–418	let's add arg name
418–419	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
596	TODO
968	TODO
src/common/KReportRenderObjects.h
56–57	TODO
68	TODO
216–217	+param name
272–273	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–225	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 (801 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
8b985e76bbb3	8976c4f17330	8f7e6edab521	Adam Pigg	Move function to cpp	Dec 4 2016, 10:11 PM
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 8751

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 8751

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