Browse Source

Fix a number of small issues with number format handling

I noticed that number formats weren't being unmarshalled correctly from
the excel files I was using, and traced it back to the fact it's
attributes weren't marked as attributes. Since Marshal is handwritten it
was only affecting unmarshalling. Fixing this exposed some other issues in
number format handling that was causing the tests to fail:
1. The number formats were kept in a global map, even though their ids
   are reused between different workbooks. This caused the tests to
   interfere with each other. I fixed this by making it a per-stylesheet
   which matches Excel's model.
2. The method makeXLSXStyleElements returns a nil initialized numFmt.
   Its id was 0 which conflicts with the builtin general number format.
   It was only used in 1 place (and it no-oped before because of the
   previous bug) so I just removed it.
Brian Smith 11 years ago
parent
commit
760fc7584f
5 changed files with 18 additions and 30 deletions
  1. 4 4
      cell_test.go
  2. 3 4
      sheet.go
  3. 1 1
      style.go
  4. 1 1
      style_test.go
  5. 9 20
      xmlStyle.go

+ 4 - 4
cell_test.go

@@ -43,7 +43,7 @@ func (s *CellSuite) TestSetStyleWithFonts(c *C) {
 	style.Font = *font
 	style.Font = *font
 	cell.SetStyle(style)
 	cell.SetStyle(style)
 	style = cell.GetStyle()
 	style = cell.GetStyle()
-	_, xFont, _, _, _, _ := style.makeXLSXStyleElements()
+	xFont, _, _, _, _ := style.makeXLSXStyleElements()
 	c.Assert(xFont.Sz.Val, Equals, "12")
 	c.Assert(xFont.Sz.Val, Equals, "12")
 	c.Assert(xFont.Name.Val, Equals, "Calibra")
 	c.Assert(xFont.Name.Val, Equals, "Calibra")
 }
 }
