svn diff of 3 changed files

patch44609-diff.txt (text/plain), 11.00 KB, created by Josh Micich on 2008-03-14 14:49:40 UTC

(hide)

Description: svn diff of 3 changed files

Filename:

MIME Type:

Creator: Josh Micich

Created: 2008-03-14 14:49:40 UTC

Size: 11.00 KB

patch

obsolete

>Index: C:\josh\client\poi\svn\trunk/src/java/org/apache/poi/hssf/model/FormulaParser.java >=================================================================== >--- C:\josh\client\poi\svn\trunk/src/java/org/apache/poi/hssf/model/FormulaParser.java (revision 637139) >+++ C:\josh\client\poi\svn\trunk/src/java/org/apache/poi/hssf/model/FormulaParser.java (working copy) >@@ -943,23 +943,7 @@ > } > Stack stack = new Stack(); > >- // Excel allows to have AttrPtg at position 0 (such as Blanks) which >- // do not have any operands. Skip them. >- int i; >- if(ptgs[0] instanceof AttrPtg) { >- AttrPtg attrPtg0 = (AttrPtg) ptgs[0]; >- if(attrPtg0.isSemiVolatile()) { >- // no visible formula for semi-volatile >- } else { >- // TODO -this requirement is unclear and is not addressed by any junits >- stack.push(ptgs[0].toFormulaString(book)); >- } >- i=1; >- } else { >- i=0; >- } >- >- for ( ; i < ptgs.length; i++) { >+ for (int i=0 ; i < ptgs.length; i++) { > Ptg ptg = ptgs[i]; > // TODO - what about MemNoMemPtg? > if(ptg instanceof MemAreaPtg || ptg instanceof MemFuncPtg || ptg instanceof MemErrPtg) { >@@ -973,21 +957,30 @@ > continue; > } > >- if (ptg instanceof AttrPtg && ((AttrPtg) ptg).isOptimizedIf()) { >- continue; >+ if (ptg instanceof AttrPtg) { >+ AttrPtg attrPtg = ((AttrPtg) ptg); >+ if (attrPtg.isOptimizedIf()) { >+ continue; >+ } >+ if (attrPtg.isSpace()) { >+ // POI currently doesn't render spaces in formulas >+ continue; >+ // but if it ever did, care must be taken: >+ // tAttrSpace comes *before* the operand it applies to, which may be consistent >+ // with how the formula text appears but is against the RPN ordering assumed here >+ } > } > > final OperationPtg o = (OperationPtg) ptg; > int nOperands = o.getNumberOfOperands(); > final String[] operands = new String[nOperands]; > >- for (int j = nOperands-1; j >= 0; j--) { >+ for (int j = nOperands-1; j >= 0; j--) { // reverse iteration because args were pushed in-order > if(stack.isEmpty()) { >- //TODO: write junit to prove this works > String msg = "Too few arguments suppled to operation token (" > + o.getClass().getName() + "). Expected (" + nOperands >- + " but got " + (nOperands - j + 1); >- throw new FormulaParseException(msg); >+ + ") operands but got (" + (nOperands - j + 1) + ")"; >+ throw new IllegalStateException(msg); > } > operands[j] = (String) stack.pop(); > } >Index: C:\josh\client\poi\svn\trunk/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java >=================================================================== >--- C:\josh\client\poi\svn\trunk/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java (revision 637139) >+++ C:\josh\client\poi\svn\trunk/src/java/org/apache/poi/hssf/record/formula/AttrPtg.java (working copy) >@@ -33,20 +33,42 @@ > * @author Jason Height (jheight at chariot dot net dot au) > */ > >-public class AttrPtg >- extends OperationPtg >-{ >+public final class AttrPtg extends OperationPtg { > public final static byte sid = 0x19; > private final static int SIZE = 4; > private byte field_1_options; > private short field_2_data; >- private BitField semiVolatile = BitFieldFactory.getInstance(0x01); >- private BitField optiIf = BitFieldFactory.getInstance(0x02); >- private BitField optiChoose = BitFieldFactory.getInstance(0x04); >- private BitField optGoto = BitFieldFactory.getInstance(0x08); >- private BitField sum = BitFieldFactory.getInstance(0x10); >- private BitField baxcel = BitFieldFactory.getInstance(0x20); >- private BitField space = BitFieldFactory.getInstance(0x40); >+ >+ // flags 'volatile' and 'space', can be combined. >+ // OOO spec says other combinations are theoretically possible but not likely to occur. >+ private static final BitField semiVolatile = BitFieldFactory.getInstance(0x01); >+ private static final BitField optiIf = BitFieldFactory.getInstance(0x02); >+ private static final BitField optiChoose = BitFieldFactory.getInstance(0x04); >+ private static final BitField optGoto = BitFieldFactory.getInstance(0x08); // skip >+ private static final BitField sum = BitFieldFactory.getInstance(0x10); >+ private static final BitField baxcel = BitFieldFactory.getInstance(0x20); // 'assignment-style formula in a macro sheet' >+ private static final BitField space = BitFieldFactory.getInstance(0x40); >+ >+ public static final class SpaceType { >+ private SpaceType() { >+ // no instances of this class >+ } >+ >+ /** 00H = Spaces before the next token (not allowed before tParen token) */ >+ public static final int SPACE_BEFORE = 0x00; >+ /** 01H = Carriage returns before the next token (not allowed before tParen token) */ >+ public static final int CR_BEFORE = 0x01; >+ /** 02H = Spaces before opening parenthesis (only allowed before tParen token) */ >+ public static final int SPACE_BEFORE_OPEN_PAREN = 0x02; >+ /** 03H = Carriage returns before opening parenthesis (only allowed before tParen token) */ >+ public static final int CR_BEFORE_OPEN_PAREN = 0x03; >+ /** 04H = Spaces before closing parenthesis (only allowed before tParen, tFunc, and tFuncVar tokens) */ >+ public static final int SPACE_BEFORE_CLOSE_PAERN = 0x04; >+ /** 05H = Carriage returns before closing parenthesis (only allowed before tParen, tFunc, and tFuncVar tokens) */ >+ public static final int CR_BEFORE_CLOSE_PAREN = 0x05; >+ /** 06H = Spaces following the equality sign (only in macro sheets) */ >+ public static final int SPACE_AFTER_EQUALITY = 0x06; >+ } > > public AttrPtg() { > } >@@ -56,6 +78,19 @@ > field_1_options = in.readByte(); > field_2_data = in.readShort(); > } >+ private AttrPtg(int options, int data) { >+ field_1_options = (byte) options; >+ field_2_data = (short) data; >+ } >+ >+ /** >+ * @param type a constant from <tt>SpaceType</tt> >+ * @param count the number of space characters >+ */ >+ public static AttrPtg createSpace(int type, int count) { >+ int data = type & 0x00FF | (count << 8) & 0x00FFFF; >+ return new AttrPtg(space.set(0), data); >+ } > > public void setOptions(byte options) > { >@@ -131,21 +166,31 @@ > return field_2_data; > } > >- public String toString() >- { >- StringBuffer buffer = new StringBuffer(); >+ public String toString() { >+ StringBuffer sb = new StringBuffer(64); >+ sb.append(getClass().getName()).append(" ["); > >- buffer.append("AttrPtg\n"); >- buffer.append("options=").append(field_1_options).append("\n"); >- buffer.append("data =").append(field_2_data).append("\n"); >- buffer.append("semi =").append(isSemiVolatile()).append("\n"); >- buffer.append("optimif=").append(isOptimizedIf()).append("\n"); >- buffer.append("optchos=").append(isOptimizedChoose()).append("\n"); >- buffer.append("isGoto =").append(isGoto()).append("\n"); >- buffer.append("isSum =").append(isSum()).append("\n"); >- buffer.append("isBaxce=").append(isBaxcel()).append("\n"); >- buffer.append("isSpace=").append(isSpace()).append("\n"); >- return buffer.toString(); >+ if(isSemiVolatile()) { >+ sb.append("volatile "); >+ } >+ if(isSpace()) { >+ sb.append("space count=").append((field_2_data >> 8) & 0x00FF); >+ sb.append(" type=").append(field_2_data & 0x00FF).append(" "); >+ } >+ // the rest seem to be mutually exclusive >+ if(isOptimizedIf()) { >+ sb.append("if dist=").append(getData()); >+ } else if(isOptimizedChoose()) { >+ sb.append("choose dist=").append(getData()); >+ } else if(isGoto()) { >+ sb.append("skip dist=").append(getData()); >+ } else if(isSum()) { >+ sb.append("sum "); >+ } else if(isBaxcel()) { >+ sb.append("assign "); >+ } >+ sb.append("]"); >+ return sb.toString(); > } > > public void writeBytes(byte [] array, int offset) >Index: C:\josh\client\poi\svn\trunk/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java >=================================================================== >--- C:\josh\client\poi\svn\trunk/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java (revision 637139) >+++ C:\josh\client\poi\svn\trunk/src/testcases/org/apache/poi/hssf/model/TestFormulaParser.java (working copy) >@@ -844,4 +844,48 @@ > } > assertEquals("SUM(A32769:A32770)", cell.getCellFormula()); > } >+ >+ public void testSpaceAtStartOfFormula() { >+ // Simulating cell formula of "= 4" (note space) >+ // The same Ptg array can be observed if an excel file is saved with that exact formula >+ >+ AttrPtg spacePtg = AttrPtg.createSpace(AttrPtg.SpaceType.SPACE_BEFORE, 1); >+ Ptg[] ptgs = { spacePtg, new IntPtg(4), }; >+ String formulaString; >+ try { >+ formulaString = FormulaParser.toFormulaString(null, ptgs); >+ } catch (IllegalStateException e) { >+ if(e.getMessage().equalsIgnoreCase("too much stuff left on the stack")) { >+ throw new AssertionFailedError("Identified bug 44609"); >+ } >+ // else some unexpected error >+ throw e; >+ } >+ // FormulaParser strips spaces anyway >+ assertEquals("4", formulaString); >+ >+ ptgs = new Ptg[] { new IntPtg(3), spacePtg, new IntPtg(4), spacePtg, new AddPtg()}; >+ formulaString = FormulaParser.toFormulaString(null, ptgs); >+ assertEquals("3+4", formulaString); >+ } >+ >+ /** >+ * Checks some internal error detecting logic ('stack underflow error' in toFormulaString) >+ */ >+ public void testTooFewOperandArgs() { >+ // Simulating badly encoded cell formula of "=/1" >+ // Not sure if Excel could ever produce this >+ Ptg[] ptgs = { >+ // Excel would probably have put tMissArg here >+ new IntPtg(1), >+ new DividePtg(), >+ }; >+ try { >+ FormulaParser.toFormulaString(null, ptgs); >+ fail("Expected exception was not thrown"); >+ } catch (IllegalStateException e) { >+ // expected during successful test >+ assertTrue(e.getMessage().startsWith("Too few arguments suppled to operation token")); >+ } >+ } > } 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]