Merge pull request #343 from ryho/master

More improvements to cell formatting

cell.go

@@ -1,7 +1,6 @@
 package xlsx
 
 import (
-	"errors"
 	"fmt"
 	"math"
 	"strconv"
@@ -16,16 +15,24 @@ const (
 // CellType is an int type for storing metadata about the data type in the cell.
 type CellType int
 
-// Known types for cell values.
+// These are the cell types from the ST_CellType spec
 const (
 	CellTypeString CellType = iota
-	CellTypeFormula
+	// CellTypeStringFormula is a specific format for formulas that return string values. Formulas that return numbers
+	// and booleans are stored as those types.
+	CellTypeStringFormula
 	CellTypeNumeric
 	CellTypeBool
+	// CellTypeInline is not respected on save, all inline string cells will be saved as SharedStrings
+	// when saving to an XLSX file. This the same behavior as that found in Excel.
 	CellTypeInline
 	CellTypeError
+	// d (Date): Cell contains a date in the ISO 8601 format.
+	// That is the only mention of this format in the XLSX spec.
+	// Date seems to be unused by the current version of Excel, it stores dates as Numeric cells with a date format string.
+	// For now these cells will have their value output directly. It is unclear if the value is supposed to be parsed
+	// into a number and then formatted using the formatting or not.
 	CellTypeDate
-	CellTypeGeneral
 )
 
 func (ct CellType) Ptr() *CellType {
@@ -117,15 +124,9 @@ func (c *Cell) GetTime(date1904 bool) (t time.Time, err error) {
 // SetFloatWithFormat sets the value of a cell to a float and applies
 // formatting to the cell.
 func (c *Cell) SetFloatWithFormat(n float64, format string) {
-	// beauty the output when the float is small enough
-	if n != 0 && n < 0.00001 {
-		c.Value = strconv.FormatFloat(n, 'e', -1, 64)
-	} else {
-		c.Value = strconv.FormatFloat(n, 'f', -1, 64)
-	}
+	c.SetValue(n)
 	c.NumFmt = format
 	c.formula = ""
-	c.cellType = CellTypeNumeric
 }
 
 var timeLocationUTC, _ = time.LoadLocation("UTC")
@@ -181,7 +182,7 @@ func (c *Cell) SetDateTimeWithFormat(n float64, format string) {
 	c.Value = strconv.FormatFloat(n, 'f', -1, 64)
 	c.NumFmt = format
 	c.formula = ""
-	c.cellType = CellTypeDate
+	c.cellType = CellTypeNumeric
 }
 
 // Float returns the value of cell as a number.
@@ -230,10 +231,19 @@ func (c *Cell) SetInt(n int) {
 func (c *Cell) SetValue(n interface{}) {
 	switch t := n.(type) {
 	case time.Time:
-		c.SetDateTime(n.(time.Time))
+		c.SetDateTime(t)
 		return
-	case int, int8, int16, int32, int64, float32, float64:
-		c.setGeneral(fmt.Sprintf("%v", n))
+	case int, int8, int16, int32, int64:
+		c.setNumeric(fmt.Sprintf("%d", n))
+	case float64:
+		// When formatting floats, do not use fmt.Sprintf("%v", n), this will cause numbers below 1e-4 to be printed in
+		// scientific notation. Scientific notation is not a valid way to store numbers in XML.
+		// Also not not use fmt.Sprintf("%f", n), this will cause numbers to be stored as X.XXXXXX. Which means that
+		// numbers will lose precision and numbers with fewer significant digits such as 0 will be stored as 0.000000
+		// which causes tests to fail.
+		c.setNumeric(strconv.FormatFloat(t, 'f', -1, 64))
+	case float32:
+		c.setNumeric(strconv.FormatFloat(float64(t), 'f', -1, 32))
 	case string:
 		c.SetString(t)
 	case []byte:
@@ -245,12 +255,12 @@ func (c *Cell) SetValue(n interface{}) {
 	}
 }
 
-// SetInt sets a cell's value to an integer.
-func (c *Cell) setGeneral(s string) {
+// setNumeric sets a cell's value to a number
+func (c *Cell) setNumeric(s string) {
 	c.Value = s
 	c.NumFmt = builtInNumFmt[builtInNumFmtIndex_GENERAL]
 	c.formula = ""
-	c.cellType = CellTypeGeneral
+	c.cellType = CellTypeNumeric
 }
 
 // Int returns the value of cell as integer.
@@ -283,7 +293,7 @@ func (c *Cell) Bool() bool {
 		return c.Value == "1"
 	}
 	// If numeric, base it on a non-zero.
-	if c.cellType == CellTypeNumeric || c.cellType == CellTypeGeneral {
+	if c.cellType == CellTypeNumeric {
 		return c.Value != "0"
 	}
 	// Return whether there's an empty string.
@@ -293,7 +303,12 @@ func (c *Cell) Bool() bool {
 // SetFormula sets the format string for a cell.
 func (c *Cell) SetFormula(formula string) {
 	c.formula = formula
-	c.cellType = CellTypeFormula
+	c.cellType = CellTypeNumeric
+}
+
+func (c *Cell) SetStringFormula(formula string) {
+	c.formula = formula
+	c.cellType = CellTypeStringFormula
 }
 
 // Formula returns the formula string for the cell.
@@ -335,6 +350,8 @@ func (c *Cell) formatToInt(format string) (string, error) {
 	return fmt.Sprintf(format, int(f)), nil
 }
 
+// getNumberFormat will update the parsedNumFmt struct if it has become out of date, since a cell's NumFmt string is a
+// public field that could be edited by clients.
 func (c *Cell) getNumberFormat() *parsedNumberFormat {
 	if c.parsedNumFmt == nil || c.parsedNumFmt.numFmt != c.NumFmt {
 		c.parsedNumFmt = parseFullNumberFormatString(c.NumFmt)
@@ -346,42 +363,11 @@ func (c *Cell) getNumberFormat() *parsedNumberFormat {
 // from a Cell.  If it is possible to apply a format to the cell
 // value, it will do so, if not then an error will be returned, along
 // with the raw value of the Cell.
-//
-// This is the documentation of the "General" Format in the Office Open XML spec:
-//
-// Numbers
-// The application shall attempt to display the full number up to 11 digits (inc. decimal point). If the number is too
-// large*, the application shall attempt to show exponential format. If the number has too many significant digits, the
-// display shall be truncated. The optimal method of display is based on the available cell width. If the number cannot
-// be displayed using any of these formats in the available width, the application shall show "#" across the width of
-// the cell.
-//
-// Conditions for switching to exponential format:
-// 1. The cell value shall have at least five digits for xE-xx
-// 2. If the exponent is bigger than the size allowed, a floating point number cannot fit, so try exponential notation.
-// 3. Similarly, for negative exponents, check if there is space for even one (non-zero) digit in floating point format**.
-// 4. Finally, if there isn't room for all of the significant digits in floating point format (for a negative exponent),
-// exponential format shall display more digits if the exponent is less than -3. (The 3 is because E-xx takes 4
-// characters, and the leading 0 in floating point takes only 1 character. Thus, for an exponent less than -3, there is
-// more than 3 additional leading 0's, more than enough to compensate for the size of the E-xx.)
-//
-// Floating point rule:
-// For general formatting in cells, max overall length for cell display is 11, not including negative sign, but includes
-// leading zeros and decimal separator.***
-//
-// Added Notes:
-// * "If the number is too large" can also mean "if the number has more than 11 digits", so greater than or equal to
-// 1e11 and less than 1e-9.
-// ** Means that you should switch to scientific if there would be 9 zeros after the decimal (the decimal and first zero
-// count against the 11 character limit), so less than 1e9.
-// *** The way this is written, you can get numbers that are more than 11 characters because the golang Float fmt
-// does not support adjusting the precision while not padding with zeros, while also not switching to scientific
-// notation too early.
 func (c *Cell) FormattedValue() (string, error) {
 	fullFormat := c.getNumberFormat()
 	returnVal, err := fullFormat.FormatValue(c)
-	if fullFormat.parseEncounteredError {
-		return returnVal, errors.New("invalid number format")
+	if fullFormat.parseEncounteredError != nil {
+		return returnVal, *fullFormat.parseEncounteredError
 	}
 	return returnVal, err
 }

cell_test.go

@@ -111,7 +111,7 @@ func (l *CellSuite) TestSetFloat(c *C) {
 	cell.SetFloat(0)
 	c.Assert(cell.Value, Equals, "0")
 	cell.SetFloat(0.000005)
-	c.Assert(cell.Value, Equals, "5e-06")
+	c.Assert(cell.Value, Equals, "0.000005")
 	cell.SetFloat(100.0)
 	c.Assert(cell.Value, Equals, "100")
 	cell.SetFloat(37947.75334343)
@@ -125,84 +125,84 @@ func (l *CellSuite) TestGeneralNumberHandling(c *C) {
 	// 1.1 will get you the same, with a stored value of 1.1000000000000001.
 	// Also, numbers greater than 1e11 and less than 1e-9 wil be shown as scientific notation.
 	testCases := []struct {
-		value                string
-		formattedValueOutput string
-		noExpValueOutput     string
+		value                   string
+		formattedValueOutput    string
+		noScientificValueOutput string
 	}{
 		{
-			value:                "18.989999999999998",
-			formattedValueOutput: "18.99",
-			noExpValueOutput:     "18.99",
+			value:                   "18.989999999999998",
+			formattedValueOutput:    "18.99",
+			noScientificValueOutput: "18.99",
 		},
 		{
-			value:                "1.1000000000000001",
-			formattedValueOutput: "1.1",
-			noExpValueOutput:     "1.1",
+			value:                   "1.1000000000000001",
+			formattedValueOutput:    "1.1",
+			noScientificValueOutput: "1.1",
 		},
 		{
-			value:                "0.0000000000000001",
-			formattedValueOutput: "1E-16",
-			noExpValueOutput:     "0.0000000000000001",
+			value:                   "0.0000000000000001",
+			formattedValueOutput:    "1E-16",
+			noScientificValueOutput: "0.0000000000000001",
 		},
 		{
-			value:                "0.000000000000008",
-			formattedValueOutput: "8E-15",
-			noExpValueOutput:     "0.000000000000008",
+			value:                   "0.000000000000008",
+			formattedValueOutput:    "8E-15",
+			noScientificValueOutput: "0.000000000000008",
 		},
 		{
-			value:                "1000000000000000000",
-			formattedValueOutput: "1E+18",
-			noExpValueOutput:     "1000000000000000000",
+			value:                   "1000000000000000000",
+			formattedValueOutput:    "1E+18",
+			noScientificValueOutput: "1000000000000000000",
 		},
 		{
-			value:                "1230000000000000000",
-			formattedValueOutput: "1.23E+18",
-			noExpValueOutput:     "1230000000000000000",
+			value:                   "1230000000000000000",
+			formattedValueOutput:    "1.23E+18",
+			noScientificValueOutput: "1230000000000000000",
 		},
 		{
-			value:                "12345678",
-			formattedValueOutput: "12345678",
-			noExpValueOutput:     "12345678",
+			value:                   "12345678",
+			formattedValueOutput:    "12345678",
+			noScientificValueOutput: "12345678",
 		},
 		{
-			value:                "0",
-			formattedValueOutput: "0",
-			noExpValueOutput:     "0",
+			value:                   "0",
+			formattedValueOutput:    "0",
+			noScientificValueOutput: "0",
 		},
 		{
-			value:                "-18.989999999999998",
-			formattedValueOutput: "-18.99",
-			noExpValueOutput:     "-18.99",
+			value:                   "-18.989999999999998",
+			formattedValueOutput:    "-18.99",
+			noScientificValueOutput: "-18.99",
 		},
 		{
-			value:                "-1.1000000000000001",
-			formattedValueOutput: "-1.1",
-			noExpValueOutput:     "-1.1",
+			value:                   "-1.1000000000000001",
+			formattedValueOutput:    "-1.1",
+			noScientificValueOutput: "-1.1",
 		},
 		{
-			value:                "-0.0000000000000001",
-			formattedValueOutput: "-1E-16",
-			noExpValueOutput:     "-0.0000000000000001",
+			value:                   "-0.0000000000000001",
+			formattedValueOutput:    "-1E-16",
+			noScientificValueOutput: "-0.0000000000000001",
 		},
 		{
-			value:                "-0.000000000000008",
-			formattedValueOutput: "-8E-15",
-			noExpValueOutput:     "-0.000000000000008",
+			value:                   "-0.000000000000008",
+			formattedValueOutput:    "-8E-15",
+			noScientificValueOutput: "-0.000000000000008",
 		},
 		{
-			value:                "-1000000000000000000",
-			formattedValueOutput: "-1E+18",
-			noExpValueOutput:     "-1000000000000000000",
+			value:                   "-1000000000000000000",
+			formattedValueOutput:    "-1E+18",
+			noScientificValueOutput: "-1000000000000000000",
 		},
 		{
-			value:                "-1230000000000000000",
-			formattedValueOutput: "-1.23E+18",
-			noExpValueOutput:     "-1230000000000000000",
+			value:                   "-1230000000000000000",
+			formattedValueOutput:    "-1.23E+18",
+			noScientificValueOutput: "-1230000000000000000",
 		},
 		{
-			value:                "-12345678",
-			formattedValueOutput: "-12345678",
-			noExpValueOutput:     "-12345678",
+			value:                   "-12345678",
+			formattedValueOutput:    "-12345678",
+			noScientificValueOutput: "-12345678",
 		},
 	}
 	for _, testCase := range testCases {
@@ -220,10 +220,115 @@ func (l *CellSuite) TestGeneralNumberHandling(c *C) {
 		if err != nil {
 			c.Fatal(err)
 		}
-		c.Assert(val, Equals, testCase.noExpValueOutput)
+		c.Assert(val, Equals, testCase.noScientificValueOutput)
 	}
 }
 
+// TestCellTypeFormatHandling tests all cell types other than numeric. Numeric cells are tested above since those
+// cells have so many edge cases.
+func (l *CellSuite) TestCellTypeFormatHandling(c *C) {
+	testCases := []struct {
+		cellType             CellType
+		numFmt               string
+		value                string
+		formattedValueOutput string
+		expectError          bool
+	}{
+		// All of the string cell types, will return only the string format if there is no @ symbol in the format.
+		{
+			cellType:             CellTypeInline,
+			numFmt:               `0;0;0;"Error"`,
+			value:                "asdf",
+			formattedValueOutput: "Error",
+		},
+		{
+			cellType:             CellTypeString,
+			numFmt:               `0;0;0;"Error"`,
+			value:                "asdf",
+			formattedValueOutput: "Error",
+		},
+		{
+			cellType:             CellTypeStringFormula,
+			numFmt:               `0;0;0;"Error"`,
+			value:                "asdf",
+			formattedValueOutput: "Error",
+		},
+		// Errors are returned as is regardless of what the format shows
+		{
+			cellType:             CellTypeError,
+			numFmt:               `0;0;0;"Error"`,
+			value:                "#NAME?",
+			formattedValueOutput: "#NAME?",
+		},
+		{
+			cellType:             CellTypeError,
+			numFmt:               `"$"@`,
+			value:                "#######",
+			formattedValueOutput: "#######",
+		},
+		// Dates are returned as is regardless of what the format shows
+		{
+			cellType:             CellTypeDate,
+			numFmt:               `"$"@`,
+			value:                "2017-10-24T15:29:30+00:00",
+			formattedValueOutput: "2017-10-24T15:29:30+00:00",
+		},
+		// Make sure the format used above would have done something for a string
+		{
+			cellType:             CellTypeString,
+			numFmt:               `"$"@`,
+			value:                "#######",
+			formattedValueOutput: "$#######",
+		},
+		// For bool cells, 0 is false, 1 is true, anything else will error
+		{
+			cellType:             CellTypeBool,
+			numFmt:               `"$"@`,
+			value:                "1",
+			formattedValueOutput: "TRUE",
+		},
+		{
+			cellType:             CellTypeBool,
+			numFmt:               `"$"@`,
+			value:                "0",
+			formattedValueOutput: "FALSE",
+		},
+		{
+			cellType:             CellTypeBool,
+			numFmt:               `"$"@`,
+			value:                "2",
+			expectError:          true,
+			formattedValueOutput: "2",
+		},
+		{
+			cellType:             CellTypeBool,
+			numFmt:               `"$"@`,
+			value:                "2",
+			expectError:          true,
+			formattedValueOutput: "2",
+		},
+		// Invalid cell type should cause an error
+		{
+			cellType:             CellType(7),
+			numFmt:               `0`,
+			value:                "1.0",
+			expectError:          true,
+			formattedValueOutput: "1.0",
+		},
+	}
+	for _, testCase := range testCases {
+		cell := Cell{
+			cellType: testCase.cellType,
+			NumFmt:   testCase.numFmt,
+			Value:    testCase.value,
+		}
+		val, err := cell.FormattedValue()
+		if err != nil != testCase.expectError {
+			c.Fatal(err)
+		}
+		c.Assert(val, Equals, testCase.formattedValueOutput)
+	}
+}
 func (s *CellSuite) TestGetTime(c *C) {
 	cell := Cell{}
 	cell.SetFloat(0)
@@ -508,13 +613,13 @@ func (s *CellSuite) TestSetterGetters(c *C) {
 	intValue, _ := cell.Int()
 	c.Assert(intValue, Equals, 1024)
 	c.Assert(cell.NumFmt, Equals, builtInNumFmt[builtInNumFmtIndex_GENERAL])
-	c.Assert(cell.Type(), Equals, CellTypeGeneral)
+	c.Assert(cell.Type(), Equals, CellTypeNumeric)
 
 	cell.SetInt64(1024)
 	int64Value, _ := cell.Int64()
 	c.Assert(int64Value, Equals, int64(1024))
 	c.Assert(cell.NumFmt, Equals, builtInNumFmt[builtInNumFmtIndex_GENERAL])
-	c.Assert(cell.Type(), Equals, CellTypeGeneral)
+	c.Assert(cell.Type(), Equals, CellTypeNumeric)
 
 	cell.SetFloat(1.024)
 	float, _ := cell.Float()
@@ -522,11 +627,15 @@ func (s *CellSuite) TestSetterGetters(c *C) {
 	c.Assert(float, Equals, 1.024)
 	c.Assert(intValue, Equals, 1)
 	c.Assert(cell.NumFmt, Equals, builtInNumFmt[builtInNumFmtIndex_GENERAL])
-	c.Assert(cell.Type(), Equals, CellTypeGeneral)
+	c.Assert(cell.Type(), Equals, CellTypeNumeric)
 
 	cell.SetFormula("10+20")
 	c.Assert(cell.Formula(), Equals, "10+20")
-	c.Assert(cell.Type(), Equals, CellTypeFormula)
+	c.Assert(cell.Type(), Equals, CellTypeNumeric)
+
+	cell.SetStringFormula("A1")
+	c.Assert(cell.Formula(), Equals, "A1")
+	c.Assert(cell.Type(), Equals, CellTypeStringFormula)
 }
 
 // TestOddInput is a regression test for #101. When the number format
@@ -588,6 +697,14 @@ func (s *CellSuite) TestSetValue(c *C) {
 		c.Assert(err, IsNil)
 		c.Assert(val, Equals, 1.11)
 	}
+	// In the naive implementation using go fmt "%v", this test would fail and the cell.Value would be "1e-06"
+	for _, i := range []interface{}{0.000001, float32(0.000001), float64(0.000001)} {
+		cell.SetValue(i)
+		c.Assert(cell.Value, Equals, "0.000001")
+		val, err := cell.Float()
+		c.Assert(err, IsNil)
+		c.Assert(val, Equals, 0.000001)
+	}
 
 	// time
 	cell.SetValue(time.Unix(0, 0))

col.go

@@ -15,22 +15,26 @@ type Col struct {
 	style        *Style
 }
 
+// SetType will set the format string of a column based on the type that you want to set it to.
+// This function does not really make a lot of sense.
 func (c *Col) SetType(cellType CellType) {
 	switch cellType {
 	case CellTypeString:
 		c.numFmt = builtInNumFmt[builtInNumFmtIndex_STRING]
-	case CellTypeBool:
-		c.numFmt = builtInNumFmt[builtInNumFmtIndex_GENERAL] //TEMP
 	case CellTypeNumeric:
 		c.numFmt = builtInNumFmt[builtInNumFmtIndex_INT]
-	case CellTypeDate:
-		c.numFmt = builtInNumFmt[builtInNumFmtIndex_DATE]
-	case CellTypeFormula:
-		c.numFmt = builtInNumFmt[builtInNumFmtIndex_GENERAL]
+	case CellTypeBool:
+		c.numFmt = builtInNumFmt[builtInNumFmtIndex_GENERAL] //TEMP
+	case CellTypeInline:
+		c.numFmt = builtInNumFmt[builtInNumFmtIndex_STRING]
 	case CellTypeError:
 		c.numFmt = builtInNumFmt[builtInNumFmtIndex_GENERAL] //TEMP
-	case CellTypeGeneral:
+	case CellTypeDate:
+		// Cells that are stored as dates are not properly supported in this library.
+		// They should instead be stored as a Numeric with a date format.
 		c.numFmt = builtInNumFmt[builtInNumFmtIndex_GENERAL]
+	case CellTypeStringFormula:
+		c.numFmt = builtInNumFmt[builtInNumFmtIndex_STRING]
 	}
 }
 

file.go

@@ -50,8 +50,7 @@ func OpenFileWithRowLimit(fileName string, rowLimit int) (file *File, err error)
 	if err != nil {
 		return nil, err
 	}
-	file, err = ReadZipWithRowLimit(z, rowLimit)
-	return
+	return ReadZipWithRowLimit(z, rowLimit)
 }
 
 // OpenBinary() take bytes of an XLSX file and returns a populated

file_test.go

@@ -951,7 +951,7 @@ func (l *FileSuite) TestReadWorkbookWithTypes(c *C) {
 	c.Assert(sheet.Rows[5].Cells[0].Bool(), Equals, true)
 
 	// formula
-	c.Assert(sheet.Rows[6].Cells[0].Type(), Equals, CellTypeFormula)
+	c.Assert(sheet.Rows[6].Cells[0].Type(), Equals, CellTypeNumeric)
 	c.Assert(sheet.Rows[6].Cells[0].Formula(), Equals, "10+20")
 	c.Assert(sheet.Rows[6].Cells[0].Value, Equals, "30")
 

format_code.go

@@ -19,7 +19,7 @@ type parsedNumberFormat struct {
 	negativeFormat                *formatOptions
 	zeroFormat                    *formatOptions
 	textFormat                    *formatOptions
-	parseEncounteredError         bool
+	parseEncounteredError         *error
 }
 
 type formatOptions struct {
@@ -31,8 +31,60 @@ type formatOptions struct {
 	suffix              string
 }
 
+// FormatValue returns a value, and possibly an error condition
+// from a Cell.  If it is possible to apply a format to the cell
+// value, it will do so, if not then an error will be returned, along
+// with the raw value of the Cell.
+//
+// This is the documentation of the "General" Format in the Office Open XML spec:
+//
+// Numbers
+// The application shall attempt to display the full number up to 11 digits (inc. decimal point). If the number is too
+// large*, the application shall attempt to show exponential format. If the number has too many significant digits, the
+// display shall be truncated. The optimal method of display is based on the available cell width. If the number cannot
+// be displayed using any of these formats in the available width, the application shall show "#" across the width of
+// the cell.
+//
+// Conditions for switching to exponential format:
+// 1. The cell value shall have at least five digits for xE-xx
+// 2. If the exponent is bigger than the size allowed, a floating point number cannot fit, so try exponential notation.
+// 3. Similarly, for negative exponents, check if there is space for even one (non-zero) digit in floating point format**.
+// 4. Finally, if there isn't room for all of the significant digits in floating point format (for a negative exponent),
+// exponential format shall display more digits if the exponent is less than -3. (The 3 is because E-xx takes 4
+// characters, and the leading 0 in floating point takes only 1 character. Thus, for an exponent less than -3, there is
+// more than 3 additional leading 0's, more than enough to compensate for the size of the E-xx.)
+//
+// Floating point rule:
+// For general formatting in cells, max overall length for cell display is 11, not including negative sign, but includes
+// leading zeros and decimal separator.***
+//
+// Added Notes:
+// * "If the number is too large" can also mean "if the number has more than 11 digits", so greater than or equal to
+// 1e11 and less than 1e-9.
+// ** Means that you should switch to scientific if there would be 9 zeros after the decimal (the decimal and first zero
+// count against the 11 character limit), so less than 1e9.
+// *** The way this is written, you can get numbers that are more than 11 characters because the golang Float fmt
+// does not support adjusting the precision while not padding with zeros, while also not switching to scientific
+// notation too early.
 func (fullFormat *parsedNumberFormat) FormatValue(cell *Cell) (string, error) {
-	if cell.cellType != CellTypeNumeric {
+	switch cell.cellType {
+	case CellTypeError:
+		// The error type is what XLSX uses in error cases such as when formulas are invalid.
+		// There will be text in the cell's value that can be shown, something ugly like #NAME? or #######
+		return cell.Value, nil
+	case CellTypeBool:
+		if cell.Value == "0" {
+			return "FALSE", nil
+		} else if cell.Value == "1" {
+			return "TRUE", nil
+		} else {
+			return cell.Value, errors.New("invalid value in bool cell")
+		}
+	case CellTypeString:
+		fallthrough
+	case CellTypeInline:
+		fallthrough
+	case CellTypeStringFormula:
 		textFormat := cell.parsedNumFmt.textFormat
 		// This switch statement is only for String formats
 		switch textFormat.reducedFormatString {
@@ -41,22 +93,51 @@ func (fullFormat *parsedNumberFormat) FormatValue(cell *Cell) (string, error) {
 		case builtInNumFmt[builtInNumFmtIndex_STRING]: // String is "@"
 			return textFormat.prefix + cell.Value + textFormat.suffix, nil
 		case "":
+			// If cell is not "General" and there is not an "@" symbol in the format, then the cell's value is not
+			// used when determining what to display. It would be completely legal to have a format of "Error"
+			// for strings, and all values that are not numbers would show up as "Error". In that case, this code would
+			// have a prefix of "Error" and a reduced format string of "" (empty string).
 			return textFormat.prefix + textFormat.suffix, nil
 		default:
-			return cell.Value, errors.New("invalid or unsupported format")
+			return cell.Value, errors.New("invalid or unsupported format, unsupported string format")
 		}
+	case CellTypeDate:
+		// These are dates that are stored in date format instead of being stored as numbers with a format to turn them
+		// into a date string.
+		return cell.Value, nil
+	case CellTypeNumeric:
+		return fullFormat.formatNumericCell(cell)
+	default:
+		return cell.Value, errors.New("unknown cell type")
 	}
+}
+
+func (fullFormat *parsedNumberFormat) formatNumericCell(cell *Cell) (string, error) {
+	rawValue := strings.TrimSpace(cell.Value)
+	// If there wasn't a value in the cell, it shouldn't have been marked as Numeric.
+	// It's better to support this case though.
+	if rawValue == "" {
+		return "", nil
+	}
+
 	if fullFormat.isTimeFormat {
-		return fullFormat.parseTime(cell.Value, cell.date1904)
+		return fullFormat.parseTime(rawValue, cell.date1904)
 	}
 	var numberFormat *formatOptions
-	floatVal, floatErr := strconv.ParseFloat(cell.Value, 64)
+	floatVal, floatErr := strconv.ParseFloat(rawValue, 64)
 	if floatErr != nil {
-		return cell.Value, floatErr
+		return rawValue, floatErr
 	}
+	// Choose the correct format. There can be different formats for positive, negative, and zero numbers.
+	// Excel only uses the zero format if the value is literally zero, even if the number is so small that it shows
+	// up as "0" when the positive format is used.
 	if floatVal > 0 {
 		numberFormat = fullFormat.positiveFormat
 	} else if floatVal < 0 {
+		// If format string specified a different format for negative numbers, then the number should be made positive
+		// before getting formatted. The format string itself will contain formatting that denotes a negative number and
+		// this formatting will end up in the prefix or suffix. Commonly if there is a negative format specified, the
+		// number will get surrounded by parenthesis instead of showing it with a minus sign.
 		if fullFormat.negativeFormatExpectsPositive {
 			floatVal = math.Abs(floatVal)
 		}
@@ -65,6 +146,9 @@ func (fullFormat *parsedNumberFormat) FormatValue(cell *Cell) (string, error) {
 		numberFormat = fullFormat.zeroFormat
 	}
 
+	// When showPercent is true, multiply the number by 100.
+	// The percent sign will be in the prefix or suffix already, so it does not need to be added in this function.
+	// The number format itself will be the same as any other number format once the value is multiplied by 100.
 	if numberFormat.showPercent {
 		floatVal = 100 * floatVal
 	}
@@ -74,21 +158,19 @@ func (fullFormat *parsedNumberFormat) FormatValue(cell *Cell) (string, error) {
 	// Some of these "supported" formats should have thousand separators, but don't get them since Go fmt
 	// doesn't have a way to request thousands separators.
 	// The only things that should be supported here are in the array formattingCharacters,
-	// everything else has been stripped out before.
+	// everything else has been stripped out before and will be placed in the prefix or suffix.
 	// The formatting characters can have non-formatting characters mixed in with them and those should be maintained.
 	// However, at this time we fail to parse those formatting codes and they get replaced with "General"
-
-	// This switch statement is only for number formats
 	var formattedNum string
 	switch numberFormat.reducedFormatString {
 	case builtInNumFmt[builtInNumFmtIndex_GENERAL]: // General is literally "general"
 		// prefix, showPercent, and suffix cannot apply to the general format
 		// The logic for showing numbers when the format is "general" is much more complicated than the rest of these.
-		val, err := generalNumericScientific(cell.Value, true)
+		generalFormatted, err := generalNumericScientific(cell.Value, true)
 		if err != nil {
-			return cell.Value, nil
+			return rawValue, nil
 		}
-		return val, nil
+		return generalFormatted, nil
 	case builtInNumFmt[builtInNumFmtIndex_STRING]: // String is "@"
 		formattedNum = cell.Value
 	case builtInNumFmt[builtInNumFmtIndex_INT], "#,##0": // Int is "0"
@@ -107,7 +189,7 @@ func (fullFormat *parsedNumberFormat) FormatValue(cell *Cell) (string, error) {
 	case "":
 		// Do nothing.
 	default:
-		return cell.Value, nil
+		return rawValue, nil
 	}
 	return numberFormat.prefix + formattedNum + numberFormat.suffix, nil
 }
@@ -172,17 +254,18 @@ func parseFullNumberFormatString(numFmt string) *parsedNumberFormat {
 			if err != nil {
 				// If an invalid number section is found, fall back to general
 				parsedFormat = fallbackErrorFormat
-				parsedNumFmt.parseEncounteredError = true
+				parsedNumFmt.parseEncounteredError = &err
 			}
 			fmtOptions = append(fmtOptions, parsedFormat)
 		}
 	} else {
 		fmtOptions = append(fmtOptions, fallbackErrorFormat)
-		parsedNumFmt.parseEncounteredError = true
+		parsedNumFmt.parseEncounteredError = &err
 	}
 	if len(fmtOptions) > 4 {
 		fmtOptions = []*formatOptions{fallbackErrorFormat}
-		parsedNumFmt.parseEncounteredError = true
+		err = errors.New("invalid number format, too many format sections")
+		parsedNumFmt.parseEncounteredError = &err
 	}
 
 	if len(fmtOptions) == 1 {
@@ -242,7 +325,7 @@ func splitFormatOnSemicolon(format string) ([]string, error) {
 			endQuoteIndex := strings.Index(format[i+1:], "\"")
 			if endQuoteIndex == -1 {
 				// This is an invalid format string, fall back to general
-				return nil, errors.New("invalid format string")
+				return nil, errors.New("invalid format string, unmatched double quote")
 			}
 			i += endQuoteIndex + 1
 		}
@@ -376,7 +459,7 @@ func parseLiterals(format string) (string, string, bool, error) {
 			// If there is a quote skip to the next quote, and add the quoted characters to the prefix
 			endQuoteIndex := strings.Index(curReducedFormat[1:], "\"")
 			if endQuoteIndex == -1 {
-				return "", "", false, errors.New("invalid formatting code")
+				return "", "", false, errors.New("invalid formatting code, unmatched double quote")
 			}
 			prefix = prefix + curReducedFormat[1:endQuoteIndex+1]
 			i += endQuoteIndex + 1
@@ -389,7 +472,7 @@ func parseLiterals(format string) (string, string, bool, error) {
 			// conditionals (e.g. [>100], the valid conditionals are =, >, <, >=, <=, <>)
 			bracketIndex := strings.Index(curReducedFormat, "]")
 			if bracketIndex == -1 {
-				return "", "", false, errors.New("invalid formatting code")
+				return "", "", false, errors.New("invalid formatting code, invalid brackets")
 			}
 			// Currencies in Excel are annotated with this format: [$<Currency String>-<Language Info>]
 			// Currency String is something like $, ¥, €, or £
@@ -400,7 +483,7 @@ func parseLiterals(format string) (string, string, bool, error) {
 					// Get the currency symbol, and skip to the end of the currency format
 					prefix += curReducedFormat[2:dashIndex]
 				} else {
-					return "", "", false, errors.New("invalid formatting code")
+					return "", "", false, errors.New("invalid formatting code, invalid currency annotation")
 				}
 			}
 			i += bracketIndex
@@ -414,8 +497,8 @@ func parseLiterals(format string) (string, string, bool, error) {
 					return prefix, format[i:], showPercent, nil
 				}
 			}
-			// Symbols that don't have meaning and aren't in the exempt literal characters, but be escaped.
-			return "", "", false, errors.New("invalid formatting code")
+			// Symbols that don't have meaning and aren't in the exempt literal characters and are not escaped.
+			return "", "", false, errors.New("invalid formatting code: unsupported or unescaped characters")
 		}
 	}
 	return prefix, "", showPercent, nil

lib.go

@@ -443,53 +443,56 @@ func shiftCell(cellID string, dx, dy int) string {
 // fillCellData attempts to extract a valid value, usable in
 // CSV form from the raw cell value.  Note - this is not actually
 // general enough - we should support retaining tabs and newlines.
-func fillCellData(rawcell xlsxC, reftable *RefTable, sharedFormulas map[int]sharedFormula, cell *Cell) {
-	var data = rawcell.V
-	if len(data) > 0 {
-		vval := strings.Trim(data, " \t\n\r")
-		switch rawcell.T {
-		case "s": // Shared String
-			ref, error := strconv.Atoi(vval)
-			if error != nil {
-				panic(error)
-			}
-			cell.Value = reftable.ResolveSharedString(ref)
-			cell.cellType = CellTypeString
-		case "b": // Boolean
-			cell.Value = vval
-			cell.cellType = CellTypeBool
-		case "e": // Error
-			cell.Value = vval
-			cell.formula = formulaForCell(rawcell, sharedFormulas)
-			cell.cellType = CellTypeError
-		default:
-			if rawcell.F == nil {
-				// Numeric
-				cell.Value = vval
-				cell.cellType = CellTypeNumeric
-			} else {
-				// Formula
-				cell.Value = vval
-				cell.formula = formulaForCell(rawcell, sharedFormulas)
-				cell.cellType = CellTypeFormula
+func fillCellData(rawCell xlsxC, refTable *RefTable, sharedFormulas map[int]sharedFormula, cell *Cell) {
+	val := strings.Trim(rawCell.V, " \t\n\r")
+	cell.formula = formulaForCell(rawCell, sharedFormulas)
+	switch rawCell.T {
+	case "s": // Shared String
+		cell.cellType = CellTypeString
+		if val != "" {
+			ref, err := strconv.Atoi(val)
+			if err != nil {
+				panic(err)
 			}
+			cell.Value = refTable.ResolveSharedString(ref)
 		}
-	} else {
-		if rawcell.Is != nil {
-			fillCellDataFromInlineString(rawcell, cell)
-		}
+	case "inlineStr":
+		cell.cellType = CellTypeInline
+		fillCellDataFromInlineString(rawCell, cell)
+	case "b": // Boolean
+		cell.Value = val
+		cell.cellType = CellTypeBool
+	case "e": // Error
+		cell.Value = val
+		cell.cellType = CellTypeError
+	case "str":
+		// String Formula (special type for cells with formulas that return a string value)
+		// Unlike the other string cell types, the string is stored directly in the value.
+		cell.Value = val
+		cell.cellType = CellTypeStringFormula
+	case "d": // Date: Cell contains a date in the ISO 8601 format.
+		cell.Value = val
+		cell.cellType = CellTypeDate
+	case "": // Numeric is the default
+		fallthrough
+	case "n": // Numeric
+		cell.Value = val
+		cell.cellType = CellTypeNumeric
+	default:
+		panic(errors.New("invalid cell type"))
 	}
 }
 
 // fillCellDataFromInlineString attempts to get inline string data and put it into a Cell.
 func fillCellDataFromInlineString(rawcell xlsxC, cell *Cell) {
-	if rawcell.Is.T != "" {
-		cell.Value = strings.Trim(rawcell.Is.T, " \t\n\r")
-		cell.cellType = CellTypeInline
-	} else {
-		cell.Value = ""
-		for _, r := range rawcell.Is.R {
-			cell.Value += r.T
+	cell.Value = ""
+	if rawcell.Is != nil {
+		if rawcell.Is.T != "" {
+			cell.Value = strings.Trim(rawcell.Is.T, " \t\n\r")
+		} else {
+			for _, r := range rawcell.Is.R {
+				cell.Value += r.T
+			}
 		}
 	}
 }
@@ -669,7 +672,6 @@ func readSheetFromFile(sc chan *indexedSheet, index int, rsheet xlsxSheet, fi *F
 	result := &indexedSheet{Index: index, Sheet: nil, Error: nil}
 	defer func() {
 		if e := recover(); e != nil {
-
 			switch e.(type) {
 			case error:
 				result.Error = e.(error)

lib_test.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/xml"
 	"os"
-
 	"strings"
 
 	. "gopkg.in/check.v1"
@@ -1013,7 +1012,7 @@ func (l *LibSuite) TestReadRowsFromSheetWithMultipleTypes(c *C) {
 	c.Assert(cell4.Bool(), Equals, true)
 
 	cell5 := row.Cells[4]
-	c.Assert(cell5.Type(), Equals, CellTypeFormula)
+	c.Assert(cell5.Type(), Equals, CellTypeNumeric)
 	c.Assert(cell5.Formula(), Equals, "10+20")
 	c.Assert(cell5.Value, Equals, "30")
 

sheet.go

@@ -1,6 +1,7 @@
 package xlsx
 
 import (
+	"errors"
 	"fmt"
 	"strconv"
 )
@@ -287,37 +288,40 @@ func (s *Sheet) makeXLSXSheet(refTable *RefTable, styles *xlsxStyleSheet) *xlsxW
 			if c > maxCell {
 				maxCell = c
 			}
-			xC := xlsxC{}
-			xC.R = GetCellIDStringFromCoords(c, r)
+			xC := xlsxC{
+				S: XfId,
+				R: GetCellIDStringFromCoords(c, r),
+			}
+			if cell.formula != "" {
+				xC.F = &xlsxF{Content: cell.formula}
+			}
 			switch cell.cellType {
+			case CellTypeInline:
+				// Inline strings are turned into shared strings since they are more efficient.
+				// This is what Excel does as well.
+				fallthrough
 			case CellTypeString:
 				if len(cell.Value) > 0 {
 					xC.V = strconv.Itoa(refTable.AddString(cell.Value))
 				}
 				xC.T = "s"
-				xC.S = XfId
-			case CellTypeBool:
-				xC.V = cell.Value
-				xC.T = "b"
-				xC.S = XfId
 			case CellTypeNumeric:
+				// Numeric is the default, so the type can be left blank
 				xC.V = cell.Value
-				xC.S = XfId
-			case CellTypeDate:
-				xC.V = cell.Value
-				xC.S = XfId
-			case CellTypeFormula:
+			case CellTypeBool:
 				xC.V = cell.Value
-				xC.F = &xlsxF{Content: cell.formula}
-				xC.S = XfId
+				xC.T = "b"
 			case CellTypeError:
 				xC.V = cell.Value
-				xC.F = &xlsxF{Content: cell.formula}
 				xC.T = "e"
-				xC.S = XfId
-			case CellTypeGeneral:
+			case CellTypeDate:
+				xC.V = cell.Value
+				xC.T = "d"
+			case CellTypeStringFormula:
 				xC.V = cell.Value
-				xC.S = XfId
+				xC.T = "str"
+			default:
+				panic(errors.New("unknown cell type cannot be marshaled"))
 			}
 
 			xRow.C = append(xRow.C, xC)