@@ -55,7 +55,7 @@ func (s *CellSuite) TestGetStyleWithFills(c *C) {
 	style.Fill = fill
 	style.Fill = fill
 	cell := &Cell{Value: "123", style: style}
 	cell := &Cell{Value: "123", style: style}
 	style = cell.GetStyle()
 	style = cell.GetStyle()
-	_, _, xFill, _, _, _ := style.makeXLSXStyleElements()
+	_, xFill, _, _, _ := style.makeXLSXStyleElements()
 	c.Assert(xFill.PatternFill.PatternType, Equals, "solid")
 	c.Assert(xFill.PatternFill.PatternType, Equals, "solid")
 	c.Assert(xFill.PatternFill.BgColor.RGB, Equals, "00FF0000")
 	c.Assert(xFill.PatternFill.BgColor.RGB, Equals, "00FF0000")
 	c.Assert(xFill.PatternFill.FgColor.RGB, Equals, "FF000000")
 	c.Assert(xFill.PatternFill.FgColor.RGB, Equals, "FF000000")
@@ -72,7 +72,7 @@ func (s *CellSuite) TestSetStyleWithFills(c *C) {
 	style.Fill = *fill
 	style.Fill = *fill
 	cell.SetStyle(style)
 	cell.SetStyle(style)
 	style = cell.GetStyle()
 	style = cell.GetStyle()
-	_, _, xFill, _, _, _ := style.makeXLSXStyleElements()
+	_, xFill, _, _, _ := style.makeXLSXStyleElements()
 	xPatternFill := xFill.PatternFill
 	xPatternFill := xFill.PatternFill
 	c.Assert(xPatternFill.PatternType, Equals, "solid")
 	c.Assert(xPatternFill.PatternType, Equals, "solid")
 	c.Assert(xPatternFill.FgColor.RGB, Equals, "00FF0000")
 	c.Assert(xPatternFill.FgColor.RGB, Equals, "00FF0000")
@@ -86,7 +86,7 @@ func (s *CellSuite) TestGetStyleWithBorders(c *C) {
 	style.Border = border
 	style.Border = border
 	cell := Cell{Value: "123", style: style}
 	cell := Cell{Value: "123", style: style}
 	style = cell.GetStyle()
 	style = cell.GetStyle()
-	_, _, _, xBorder, _, _ := style.makeXLSXStyleElements()
+	_, _, xBorder, _, _ := style.makeXLSXStyleElements()
 	c.Assert(xBorder.Left.Style, Equals, "thin")
 	c.Assert(xBorder.Left.Style, Equals, "thin")
 	c.Assert(xBorder.Right.Style, Equals, "thin")
 	c.Assert(xBorder.Right.Style, Equals, "thin")
 	c.Assert(xBorder.Top.Style, Equals, "thin")
 	c.Assert(xBorder.Top.Style, Equals, "thin")

+ 3 - 4
sheet.go

@@ -75,19 +75,18 @@ func (s *Sheet) makeXLSXSheet(refTable *RefTable, styles *xlsxStyleSheet) *xlsxW
 		for c, cell := range row.Cells {
 		for c, cell := range row.Cells {
 			style := cell.GetStyle()
 			style := cell.GetStyle()
 			if style != nil {
 			if style != nil {
-			xNumFmt, xFont, xFill, xBorder, xCellStyleXf, xCellXf := style.makeXLSXStyleElements()
+			xFont, xFill, xBorder, xCellStyleXf, xCellXf := style.makeXLSXStyleElements()
 			fontId := styles.addFont(xFont)
 			fontId := styles.addFont(xFont)
 			fillId := styles.addFill(xFill)
 			fillId := styles.addFill(xFill)
 			borderId := styles.addBorder(xBorder)
 			borderId := styles.addBorder(xBorder)
-			styles.addNumFmt(xNumFmt)
 			xCellStyleXf.FontId = fontId
 			xCellStyleXf.FontId = fontId
 			xCellStyleXf.FillId = fillId
 			xCellStyleXf.FillId = fillId
 			xCellStyleXf.BorderId = borderId
 			xCellStyleXf.BorderId = borderId
-			xCellStyleXf.NumFmtId = xNumFmt.NumFmtId
+			xCellStyleXf.NumFmtId = 0 // General
 			xCellXf.FontId = fontId
 			xCellXf.FontId = fontId
 			xCellXf.FillId = fillId
 			xCellXf.FillId = fillId
 			xCellXf.BorderId = borderId
 			xCellXf.BorderId = borderId
-			xCellXf.NumFmtId = xNumFmt.NumFmtId
+			xCellXf.NumFmtId = 0 // General
 			styles.addCellStyleXf(xCellStyleXf)
 			styles.addCellStyleXf(xCellStyleXf)
 			XfId = styles.addCellXf(xCellXf)
 			XfId = styles.addCellXf(xCellXf)
 			}
 			}

+ 1 - 1
style.go

@@ -19,7 +19,7 @@ func NewStyle() *Style {
 }
 }
 
 
 // Generate the underlying XLSX style elements that correspond to the Style.
 // Generate the underlying XLSX style elements that correspond to the Style.
-func (style *Style) makeXLSXStyleElements() (xNumFmt xlsxNumFmt, xFont xlsxFont, xFill xlsxFill, xBorder xlsxBorder, xCellStyleXf xlsxXf, xCellXf xlsxXf) {
+func (style *Style) makeXLSXStyleElements() (xFont xlsxFont, xFill xlsxFill, xBorder xlsxBorder, xCellStyleXf xlsxXf, xCellXf xlsxXf) {
 	xFont = xlsxFont{}
 	xFont = xlsxFont{}
 	xFill = xlsxFill{}
 	xFill = xlsxFill{}
 	xBorder = xlsxBorder{}
 	xBorder = xlsxBorder{}

+ 1 - 1
style_test.go

@@ -25,7 +25,7 @@ func (s *StyleSuite) TestMakeXLSXStyleElements(c *C) {
 	style.ApplyFill = true
 	style.ApplyFill = true
 
 
 	style.ApplyFont = true
 	style.ApplyFont = true
-	_, xFont, xFill, xBorder, xCellStyleXf, xCellXf := style.makeXLSXStyleElements()
+	xFont, xFill, xBorder, xCellStyleXf, xCellXf := style.makeXLSXStyleElements()
 	// c.Assert(xNumFmt.NumFmtId, Equals, 164)
 	// c.Assert(xNumFmt.NumFmtId, Equals, 164)
 	// c.Assert(xNumFmt.FormatCode, Equals, "GENERAL")
 	// c.Assert(xNumFmt.FormatCode, Equals, "GENERAL")
 	c.Assert(xFont.Sz.Val, Equals, "12")
 	c.Assert(xFont.Sz.Val, Equals, "12")

+ 9 - 20
xmlStyle.go

@@ -15,10 +15,6 @@ import (
 	"sync"
 	"sync"
 )
 )
 
 
-var (
-	NumFmtRefTable map[int]xlsxNumFmt
-)
-
 // xlsxStyle directly maps the styleSheet element in the namespace
 // xlsxStyle directly maps the styleSheet element in the namespace
 // http://schemas.openxmlformats.org/spreadsheetml/2006/main -
 // http://schemas.openxmlformats.org/spreadsheetml/2006/main -
 // currently I have not checked it for completeness - it does as much
 // currently I have not checked it for completeness - it does as much
@@ -34,6 +30,7 @@ type xlsxStyleSheet struct {
 	NumFmts      xlsxNumFmts      `xml:"numFmts,omitempty"`
 	NumFmts      xlsxNumFmts      `xml:"numFmts,omitempty"`
 
 
 	styleCache map[int]*Style // `-`
 	styleCache map[int]*Style // `-`
+	numFmtRefTable map[int]xlsxNumFmt `xml:"-"`
 	lock       *sync.RWMutex
 	lock       *sync.RWMutex
 }
 }
 
 
@@ -44,14 +41,6 @@ func newXlsxStyleSheet() *xlsxStyleSheet {
 	return stylesheet
 	return stylesheet
 }
 }
 
 
-func (styles *xlsxStyleSheet) buildNumFmtRefTable() (numFmtRefTable map[int]xlsxNumFmt) {
-	numFmtRefTable = make(map[int]xlsxNumFmt)
-	for _, numFmt := range styles.NumFmts.NumFmt {
-		numFmtRefTable[numFmt.NumFmtId] = numFmt
-	}
-	return numFmtRefTable
-}
-
 func (styles *xlsxStyleSheet) reset() {
 func (styles *xlsxStyleSheet) reset() {
 	styles.Fonts = xlsxFonts{}
 	styles.Fonts = xlsxFonts{}
 	styles.Fills = xlsxFills{}
 	styles.Fills = xlsxFills{}
@@ -122,13 +111,13 @@ func (styles *xlsxStyleSheet) getStyle(styleIndex int) (style *Style) {
 }
 }
 
 
 func (styles *xlsxStyleSheet) getNumberFormat(styleIndex int) string {
 func (styles *xlsxStyleSheet) getNumberFormat(styleIndex int) string {
-	if styles.CellXfs.Xf == nil {
+	if styles.CellXfs.Xf == nil || styles.numFmtRefTable == nil {
 		return ""
 		return ""
 	}
 	}
 	var numberFormat string = ""
 	var numberFormat string = ""
 	if styleIndex > -1 && styleIndex <= styles.CellXfs.Count {
 	if styleIndex > -1 && styleIndex <= styles.CellXfs.Count {
 		xf := styles.CellXfs.Xf[styleIndex]
 		xf := styles.CellXfs.Xf[styleIndex]
-		numFmt := NumFmtRefTable[xf.NumFmtId]
+		numFmt := styles.numFmtRefTable[xf.NumFmtId]
 		numberFormat = numFmt.FormatCode
 		numberFormat = numFmt.FormatCode
 	}
 	}
 	return strings.ToLower(numberFormat)
 	return strings.ToLower(numberFormat)
@@ -204,13 +193,13 @@ func (styles *xlsxStyleSheet) addCellXf(xCellXf xlsxXf) (index int) {
 }
 }
 
 
 func (styles *xlsxStyleSheet) addNumFmt(xNumFmt xlsxNumFmt) (index int) {
 func (styles *xlsxStyleSheet) addNumFmt(xNumFmt xlsxNumFmt) (index int) {
-	numFmt, ok := NumFmtRefTable[xNumFmt.NumFmtId]
+	numFmt, ok := styles.numFmtRefTable[xNumFmt.NumFmtId]
 	if !ok {
 	if !ok {
-		if NumFmtRefTable == nil {
-			NumFmtRefTable = make(map[int]xlsxNumFmt)
+		if styles.numFmtRefTable == nil {
+			styles.numFmtRefTable = make(map[int]xlsxNumFmt)
 		}
 		}
 		styles.NumFmts.NumFmt = append(styles.NumFmts.NumFmt, xNumFmt)
 		styles.NumFmts.NumFmt = append(styles.NumFmts.NumFmt, xNumFmt)
-		NumFmtRefTable[xNumFmt.NumFmtId] = xNumFmt
+		styles.numFmtRefTable[xNumFmt.NumFmtId] = xNumFmt
 		index = styles.NumFmts.Count
 		index = styles.NumFmts.Count
 		styles.NumFmts.Count += 1
 		styles.NumFmts.Count += 1
 		return
 		return
@@ -304,8 +293,8 @@ func (numFmts *xlsxNumFmts) Marshal() (result string, err error) {
 // currently I have not checked it for completeness - it does as much
 // currently I have not checked it for completeness - it does as much
 // as I need.
 // as I need.
 type xlsxNumFmt struct {
 type xlsxNumFmt struct {
-	NumFmtId   int    `xml:"numFmtId,omitempty"`
-	FormatCode string `xml:"formatCode,omitempty"`
+	NumFmtId   int    `xml:"numFmtId,attr,omitempty"`
+	FormatCode string `xml:"formatCode,attr,omitempty"`
 }
 }
 
 
 func (numFmt *xlsxNumFmt) Marshal() (result string, err error) {
 func (numFmt *xlsxNumFmt) Marshal() (result string, err error) {