Allow append rows in Streaming workbook#528
Closed
jlolling wants to merge 1 commit intoapache:trunkfrom
Closed
Conversation
pjfanning
requested changes
Oct 9, 2023
| * | ||
| * For example, {@code sheet.setColumnBreak(2);} breaks the sheet into two parts | ||
| * with columns A,B,C in the first and D,E,... in the second. Similar, {@code sheet.setRowBreak(2);} | ||
| * with columns A,B,C in the first and D,E,... in the second. Simuilar, {@code sheet.setRowBreak(2);} |
|
|
||
| /** | ||
| * Ungroup a range of columns that were previously grouped | ||
| * Ungroup a range of columns that were previously groupped |
Member
There was a problem hiding this comment.
another typo - please do not give us PRs where you randomly change code unrelated to your fix
| */ | ||
| @Override | ||
| public void groupColumn(int fromColumn, int toColumn) { | ||
| _sh.groupColumn(fromColumn, toColumn); |
Member
There was a problem hiding this comment.
why are you reformatting this code?
| } else { | ||
| //expandRow(rowIndex); | ||
| throw new IllegalStateException("Unable to expand row: Not Implemented"); | ||
| throw new RuntimeException("Unable to expand row: Not Implemented"); |
Member
There was a problem hiding this comment.
please do not change the exception type
pjfanning
reviewed
Oct 9, 2023
| } catch (UnsatisfiedLinkError | InternalError e) { | ||
| LOG.atWarn().log("Failed to create AutoSizeColumnTracker, possibly due to fonts not being installed in your OS", e); | ||
| } | ||
| _autoSizeColumnTracker = new AutoSizeColumnTracker(this); |
Member
There was a problem hiding this comment.
please do not randomly change code like this- undo this change or the PR is rejected
Author
|
I will try to add a test case
I will also fix the typos but they are not caused by me.
… On 9. Oct 2023, at 13:43, PJ Fanning ***@***.***> wrote:
@pjfanning requested changes on this pull request.
Can you add a test case?
In poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java <#528 (comment)>:
> @@ -1145,7 +1152,7 @@ public boolean isDisplayRowColHeadings() {
* Breaks occur above the specified row and left of the specified column inclusive.
*
* For example, ***@***.*** sheet.setColumnBreak(2);} breaks the sheet into two parts
- * with columns A,B,C in the first and D,E,... in the second. Similar, ***@***.*** sheet.setRowBreak(2);}
+ * with columns A,B,C in the first and D,E,... in the second. Simuilar, ***@***.*** sheet.setRowBreak(2);}
Can you fix the typo here?
In poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java <#528 (comment)>:
> }
/**
- * Ungroup a range of columns that were previously grouped
+ * Ungroup a range of columns that were previously groupped
another typo - please do not give us PRs where you randomly change code unrelated to your fix
In poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java <#528 (comment)>:
> @@ -1240,11 +1247,11 @@ public void setColumnGroupCollapsed(int columnNumber, boolean collapsed) {
*/
@OverRide
public void groupColumn(int fromColumn, int toColumn) {
- _sh.groupColumn(fromColumn, toColumn);
why are you reformatting this code?
In poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java <#528 (comment)>:
> */
@OverRide
public void setRowGroupCollapsed(int row, boolean collapse) {
if (collapse) {
collapseRow(row);
} else {
//expandRow(rowIndex);
- throw new IllegalStateException("Unable to expand row: Not Implemented");
+ throw new RuntimeException("Unable to expand row: Not Implemented");
please do not change the exception type
—
Reply to this email directly, view it on GitHub <#528 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5KS7OWFL2WUVNSV4PJURLX6PPPBAVCNFSM6AAAAAA5YVCZFSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMNRUGI3DGNBUHA>.
You are receiving this because you modified the open/close state.
|
Author
|
I have not “randomly” changed code. I have started from a code some weeks earlier. I will rebase my branch and continue.
… On 9. Oct 2023, at 13:44, PJ Fanning ***@***.***> wrote:
@pjfanning commented on this pull request.
In poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java <#528 (comment)>:
> @@ -61,11 +62,7 @@ protected SXSSFSheet(SXSSFWorkbook workbook, XSSFSheet xSheet, int randomAccessW
_sh = xSheet;
calculateLeftAndRightMostColumns(xSheet);
setRandomAccessWindowSize(randomAccessWindowSize);
- try {
- _autoSizeColumnTracker = new AutoSizeColumnTracker(this);
- } catch (UnsatisfiedLinkError | InternalError e) {
- LOG.atWarn().log("Failed to create AutoSizeColumnTracker, possibly due to fonts not being installed in your OS", e);
- }
+ _autoSizeColumnTracker = new AutoSizeColumnTracker(this);
please do not randomly change code like this- undo this change or the PR is rejected
—
Reply to this email directly, view it on GitHub <#528 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5KS7P7KN4QKA6Y6X6HEB3X6PPR7AVCNFSM6AAAAAA5YVCZFSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMNRUGI3DQNJWGA>.
You are receiving this because you modified the open/close state.
|
pjfanning
reviewed
Oct 9, 2023
| return _rows.get(rownum); | ||
| public Row getRow(int rownum) { | ||
| Row row = _rows.get(rownum); | ||
| // jlolling: allow reading all the content |
Member
There was a problem hiding this comment.
In the ASF, we believe in shared ownership of the code. Please do not annotate code with your name.
Author
|
I will remove my name and set the ticket number instead. OK?
… On 9. Oct 2023, at 14:06, PJ Fanning ***@***.***> wrote:
@pjfanning commented on this pull request.
In poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java <#528 (comment)>:
> @@ -191,8 +192,13 @@ public void removeRow(Row row) {
* @return Row representing the rownumber or null if its not defined on the sheet
*/
@OverRide
- public SXSSFRow getRow(int rownum) {
- return _rows.get(rownum);
+ public Row getRow(int rownum) {
+ Row row = _rows.get(rownum);
+ // jlolling: allow reading all the content
In the ASF, we believe in shared ownership of the code. Please do not annotate code with your name.
—
Reply to this email directly, view it on GitHub <#528 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5KS7OVGRS45UHBJ7OJ7ALX6PSETAVCNFSM6AAAAAA5YVCZFSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMNRUGMYDKMBUHA>.
You are receiving this because you modified the open/close state.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
According to the ticket: https://bz.apache.org/bugzilla/show_bug.cgi?id=67646
Here the pull request to solve the issue.