Remove KReportPageOptions
ClosedPublic

Authored by piggz on Nov 9 2016, 8:50 PM.

Details

Summary

Minor, non intrusive change

Diff Detail

Repository
R14 KReport
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
piggz updated this revision to Diff 8054.Nov 9 2016, 8:50 PM
piggz retitled this revision from to Add d-ptr to KReportPageOptions.
piggz updated this object.
piggz edited the test plan for this revision. (Show Details)
piggz added a reviewer: staniek.
Restricted Application added a project: KReport. · View Herald TranscriptNov 9 2016, 8:50 PM
staniek requested changes to this revision.Nov 9 2016, 9:16 PM
staniek edited edge metadata.
staniek added inline comments.
src/common/KReportPageOptions.h
0

Can't we go for QPageLayout?

This revision now requires changes to proceed.Nov 9 2016, 9:16 PM
staniek added inline comments.Nov 9 2016, 9:17 PM
src/common/KReportPageOptions.h
0

BTW KReportDesign already uses it.

piggz updated this revision to Diff 8103.Nov 11 2016, 10:00 PM
piggz edited edge metadata.
  • Removal of KReportPageOptions
  • Rename pageOptions > pageLayout
piggz retitled this revision from Add d-ptr to KReportPageOptions to Remove KReportPageOptions.Nov 12 2016, 9:25 PM
piggz edited edge metadata.
staniek requested changes to this revision.Nov 14 2016, 9:18 PM
staniek edited edge metadata.

Nice!

src/common/KReportDocument.cpp
48

Will be moved later to dptr, right?

107–112

--8X--

src/common/KReportDocument.h
98

+const

98

If it's unused shouldn't we add #if 0 around this getter and setter? We don't know exact API we'll need for label types...

src/wrtembed/KReportDesigner.cpp
868–871

Better no abbreviations like this...

This revision now requires changes to proceed.Nov 14 2016, 9:18 PM

Build and run the test app and Kexi.

Only for the map element: ASSERT: "m_topLevel <= m_bottomLevel" in file src/marble/src/lib/marble/TileCoordsPyramid.cpp, line 37
No idea if that's related, not tried maps for a while.

piggz marked 4 inline comments as done.Nov 15 2016, 7:58 PM
piggz added inline comments.
src/common/KReportDocument.cpp
48

Ok, i'll do that in a separate commit

src/common/KReportDocument.h
98

i agree, however, the parser has always supported loading the label info from the XML, we just dont expose it, so I think we need somewhere to load to, even though we dont expose it yet. The loading code is ported from the original OpenRPT, and the label definitions are also from there.

What do u think?

piggz updated this revision to Diff 8181.Nov 15 2016, 8:26 PM
piggz edited edge metadata.
piggz marked an inline comment as done.
  • Implement comments and fix margin bug
staniek requested changes to this revision.Nov 15 2016, 10:04 PM
staniek edited edge metadata.

Thanks, almost there!

src/common/KReportDocument.cpp
91

--8x--

src/common/KReportDocument.h
18–19

While we're at it:

-> KREPORTDOCUMENT_H

98

I see KReportPreRendererPrivate::generateDocument() uses labelType(). Because KReportPreRendererPrivate is a friend of KReportDocument, would it be possible to make labelType/setLabelType private for now?
With a //! @todo add support for labels?

This revision now requires changes to proceed.Nov 15 2016, 10:04 PM
piggz marked 5 inline comments as done.Nov 15 2016, 10:15 PM
piggz updated this revision to Diff 8183.Nov 15 2016, 10:18 PM
piggz edited edge metadata.
  • Implement comments.
staniek accepted this revision.Nov 15 2016, 10:28 PM
staniek edited edge metadata.

1 line to remove and OK!

src/common/KReportDocument.cpp
107–112

please remove the !Page Layout====

This revision is now accepted and ready to land.Nov 15 2016, 10:28 PM
This revision was automatically updated to reflect the committed changes.