[patch] Proposed bug fix

FormShifter_patch.txt (text/plain), 8.25 KB, created by Thies Wellpott on 2012-08-30 13:40:12 UTC

(hide)

Description: Proposed bug fix

Filename:

MIME Type:

Creator: Thies Wellpott

Created: 2012-08-30 13:40:12 UTC

Size: 8.25 KB

patch

obsolete

>Index: FormulaShifter.java >=================================================================== >--- FormulaShifter.java (revision 1368395) >+++ FormulaShifter.java (working copy) >@@ -17,11 +17,23 @@ > > package org.apache.poi.ss.formula; > >-import org.apache.poi.ss.formula.ptg.*; >+import org.apache.poi.ss.formula.ptg.Area2DPtgBase; >+import org.apache.poi.ss.formula.ptg.Area3DPtg; >+import org.apache.poi.ss.formula.ptg.AreaErrPtg; >+import org.apache.poi.ss.formula.ptg.AreaPtg; >+import org.apache.poi.ss.formula.ptg.AreaPtgBase; >+import org.apache.poi.ss.formula.ptg.DeletedArea3DPtg; >+import org.apache.poi.ss.formula.ptg.DeletedRef3DPtg; >+import org.apache.poi.ss.formula.ptg.Ptg; >+import org.apache.poi.ss.formula.ptg.Ref3DPtg; >+import org.apache.poi.ss.formula.ptg.RefErrorPtg; >+import org.apache.poi.ss.formula.ptg.RefPtg; >+import org.apache.poi.ss.formula.ptg.RefPtgBase; > > > /** > * @author Josh Micich >+ * @author Thies Wellpott (only bug fix) > */ > public final class FormulaShifter { > >@@ -67,7 +79,7 @@ > /** > * Create an instance for shifting sheets. > * >- * For example, this will be called on {@link org.apache.poi.hssf.usermodel.HSSFWorkbook#setSheetOrder(String, int)} >+ * For example, this will be called on {@link org.apache.poi.hssf.usermodel.HSSFWorkbook#setSheetOrder(String, int)} > */ > private FormulaShifter(int srcSheetIndex, int dstSheetIndex) { > _externSheetIndex = _firstMovedIndex = _lastMovedIndex = _amountToMove = -1; >@@ -85,6 +97,7 @@ > return new FormulaShifter(srcSheetIndex, dstSheetIndex); > } > >+ @Override > public String toString() { > StringBuffer sb = new StringBuffer(); > >@@ -179,6 +192,14 @@ > } > > private Ptg rowMoveRefPtg(RefPtgBase rptg) { >+ if (!rptg.isRowRelative()) { >+ // absolute references will not be updated >+ // TODO Formula needs recalculation! >+ // idea: use fr.setCalcOnLoad(true) inside ValueRecordsAggregate.updateFormulasAfterRowShift() >+ // because Workbook.setForceFormulaRecalculation(true) is a bit too much >+ return null; >+ } >+ > int refRow = rptg.getRow(); > if (_firstMovedIndex <= refRow && refRow <= _lastMovedIndex) { > // Rows being moved completely enclose the ref. >@@ -208,13 +229,22 @@ > } > > private Ptg rowMoveAreaPtg(AreaPtgBase aptg) { >+ if (! (aptg.isFirstRowRelative() || aptg.isLastRowRelative())) { >+ // absolute references will not be updated >+ return null; >+ } >+ > int aFirstRow = aptg.getFirstRow(); > int aLastRow = aptg.getLastRow(); > if (_firstMovedIndex <= aFirstRow && aLastRow <= _lastMovedIndex) { > // Rows being moved completely enclose the area ref. > // - move the area ref along with the rows regardless of destination >- aptg.setFirstRow(aFirstRow + _amountToMove); >- aptg.setLastRow(aLastRow + _amountToMove); >+ if (aptg.isFirstRowRelative()) { >+ aptg.setFirstRow(aFirstRow + _amountToMove); >+ } >+ if (aptg.isLastRowRelative()) { >+ aptg.setLastRow(aLastRow + _amountToMove); >+ } > return aptg; > } > // else rules for adjusting area may also depend on the destination of the moved rows >@@ -227,11 +257,11 @@ > > // If the destination of the rows overlaps either the top > // or bottom of the area ref there will be a change >- if (destFirstRowIndex < aFirstRow && aFirstRow <= destLastRowIndex) { >+ if (aptg.isFirstRowRelative() && destFirstRowIndex < aFirstRow && aFirstRow <= destLastRowIndex) { > // truncate the top of the area by the moved rows > aptg.setFirstRow(destLastRowIndex+1); > return aptg; >- } else if (destFirstRowIndex <= aLastRow && aLastRow < destLastRowIndex) { >+ } else if (aptg.isLastRowRelative() && destFirstRowIndex <= aLastRow && aLastRow < destLastRowIndex) { > // truncate the bottom of the area by the moved rows > aptg.setLastRow(destFirstRowIndex-1); > return aptg; >@@ -244,9 +274,13 @@ > // Rows moved include the first row of the area ref, but not the last row > // btw: (aLastRow > _lastMovedIndex) > if (_amountToMove < 0) { >- // simple case - expand area by shifting top upward >- aptg.setFirstRow(aFirstRow + _amountToMove); >- return aptg; >+ if (aptg.isFirstRowRelative()) { >+ // simple case - expand area by shifting top upward >+ aptg.setFirstRow(aFirstRow + _amountToMove); >+ return aptg; >+ } else { >+ return null; >+ } > } > if (destFirstRowIndex > aLastRow) { > // in this case, excel ignores the row move >@@ -254,10 +288,14 @@ > } > int newFirstRowIx = aFirstRow + _amountToMove; > if (destLastRowIndex < aLastRow) { >- // end of area is preserved (will remain exact same row) >- // the top area row is moved simply >- aptg.setFirstRow(newFirstRowIx); >- return aptg; >+ if (aptg.isFirstRowRelative()) { >+ // end of area is preserved (will remain exact same row) >+ // the top area row is moved simply >+ aptg.setFirstRow(newFirstRowIx); >+ return aptg; >+ } else { >+ return null; >+ } > } > // else - bottom area row has been replaced - both area top and bottom may move now > int areaRemainingTopRowIx = _lastMovedIndex + 1; >@@ -265,17 +303,25 @@ > // old top row of area has moved deep within the area, and exposed a new top row > newFirstRowIx = areaRemainingTopRowIx; > } >- aptg.setFirstRow(newFirstRowIx); >- aptg.setLastRow(Math.max(aLastRow, destLastRowIndex)); >+ if (aptg.isFirstRowRelative()) { >+ aptg.setFirstRow(newFirstRowIx); >+ } >+ if (aptg.isLastRowRelative()) { >+ aptg.setLastRow(Math.max(aLastRow, destLastRowIndex)); >+ } > return aptg; > } > if (_firstMovedIndex <= aLastRow && aLastRow <= _lastMovedIndex) { > // Rows moved include the last row of the area ref, but not the first > // btw: (aFirstRow < _firstMovedIndex) > if (_amountToMove > 0) { >- // simple case - expand area by shifting bottom downward >- aptg.setLastRow(aLastRow + _amountToMove); >- return aptg; >+ if (aptg.isLastRowRelative()) { >+ // simple case - expand area by shifting bottom downward >+ aptg.setLastRow(aLastRow + _amountToMove); >+ return aptg; >+ } else { >+ return null; >+ } > } > if (destLastRowIndex < aFirstRow) { > // in this case, excel ignores the row move >@@ -283,10 +329,14 @@ > } > int newLastRowIx = aLastRow + _amountToMove; > if (destFirstRowIndex > aFirstRow) { >- // top of area is preserved (will remain exact same row) >- // the bottom area row is moved simply >- aptg.setLastRow(newLastRowIx); >- return aptg; >+ if (aptg.isLastRowRelative()) { >+ // top of area is preserved (will remain exact same row) >+ // the bottom area row is moved simply >+ aptg.setLastRow(newLastRowIx); >+ return aptg; >+ } else { >+ return null; >+ } > } > // else - top area row has been replaced - both area top and bottom may move now > int areaRemainingBottomRowIx = _firstMovedIndex - 1; >@@ -294,8 +344,12 @@ > // old bottom row of area has moved up deep within the area, and exposed a new bottom row > newLastRowIx = areaRemainingBottomRowIx; > } >- aptg.setFirstRow(Math.min(aFirstRow, destFirstRowIndex)); >- aptg.setLastRow(newLastRowIx); >+ if (aptg.isFirstRowRelative()) { >+ aptg.setFirstRow(Math.min(aFirstRow, destFirstRowIndex)); >+ } >+ if (aptg.isLastRowRelative()) { >+ aptg.setLastRow(newLastRowIx); >+ } > return aptg; > } > // else source rows include none of the rows of the area ref >@@ -317,16 +371,24 @@ > } > > if (destFirstRowIndex < aFirstRow && aFirstRow <= destLastRowIndex) { >- // dest rows overlap top of area >- // - truncate the top >- aptg.setFirstRow(destLastRowIndex+1); >- return aptg; >+ if (aptg.isFirstRowRelative()) { >+ // dest rows overlap top of area >+ // - truncate the top >+ aptg.setFirstRow(destLastRowIndex+1); >+ return aptg; >+ } else { >+ return null; >+ } > } > if (destFirstRowIndex < aLastRow && aLastRow <= destLastRowIndex) { >- // dest rows overlap bottom of area >- // - truncate the bottom >- aptg.setLastRow(destFirstRowIndex-1); >- return aptg; >+ if (aptg.isLastRowRelative()) { >+ // dest rows overlap bottom of area >+ // - truncate the bottom >+ aptg.setLastRow(destFirstRowIndex-1); >+ return aptg; >+ } else { >+ return null; >+ } > } > throw new IllegalStateException("Situation not covered: (" + _firstMovedIndex + ", " + > _lastMovedIndex + ", " + _amountToMove + ", " + aFirstRow + ", " + aLastRow + ")"); View Attachment As Diff View Attachment As Raw

This is ASF Bugzilla: the Apache Software Foundation bug system. In case of problems with the functioning of ASF Bugzilla, please contact bugzilla-admin@apache.org. Please Note: this e-mail address is only for reporting problems with ASF Bugzilla. Mail about any other subject will be silently ignored.

• • Home
  • | New
  • | Browse
  • | Search
  • |
    [?]
  • | Reports
  • | Help
  • | New Account
  • | Log In
    Remember [x]
  • | Forgot Password
    Login: [x]