You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by "jlolling (via GitHub)" <gi...@apache.org> on 2023/10/09 11:38:49 UTC
[PR] Allow append rows in Streaming workbook [poi]
jlolling opened a new pull request, #528:
URL: https://github.com/apache/poi/pull/528
According to the ticket: https://bz.apache.org/bugzilla/show_bug.cgi?id=67646
Here the pull request to solve the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: [PR] Allow append rows in Streaming workbook [poi]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #528:
URL: https://github.com/apache/poi/pull/528#discussion_r1350196110
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -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);
Review Comment:
please do not randomly change code like this- undo this change or the PR is rejected
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: [PR] Allow append rows in Streaming workbook [poi]
Posted by "jlolling (via GitHub)" <gi...@apache.org>.
jlolling commented on PR #528:
URL: https://github.com/apache/poi/pull/528#issuecomment-1752859725
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 <https://github.com/apache/poi/pull/528#discussion_r1350196110>:
>
> > @@ -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 <https://github.com/apache/poi/pull/528#pullrequestreview-1664268560>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5KS7P7KN4QKA6Y6X6HEB3X6PPR7AVCNFSM6AAAAAA5YVCZFSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMNRUGI3DQNJWGA>.
> You are receiving this because you modified the open/close state.
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: [PR] Allow append rows in Streaming workbook [poi]
Posted by "jlolling (via GitHub)" <gi...@apache.org>.
jlolling commented on PR #528:
URL: https://github.com/apache/poi/pull/528#issuecomment-1752856333
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 <https://github.com/apache/poi/pull/528#discussion_r1350192943>:
>
> > @@ -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 <https://github.com/apache/poi/pull/528#discussion_r1350194065>:
>
> > }
>
> /**
> - * 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 <https://github.com/apache/poi/pull/528#discussion_r1350194317>:
>
> > @@ -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 <https://github.com/apache/poi/pull/528#discussion_r1350194811>:
>
> > */
> @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 <https://github.com/apache/poi/pull/528#pullrequestreview-1664263448>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5KS7OWFL2WUVNSV4PJURLX6PPPBAVCNFSM6AAAAAA5YVCZFSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMNRUGI3DGNBUHA>.
> You are receiving this because you modified the open/close state.
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: [PR] Allow append rows in Streaming workbook [poi]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #528:
URL: https://github.com/apache/poi/pull/528#discussion_r1350194065
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1240,11 +1247,11 @@ public void setColumnGroupCollapsed(int columnNumber, boolean collapsed) {
*/
@Override
public void groupColumn(int fromColumn, int toColumn) {
- _sh.groupColumn(fromColumn, toColumn);
+ _sh.groupColumn(fromColumn,toColumn);
}
/**
- * Ungroup a range of columns that were previously grouped
+ * Ungroup a range of columns that were previously groupped
Review Comment:
another typo - please do not give us PRs where you randomly change code unrelated to your fix
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1145,7 +1152,7 @@ public boolean isDisplayRowColHeadings() {
* Breaks occur above the specified row and left of the specified column inclusive.
*
* 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);}
Review Comment:
Can you fix the typo here?
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1349,33 +1361,33 @@ public void ungroupRow(int fromRow, int toRow) {
*
* <i>Not implemented for expanding (i.e. collapse == false)</i>
*
- * @param row start row of a grouped range of rows (0-based)
+ * @param row start row of a groupped range of rows (0-based)
* @param collapse whether to expand/collapse the detail rows
- * @throws IllegalStateException if collapse is false as this is not implemented for SXSSF.
+ * @throws RuntimeException if collapse is false as this is not implemented for SXSSF.
*/
@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");
Review Comment:
please do not change the exception type
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1240,11 +1247,11 @@ public void setColumnGroupCollapsed(int columnNumber, boolean collapsed) {
*/
@Override
public void groupColumn(int fromColumn, int toColumn) {
- _sh.groupColumn(fromColumn, toColumn);
Review Comment:
why are you reformatting this code?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: [PR] Allow append rows in Streaming workbook [poi]
Posted by "jlolling (via GitHub)" <gi...@apache.org>.
jlolling commented on PR #528:
URL: https://github.com/apache/poi/pull/528#issuecomment-1752888018
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 <https://github.com/apache/poi/pull/528#discussion_r1350219068>:
>
> > @@ -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 <https://github.com/apache/poi/pull/528#pullrequestreview-1664305048>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5KS7OVGRS45UHBJ7OJ7ALX6PSETAVCNFSM6AAAAAA5YVCZFSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMNRUGMYDKMBUHA>.
> You are receiving this because you modified the open/close state.
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: [PR] Allow append rows in Streaming workbook [poi]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #528:
URL: https://github.com/apache/poi/pull/528#discussion_r1350219068
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -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
Review Comment:
In the ASF, we believe in shared ownership of the code. Please do not annotate code with your name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: [PR] Allow append rows in Streaming workbook [poi]
Posted by "jlolling (via GitHub)" <gi...@apache.org>.
jlolling closed pull request #528: Allow append rows in Streaming workbook
URL: https://github.com/apache/poi/pull/528
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org