From 61030d831958583cc26580c189792301874e5f51 Mon Sep 17 00:00:00 2001 From: Beta Kuang Date: Thu, 12 Sep 2024 18:40:28 +0800 Subject: [PATCH 1/4] fix: wrong cell & border colors using indexedColors --- xmlStyle.go | 19 ++++++++++--------- xmlStyle_test.go | 4 ++-- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/xmlStyle.go b/xmlStyle.go index 5636b285..e7cb118e 100644 --- a/xmlStyle.go +++ b/xmlStyle.go @@ -237,16 +237,15 @@ func (styles *xlsxStyleSheet) populateStyleFromXf(style *Style, xf xlsxXf) { style.ApplyAlignment = xf.ApplyAlignment if xf.BorderId > -1 && xf.BorderId < styles.Borders.Count { - var border xlsxBorder - border = styles.Borders.Border[xf.BorderId] + border := styles.Borders.Border[xf.BorderId] style.Border.Left = border.Left.Style - style.Border.LeftColor = border.Left.Color.RGB + style.Border.LeftColor = styles.argbValue(border.Left.Color) style.Border.Right = border.Right.Style - style.Border.RightColor = border.Right.Color.RGB + style.Border.RightColor = styles.argbValue(border.Right.Color) style.Border.Top = border.Top.Style - style.Border.TopColor = border.Top.Color.RGB + style.Border.TopColor = styles.argbValue(border.Top.Color) style.Border.Bottom = border.Bottom.Style - style.Border.BottomColor = border.Bottom.Color.RGB + style.Border.BottomColor = styles.argbValue(border.Bottom.Color) } if xf.FillId > -1 && xf.FillId < styles.Fills.Count { @@ -1150,13 +1149,15 @@ type xlsxColors struct { MruColors []xlsxColor `xml:"mruColors>color,omitempty"` } +// indexerdColor returns ARGB color string for the given index of the IndexedColors. +// Indexes start from 0, see section 18.8.27 of ECMA-376 (part 1, 4th edition). func (c *xlsxColors) indexedColor(index int) string { if c.IndexedColors != nil { - return c.IndexedColors[index-1].Rgb + return c.IndexedColors[index].Rgb } else { - if index < 1 || index > 64 { + if index < 0 || index > 63 { return "" } - return xlsxIndexedColors[index-1] + return xlsxIndexedColors[index] } } diff --git a/xmlStyle_test.go b/xmlStyle_test.go index a7d5115b..1363cd1b 100644 --- a/xmlStyle_test.go +++ b/xmlStyle_test.go @@ -12,12 +12,12 @@ func TestIndexedColor(t *testing.T) { colors := xlsxColors{} c.Run("Unitialised", func(c *qt.C) { - c.Assert(colors.indexedColor(1), qt.Equals, "FF000000") + c.Assert(colors.indexedColor(0), qt.Equals, "FF000000") }) c.Run("Initialised", func(c *qt.C) { colors.IndexedColors = []xlsxRgbColor{{Rgb: "00FF00FF"}} - c.Assert(colors.indexedColor(1), qt.Equals, "00FF00FF") + c.Assert(colors.indexedColor(0), qt.Equals, "00FF00FF") }) } From 989e3d58beb11dc967f6e226b08395713e839a16 Mon Sep 17 00:00:00 2001 From: Geoffrey Teale Date: Fri, 13 Sep 2024 06:22:08 +0200 Subject: [PATCH 2/4] Flyby channel leak fix --- lib.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/lib.go b/lib.go index dec567ee..41018dbf 100644 --- a/lib.go +++ b/lib.go @@ -810,22 +810,33 @@ func readSheetsFromZipFile(f *zip.File, file *File, sheetXMLMap map[string]strin }() } + var sb strings.Builder + errFound := false + err = nil for j := 0; j < sheetCount; j++ { sheet := <-sheetChan if sheet == nil { - // FIXME channel leak - return wrap(fmt.Errorf("No sheet returnded from readSheetFromFile")) + errFound = true + sb.WriteString("{SheetIndex: ") + sb.WriteString(strconv.Itoa(j)) + sb.WriteString("} No sheet returned from readSheetFromFile\n") } if sheet.Error != nil { - // FIXME channel leak - return wrap(sheet.Error) + errFound = true + sb.WriteString("{SheetIndex: ") + sb.WriteString(strconv.Itoa(sheet.Index)) + sb.WriteString("} ") + sb.WriteString(sheet.Error.Error()) } sheetName := sheet.Sheet.Name sheetsByName[sheetName] = sheet.Sheet sheets[sheet.Index] = sheet.Sheet } close(sheetChan) - return sheetsByName, sheets, nil + if errFound { + err = fmt.Errorf(sb.String()) + } + return sheetsByName, sheets, err } // readSharedStringsFromZipFile() is an internal helper function to From b3ec12733edeed3f295d1027f721b9bdc058a6c2 Mon Sep 17 00:00:00 2001 From: Geoffrey Teale Date: Fri, 13 Sep 2024 06:23:05 +0200 Subject: [PATCH 3/4] Fix test cases --- style_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/style_test.go b/style_test.go index ca7ed48d..752bc511 100644 --- a/style_test.go +++ b/style_test.go @@ -74,11 +74,11 @@ func TestReadCellColorBackground(t *testing.T) { cell, err = sheet.Cell(1, 1) c.Assert(err, qt.Equals, nil) style = cell.GetStyle() - c.Assert(style.Fill, qt.Equals, *NewFill("solid", "00CC99FF", "00333333")) + c.Assert(style.Fill, qt.Equals, *NewFill("solid", "00FFCC99", "")) cell, err = sheet.Cell(2, 1) c.Assert(err, qt.Equals, nil) style = cell.GetStyle() - c.Assert(style.Fill, qt.Equals, *NewFill("solid", "FF990099", "00333333")) + c.Assert(style.Fill, qt.Equals, *NewFill("solid", "FF990099", "")) }) } From 289650d35ae6c4b92ae2d6e18eb2e57e7c442954 Mon Sep 17 00:00:00 2001 From: Geoffrey Teale Date: Fri, 13 Sep 2024 06:23:37 +0200 Subject: [PATCH 4/4] Tidy up the range checking in IndexedColors --- xmlStyle.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/xmlStyle.go b/xmlStyle.go index e7cb118e..671d51b0 100644 --- a/xmlStyle.go +++ b/xmlStyle.go @@ -1152,12 +1152,18 @@ type xlsxColors struct { // indexerdColor returns ARGB color string for the given index of the IndexedColors. // Indexes start from 0, see section 18.8.27 of ECMA-376 (part 1, 4th edition). func (c *xlsxColors) indexedColor(index int) string { - if c.IndexedColors != nil { + if index < 0 { + return "" + } + + if c.IndexedColors != nil && index < len(c.IndexedColors) { return c.IndexedColors[index].Rgb - } else { - if index < 0 || index > 63 { - return "" - } + } + + // This is a weird fallback? Why would we be using indexed colours + // in a file that hasn't defined any? + if index < len(xlsxIndexedColors) { return xlsxIndexedColors[index] } + return "" }