xzel23 commented on code in PR #600:
URL: https://github.com/apache/poi/pull/600#discussion_r1502753906
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -166,11 +167,11 @@ public SXSSFRow createRow(int rownum) {
"in the range [0," + _writer.getLastFlushedRow() +
"] that is already written to disk.");
}
- // attempt to overwrite an existing row in the input template
+ // attempt to overwrite a existing row in the input template
Review Comment:
Why was the comment changed? "An existing" is correct.
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1798,7 +1814,7 @@ public CellRange<? extends Cell> removeArrayFormula(Cell
cell) {
// corrupted .xlsx files as rows appear multiple times in the
resulting sheetX.xml files
// return _sh.removeArrayFormula(cell);
- throw new IllegalStateException("Not Implemented");
+ throw new RuntimeException("Not Implemented");
Review Comment:
same as above
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1169,7 +1180,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:
please revert the typo
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1344,21 +1357,24 @@ public void groupRow(int fromRow, int toRow) {
public void setRowOutlineLevel(int rownum, int level) {
SXSSFRow row = _rows.get(rownum);
row.setOutlineLevel(level);
- setWorksheetOutlineLevelRowIfNecessary((short)
Math.min(Short.MAX_VALUE, level));
+ if(level > 0 && level > outlineLevelRow) {
+ outlineLevelRow = level;
+ setWorksheetOutlineLevelRow();
+ }
}
- private void setWorksheetOutlineLevelRowIfNecessary(final short levelRow) {
+ private void setWorksheetOutlineLevelRow() {
CTWorksheet ct = _sh.getCTWorksheet();
CTSheetFormatPr pr = ct.isSetSheetFormatPr() ?
ct.getSheetFormatPr() :
ct.addNewSheetFormatPr();
- if(levelRow > _sh.getSheetFormatPrOutlineLevelRow()) {
- pr.setOutlineLevelRow(levelRow);
+ if(outlineLevelRow > 0) {
+ pr.setOutlineLevelRow((short)outlineLevelRow);
}
}
/**
- * Ungroup a range of rows that were previously grouped
+ * Ungroup a range of rows that were previously groupped
Review Comment:
please revert this line
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -215,8 +220,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) {
Review Comment:
This might break user code.
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1032,7 +1043,7 @@ public void shiftRows(int startRow, int endRow, int n) {
@NotImplemented
@Override
public void shiftRows(int startRow, int endRow, int n, boolean
copyRowHeight, boolean resetOriginalRowHeight) {
- throw new IllegalStateException("Not Implemented");
+ throw new RuntimeException("Not Implemented");
Review Comment:
UnsupportedOperationException
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -205,6 +206,10 @@ public void removeRow(Row row) {
return;
}
}
+ // BugZilla 67646: allow reading all the content
+ if (row.getSheet() == _sh) {
+ _sh.removeRow(row);
+ }
Review Comment:
I don't understand the intent here. row.getSheet() is checked right at the
start of the method. And the row is removed in the loop. AFAIK the row is not
updated when that happens, so row.getSheet() will still return `this`. So what
should happen here?
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1783,7 +1799,7 @@ public CellRange<? extends Cell> setArrayFormula(String
formula, CellRangeAddres
// corrupted .xlsx files as rows appear multiple times in the
resulting sheetX.xml files
// return _sh.setArrayFormula(formula, range);
- throw new IllegalStateException("Not Implemented");
+ throw new RuntimeException("Not Implemented");
Review Comment:
This should rather be UnsopportedOperationException. IllegalStateException
is not optimal, but we should never just throw RuntimeException.
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1008,7 +1019,7 @@ public boolean getForceFormulaRecalculation() {
@NotImplemented
@Override
public void shiftRows(int startRow, int endRow, int n) {
- throw new IllegalStateException("Not Implemented");
+ throw new RuntimeException("Not Implemented");
Review Comment:
UnsupportedOperationException
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1264,11 +1275,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:
please revert this line
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1373,33 +1389,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)
Review Comment:
please revert this line
##########
poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFSheet.java:
##########
@@ -1373,33 +1389,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:
UnsupportedOperationException
--
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-unsubscr...@poi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org
