Skip to content
This repository was archived by the owner on Aug 13, 2025. It is now read-only.

Commit 414693d

Browse files
tealegtealeg
authored
Merge pull request #578 from tealeg/issue-574
Make hyperllink loading non destructive
2 parents 04a94a3 + 2962baa commit 414693d

File tree

3 files changed

+134
-53
lines changed

3 files changed

+134
-53
lines changed

‎lib.go‎

Lines changed: 72 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ func fillCellDataFromInlineString(rawcell xlsxC, cell *Cell) {
460460
// rows from a XSLXWorksheet, populates them with Cells and resolves
461461
// the value references from the reference table and stores them in
462462
// the rows and columns.
463-
func readRowsFromSheet(Worksheet *xlsxWorksheet, file *File, sheet *Sheet, rowLimit int) error {
463+
func readRowsFromSheet(Worksheet *xlsxWorksheet, file *File, sheet *Sheet, rowLimit int, linkTable hyperlinkTable) error {
464464
var row *Row
465465
var maxCol, maxRow, colCount, rowCount int
466466
var reftable *RefTable
@@ -543,7 +543,7 @@ func readRowsFromSheet(Worksheet *xlsxWorksheet, file *File, sheet *Sheet, rowLi
543543
if err != nil {
544544
return wrap(err)
545545
}
546-
x, _, err := GetCoordsFromCellIDString(rawcell.R)
546+
x, y, err := GetCoordsFromCellIDString(rawcell.R)
547547
if err != nil {
548548
return wrap(err)
549549
}
@@ -559,6 +559,11 @@ func readRowsFromSheet(Worksheet *xlsxWorksheet, file *File, sheet *Sheet, rowLi
559559
cell.NumFmt, cell.parsedNumFmt = file.styles.getNumberFormat(rawcell.S)
560560
}
561561
cell.date1904 = file.Date1904
562+
563+
if hyperlink, found := linkTable[coord{x: x, y: y}]; found {
564+
cell.Hyperlink = hyperlink
565+
}
566+
562567
// Cell is considered hidden if the row or the column of this cell is hidden
563568
col := sheet.Cols.FindColByIndex(cellX + 1)
564569
cell.Hidden = rawrow.Hidden || (col != nil && col.Hidden != nil && *col.Hidden)
@@ -613,43 +618,19 @@ func readSheetViews(xSheetViews xlsxSheetViews) []SheetView {
613618
return sheetViews
614619
}
615620

616-
// readSheetFromFile is the logic of converting a xlsxSheet struct
617-
// into a Sheet struct. This work can be done in parallel and so
618-
// readSheetsFromZipFile will spawn an instance of this function per
619-
// sheet and get the results back on the provided channel.
620-
func readSheetFromFile(rsheet xlsxSheet, fi *File, sheetXMLMap map[string]string, rowLimit int) (sheet *Sheet, errRes error) {
621-
defer func() {
622-
if x := recover(); x != nil {
623-
errRes = errors.New(fmt.Sprint(x))
624-
}
625-
}()
626-
627-
wrap := func(err error) (*Sheet, error) {
628-
return nil, fmt.Errorf("readSheetFromFile: %w", err)
629-
}
630-
631-
worksheet, err := getWorksheetFromSheet(rsheet, fi.worksheets, sheetXMLMap, rowLimit)
632-
if err != nil {
633-
return wrap(err)
634-
}
621+
type coord struct {
622+
x int
623+
y int
624+
}
635625

636-
sheet, err = NewSheetWithCellStore(rsheet.Name, fi.cellStoreConstructor)
637-
if err != nil {
638-
return wrap(err)
639-
}
626+
type hyperlinkTable map[coord]Hyperlink
640627

641-
sheet.File = fi
642-
err = readRowsFromSheet(worksheet, fi, sheet, rowLimit)
643-
if err != nil {
644-
return wrap(err)
628+
func makeHyperlinkTable(worksheet *xlsxWorksheet, fi *File, rsheet *xlsxSheet) (hyperlinkTable, error) {
629+
wrap := func(err error) (hyperlinkTable, error) {
630+
return nil, fmt.Errorf("makeHyperlinkTable: %w", err)
645631
}
646632

647-
sheet.Hidden = rsheet.State == sheetStateHidden || rsheet.State == sheetStateVeryHidden
648-
sheet.SheetViews = readSheetViews(worksheet.SheetViews)
649-
if worksheet.AutoFilter != nil {
650-
autoFilterBounds := strings.Split(worksheet.AutoFilter.Ref, ":")
651-
sheet.AutoFilter = &AutoFilter{autoFilterBounds[0], autoFilterBounds[1]}
652-
}
633+
table := make(hyperlinkTable)
653634

654635
// Convert xlsxHyperlinks to Hyperlinks
655636
if worksheet.Hyperlinks != nil {
@@ -688,13 +669,63 @@ func readSheetFromFile(rsheet xlsxSheet, fi *File, sheetXMLMap map[string]string
688669
if err != nil {
689670
return wrap(err)
690671
}
691-
row, err := sheet.Row(y)
692-
if err != nil {
693-
return wrap(err)
694-
}
695-
cell := row.GetCell(x)
696-
cell.Hyperlink = newHyperLink
672+
table[coord{x: x, y: y}] = newHyperLink
697673
}
674+
675+
// row, err := sheet.Row(y)
676+
// if err != nil {
677+
// return wrap(err)
678+
// }
679+
// fmt.Printf("%d, %d, %+v\n", x, y, row)
680+
681+
// // cell := row.GetCell(x)
682+
// // cell.Hyperlink = newHyperLink
683+
// }
684+
}
685+
return table, nil
686+
}
687+
688+
// readSheetFromFile is the logic of converting a xlsxSheet struct
689+
// into a Sheet struct. This work can be done in parallel and so
690+
// readSheetsFromZipFile will spawn an instance of this function per
691+
// sheet and get the results back on the provided channel.
692+
func readSheetFromFile(rsheet xlsxSheet, fi *File, sheetXMLMap map[string]string, rowLimit int) (sheet *Sheet, errRes error) {
693+
defer func() {
694+
if x := recover(); x != nil {
695+
errRes = errors.New(fmt.Sprint(x))
696+
}
697+
}()
698+
699+
wrap := func(err error) (*Sheet, error) {
700+
return nil, fmt.Errorf("readSheetFromFile: %w", err)
701+
}
702+
703+
worksheet, err := getWorksheetFromSheet(rsheet, fi.worksheets, sheetXMLMap, rowLimit)
704+
if err != nil {
705+
return wrap(err)
706+
}
707+
708+
linkTable, err := makeHyperlinkTable(worksheet, fi, &rsheet)
709+
if err != nil {
710+
return wrap(err)
711+
}
712+
713+
sheet, err = NewSheetWithCellStore(rsheet.Name, fi.cellStoreConstructor)
714+
if err != nil {
715+
return wrap(err)
716+
}
717+
718+
sheet.File = fi
719+
err = readRowsFromSheet(worksheet, fi, sheet, rowLimit, linkTable)
720+
if err != nil {
721+
return wrap(err)
722+
}
723+
724+
sheet.Hidden = rsheet.State == sheetStateHidden || rsheet.State == sheetStateVeryHidden
725+
sheet.SheetViews = readSheetViews(worksheet.SheetViews)
726+
if worksheet.AutoFilter != nil {
727+
autoFilterBounds := strings.Split(worksheet.AutoFilter.Ref, ":")
728+
sheet.AutoFilter = &AutoFilter{autoFilterBounds[0], autoFilterBounds[1]}
698729
}
699730

700731
sheet.SheetFormat.DefaultColWidth = worksheet.SheetFormatPr.DefaultColWidth

‎lib_test.go‎

Lines changed: 62 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,8 @@ func TestLib(t *testing.T) {
345345
file.referenceTable = MakeSharedStringRefTable(sst)
346346
sheet, err := NewSheet("test")
347347
c.Assert(err, qt.IsNil)
348-
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
348+
lt := make(hyperlinkTable)
349+
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
349350
c.Assert(err, qt.IsNil)
350351
c.Assert(sheet.MaxRow, qt.Equals, 2)
351352
c.Assert(sheet.MaxCol, qt.Equals, 2)
@@ -439,9 +440,12 @@ func TestLib(t *testing.T) {
439440

440441
sheet, err := NewSheetWithCellStore("test", constructor)
441442
c.Assert(err, qt.IsNil)
443+
444+
lt := make(hyperlinkTable)
445+
442446
// Discarding all return values; this test is a regression for
443447
// a panic due to an "index out of range."
444-
readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
448+
readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
445449
})
446450

447451
csRunC(c, "ReadRowsFromSheetWithLeadingEmptyRows", func(c *qt.C, constructor CellStoreConstructor) {
@@ -489,7 +493,9 @@ func TestLib(t *testing.T) {
489493
file.referenceTable = MakeSharedStringRefTable(sst)
490494
sheet, err := NewSheetWithCellStore("test", constructor)
491495
c.Assert(err, qt.IsNil)
492-
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
496+
lt := make(hyperlinkTable)
497+
498+
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
493499
c.Assert(err, qt.IsNil)
494500
c.Assert(sheet.MaxRow, qt.Equals, 5)
495501
c.Assert(sheet.MaxCol, qt.Equals, 1)
@@ -569,7 +575,9 @@ func TestLib(t *testing.T) {
569575
file.referenceTable = MakeSharedStringRefTable(sst)
570576
sheet, err := NewSheetWithCellStore("test", constructor)
571577
c.Assert(err, qt.IsNil)
572-
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
578+
lt := make(hyperlinkTable)
579+
580+
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
573581
c.Assert(err, qt.IsNil)
574582
c.Assert(sheet.MaxRow, qt.Equals, 2)
575583
c.Assert(sheet.MaxCol, qt.Equals, 4)
@@ -715,7 +723,9 @@ func TestLib(t *testing.T) {
715723
file.referenceTable = MakeSharedStringRefTable(sst)
716724
sheet, err := NewSheetWithCellStore("test", constructor)
717725
c.Assert(err, qt.IsNil)
718-
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
726+
lt := make(hyperlinkTable)
727+
728+
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
719729
c.Assert(err, qt.IsNil)
720730
c.Assert(sheet.MaxRow, qt.Equals, 3)
721731
c.Assert(sheet.MaxCol, qt.Equals, 3)
@@ -761,7 +771,9 @@ func TestLib(t *testing.T) {
761771
file.referenceTable = MakeSharedStringRefTable(sst)
762772
sheet, err := NewSheetWithCellStore("test", constructor)
763773
c.Assert(err, qt.IsNil)
764-
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
774+
lt := make(hyperlinkTable)
775+
776+
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
765777
c.Assert(err, qt.IsNil)
766778
c.Assert(sheet.MaxCol, qt.Equals, 4)
767779
c.Assert(sheet.MaxRow, qt.Equals, 8)
@@ -876,7 +888,10 @@ func TestLib(t *testing.T) {
876888
file.referenceTable = MakeSharedStringRefTable(sst)
877889
sheet, err := NewSheetWithCellStore("test", constructor)
878890
c.Assert(err, qt.IsNil)
879-
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
891+
892+
lt := make(hyperlinkTable)
893+
894+
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
880895
c.Assert(err, qt.IsNil)
881896
c.Assert(sheet.MaxRow, qt.Equals, 2)
882897
c.Assert(sheet.MaxCol, qt.Equals, 4)
@@ -955,7 +970,9 @@ func TestLib(t *testing.T) {
955970
file.referenceTable = MakeSharedStringRefTable(sst)
956971
sheet, err := NewSheetWithCellStore("test", constructor)
957972
c.Assert(err, qt.IsNil)
958-
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
973+
lt := make(hyperlinkTable)
974+
975+
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
959976
c.Assert(err, qt.IsNil)
960977
c.Assert(sheet.MaxRow, qt.Equals, 1)
961978
c.Assert(sheet.MaxCol, qt.Equals, 6)
@@ -1032,7 +1049,9 @@ func TestLib(t *testing.T) {
10321049
file.referenceTable = MakeSharedStringRefTable(sst)
10331050
sheet, err := NewSheetWithCellStore("test", constructor)
10341051
c.Assert(err, qt.IsNil)
1035-
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
1052+
lt := make(hyperlinkTable)
1053+
1054+
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
10361055
c.Assert(err, qt.IsNil)
10371056
c.Assert(sheet.MaxRow, qt.Equals, 1)
10381057
c.Assert(sheet.MaxCol, qt.Equals, 2)
@@ -1175,7 +1194,10 @@ func TestLib(t *testing.T) {
11751194
file.cellStoreConstructor = constructor
11761195
sheet, err := NewSheetWithCellStore("test", constructor)
11771196
c.Assert(err, qt.IsNil)
1178-
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
1197+
1198+
lt := make(hyperlinkTable)
1199+
1200+
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
11791201
c.Assert(err, qt.IsNil)
11801202
c.Assert(sheet.MaxCol, qt.Equals, 3)
11811203
c.Assert(sheet.MaxRow, qt.Equals, 2)
@@ -1320,7 +1342,10 @@ func TestLib(t *testing.T) {
13201342

13211343
sheet, err := NewSheetWithCellStore("test", constructor)
13221344
c.Assert(err, qt.IsNil)
1323-
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
1345+
1346+
lt := make(hyperlinkTable)
1347+
1348+
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
13241349
c.Assert(err, qt.IsNil)
13251350
row, err := sheet.Row(3)
13261351
c.Assert(err, qt.Equals, nil)
@@ -1414,7 +1439,8 @@ func TestReadRowsFromSheet(t *testing.T) {
14141439
worksheet.mapMergeCells()
14151440
sheet, err := NewSheetWithCellStore("test", constructor)
14161441
c.Assert(err, qt.IsNil)
1417-
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit)
1442+
lt := make(hyperlinkTable)
1443+
err = readRowsFromSheet(worksheet, file, sheet, NoRowLimit, lt)
14181444
c.Assert(err, qt.IsNil)
14191445
row, err := sheet.Row(0)
14201446
c.Assert(err, qt.Equals, nil)
@@ -1772,3 +1798,27 @@ func TestGrowRowCellSliceDuringFileLoad(t *testing.T) {
17721798
c.Assert(err, qt.Equals, nil)
17731799
})
17741800
}
1801+
1802+
func TestIssueSheetsWithHyperlinksHaveLegibleValues(t *testing.T) {
1803+
c := qt.New(t)
1804+
1805+
// Issue 574 concerned a sheet with cell values that
1806+
// incorrectly showed up blank. This issue was caused by
1807+
// mutable state being abused during the data load
1808+
// (essentially using Sheet.Row(n) before the sheet was fully
1809+
// loaded. The file: testdocs/issue574.xlsx illustrates this issue.
1810+
f, err := OpenFile("testdocs/issue574.xlsx")
1811+
c.Assert(err, qt.Equals, nil)
1812+
1813+
sheet, ok := f.Sheet["Sheet1"]
1814+
c.Assert(ok, qt.Equals, true)
1815+
c.Assert(sheet.MaxRow, qt.Equals, 4)
1816+
sheet.ForEachRow(func(r *Row) error {
1817+
r.ForEachCell(func(cell *Cell) error {
1818+
c.Assert(cell, qt.Not(qt.IsNil))
1819+
c.Assert(cell.Value, qt.Not(qt.Equals), "")
1820+
return nil
1821+
})
1822+
return nil
1823+
})
1824+
}

‎testdocs/issue574.xlsx‎

10.5 KB
Binary file not shown.

0 commit comments

Comments
 (0)