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
45	clear is not really needed
48	same here
69	Second arg not needed
74	Second arg not needed, default constructed value is returned here, which for Qt is nullptr.
93	Second arg not needed
98	Second arg not needed
146	For symmetry don't we want to also delete page?
151	For symmetry don't we want to also delete section?
178	not really needed
191	not really needed
206	Second arg not needed
211	Second arg not needed
260	For symmetry don't we want to also delete primitive?
275	KReport::recordCount() is qint64 so how about using qint64 here too?
281	Let's initialize all members here because it's possible.
310	Second arg not needed
315	Second arg not needed
410	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)
460	TODO: rename to KReportTextStyleData, add initialization of KReportTextStyleData::alignment and KReportTextStyleData::backgroundOpacity.
472	Let's initialize all this in Private::Private(). Cleaner and less Coverity warnings.
564	can be `clone() const`
573	Missing copying: wordWrap canGrow requiresPostProcessing
623	can be `clone() const`
697	can be `clone() const`
729	can be `clone() const`
799	can be `clone() const`
859	can be `clone() const`
936	can be `clone() const`
src/common/KReportRenderObjects.h
58	+title param.
63	While we're looking here. Add docs informing that ownership is transferred.
65	Add docs informing that the page object will be removed and deleted.
70	Add docs informing that ownership is transferred.
71	Setion -> Section
71	Add docs informing that the section object will be removed and deleted.
124	How about keeping X and Y by using Qt::Orientation and removing Sort?
133	+const
203	Not done...
245	let's add arg name
248	let's add arg name
251	let's add arg name
271	let's add arg name
274	let's add arg name
326	let's add arg name
346	let's add arg name
349	let's add arg name
352	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
573	TODO
944	TODO
src/common/KReportRenderObjects.h
58	TODO
71	TODO
148–149	+param name
203	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
152–155	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 (395 lines)
M			src/common/KReportRenderObjects.cpp (751 lines)
M			src/items/check/KReportItemCheck.cpp (6 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 (6 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 (36 lines)
M			src/renderer/KReportScreenRenderer_p.cpp (20 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
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 8420

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 8420

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