Skip to content

Commit

Permalink
spreadsheet: fix for sheet ordering bug
Browse files Browse the repository at this point in the history
Need to test this more, may be working by chance...

Updates unidoc#154
  • Loading branch information
tbaliance committed Feb 6, 2018
1 parent df08d55 commit 6525623
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 0 deletions.
63 changes: 63 additions & 0 deletions algo/naturalsort.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2017 Baliance. All rights reserved.
//
// Use of this source code is governed by the terms of the Affero GNU General
// Public License version 3.0 as published by the Free Software Foundation and
// appearing in the file LICENSE included in the packaging of this file. A
// commercial license can be purchased by contacting [email protected].

package algo

import (
"strconv"
)

func isdigit(c byte) bool {
return c >= '0' && c <= '9'
}

// NaturalLess compares two strings in a human manner so rId2 sorts less than rId10
func NaturalLess(lhs, rhs string) bool {
lidx, ridx := 0, 0
for lidx < len(lhs) && ridx < len(rhs) {
lc := lhs[lidx]
rc := rhs[ridx]
ldigit := isdigit(lc)
rdigit := isdigit(rc)
switch {
// digits sort before characters
case ldigit && !rdigit:
return true
// characters after digits
case !ldigit && rdigit:
return false
// no digits, so compare the characters
case !ldigit && !rdigit:
if lc != rc {
return lc < rc
}
lidx++
ridx++
// both digits, so parse and compare
default:
lend := lidx + 1
rend := ridx + 1

for lend < len(lhs) && isdigit(lhs[lend]) {
lend++
}
for rend < len(rhs) && isdigit(rhs[rend]) {
rend++
}
lv, _ := strconv.ParseUint(lhs[lidx:lend], 10, 64)
rv, _ := strconv.ParseUint(rhs[lidx:rend], 10, 64)
if lv != rv {
return lv < rv
}
// digits are equal, so keep looking
lidx = lend
ridx = rend
}
}
// fall back to comparing length
return len(lhs) < len(rhs)
}
41 changes: 41 additions & 0 deletions algo/naturalsort_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2017 Baliance. All rights reserved.
//
// Use of this source code is governed by the terms of the Affero GNU General
// Public License version 3.0 as published by the Free Software Foundation and
// appearing in the file LICENSE included in the packaging of this file. A
// commercial license can be purchased by contacting [email protected].

package algo_test

import (
"testing"

"baliance.com/gooxml/algo"
)

func TestSort(t *testing.T) {

tests := []struct {
a, b string
}{
{"rId1", "rId2"},
{"rId1", "rId10"},
{"rId2", "rId10"},
{"rId5", "rId10"},
{"rId5", "rId15"},
{"rId5", "rId51"},
{"rId1a", "rId1b"},
}

for _, tc := range tests {
if !algo.NaturalLess(tc.a, tc.b) {
t.Errorf("bad sort, expected %s < %s", tc.a, tc.b)
} else {
// no need to check if it failed the first time
if algo.NaturalLess(tc.b, tc.a) {
t.Errorf("bad sort, expected %s > %s", tc.b, tc.a)
}
}
}

}
Binary file added spreadsheet/testdata/ordered-sheets.xlsx
Binary file not shown.
21 changes: 21 additions & 0 deletions spreadsheet/workbook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,3 +269,24 @@ func TestSheetGetName(t *testing.T) {
t.Errorf("expected no error")
}
}

// TestOpenOrderedSheets test for issue #154 where sheet title didn't match sheet content.
func TestOpenOrderedSheets(t *testing.T) {
wb, err := spreadsheet.Open("./testdata/ordered-sheets.xlsx")
defer wb.Close()
if err != nil {
t.Fatalf("error opening workbook: %s", err)
}

for i, sheet := range wb.Sheets() {
expContent := fmt.Sprintf("%d", i+1)
if sheet.Name() != expContent {
t.Errorf("expected sheet %d name = %s, got %s", i, expContent, sheet.Name())
}
got := sheet.Cell("A1").GetFormattedValue()
if got != expContent {
t.Errorf("expected sheet %d cell A1 = %s, got %s", i, expContent, got)
}
}

}
14 changes: 14 additions & 0 deletions zippkg/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ import (
"io"
"io/ioutil"
"os"
"sort"
"strings"

"baliance.com/gooxml/algo"
"baliance.com/gooxml/schema/soo/pkg/relationships"
)

// RelationsPathFor returns the relations path for a given filename.
Expand All @@ -38,6 +42,16 @@ func Decode(f *zip.File, dest interface{}) error {
if err := dec.Decode(dest); err != nil {
return fmt.Errorf("error decoding %s: %s", f.Name, err)
}

// this ensures that relationship ID is increasing, which we apparently rely
// on....
if ds, ok := dest.(*relationships.Relationships); ok {
sort.Slice(ds.Relationship, func(i, j int) bool {
lhs := ds.Relationship[i]
rhs := ds.Relationship[j]
return algo.NaturalLess(lhs.IdAttr, rhs.IdAttr)
})
}
return nil
}

Expand Down

0 comments on commit 6525623

Please sign in to comment.