-
Notifications
You must be signed in to change notification settings - Fork 316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
updated handling of repeating groupheaders #112
base: trunk
Are you sure you want to change the base?
updated handling of repeating groupheaders #112
Conversation
Awesome! Thanks for the patch. While reviewing the patch, I noticed that some code has only commented out and not removed. Is there any reason why we want to keep it? see main/reportbuilder/java/com/sun/star/report/pentaho/output/text/TextRawReportTarget.java (patched line: 1245 to 1248) |
…github.com/Grosskopf/openoffice into reportbuilder-repeating-groupheader-update
Thank you for the quick answer :) True, we don't need that. I tried Squashing it but that didn't quite work the way I expected xD is this ok? Also, I committed this patch to Libreoffice too (https://gerrit.libreoffice.org/c/core/+/107780/3), so it would be under Apache License for you and under multiple licenses for TDF right? :) |
If not needed I would would opt that the dead code is removed. :) Can you remove it? or should I check how I can incorporate this wish? We need only the Apache License. If I were you I would add the License to the Comments that the patch is under dual License (Apache License, Mozilla License). But this is me. |
I think I did remove it, is it not gone? this commit history is messy...
should I do this in the code? in the conversations around the patches I mentioned this... Also I just found bugs I propably did in this patch... I tested another report without repeating groupheaders and it's not working anymore, not sure why... so this shouldn't be merged as of now... |
Probably I had some caching issue or tomatos on my eyes. I did not see that you removed the code, but now it looks good.
Okay. No issue. We have time. |
A fix for Bug 108383
Repeating groupheaders were in the page's header while normal groupheaders were in the pages body. So when a normal groupheader was before a repeating groupheader, it would go to body first, write the normal groupheader and then "return" to the next pages header to write the repeating groupheader since that is the next possible header to write to.
In this fix I have changed this, so that a repeating groupheader writes to the page body like the normal groupheader, but also creates a followup pagestyle that has the repeating groupheader in the header.
This was tested mostly in Libreoffice since their bug 51452 is the exact same and the source code is not so different in this area but it was tested once in open office and created similar results.