Skip to content

Commit 0192dc6

Browse files
SamTV12345claude
andauthored
feat(sheet): round-trip styles, dimensions and freeze panes through xlsx (#351)
lib/xlsx was still v1 (values + formulas only) from PR #306, while the sheet model has since gained style props (M2), col/row dimensions and freeze panes (M4) and font props (#328). Export now writes cell styles, column widths, row heights and frozen first row/col; Import maps them back into the allowlisted prop vocabulary, interning into the StylePool. Imported props pass through sheet.ValidateProps (now exported) as the trust-boundary gate, since uploaded files bypass Op.Validate. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 65ae9d1 commit 0192dc6

5 files changed

Lines changed: 500 additions & 14 deletions

File tree

lib/sheet/op.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func (o Op) Validate() error {
9090
if o.Row < 0 || o.Col < 0 {
9191
return fmt.Errorf("setCell negative coord")
9292
}
93-
if err := validateProps(o.Props); err != nil {
93+
if err := ValidateProps(o.Props); err != nil {
9494
return err
9595
}
9696
case OpSetStyle:
@@ -100,7 +100,7 @@ func (o Op) Validate() error {
100100
if o.Row < 0 || o.Col < 0 {
101101
return fmt.Errorf("setStyle negative coord")
102102
}
103-
if err := validateProps(o.Props); err != nil {
103+
if err := ValidateProps(o.Props); err != nil {
104104
return err
105105
}
106106
case OpClearRange:
@@ -161,10 +161,10 @@ var fontFamilies = map[string]bool{
161161
"Courier New": true, "Georgia": true, "Verdana": true,
162162
}
163163

164-
// validateProps allowlists style prop keys and values. Props come from
164+
// ValidateProps allowlists style prop keys and values. Props come from
165165
// arbitrary collaborators and end up as inline CSS on every viewer's DOM, so
166166
// anything outside the known vocabulary is rejected (e.g. bg: "url(...)").
167-
func validateProps(props map[string]string) error {
167+
func ValidateProps(props map[string]string) error {
168168
for k, v := range props {
169169
ok := false
170170
switch k {

lib/xlsx/export.go

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,33 @@ import (
1010

1111
// Export renders a workbook to .xlsx bytes. Cells whose raw starts with '='
1212
// become formulas; numeric-looking raw is written as a number, the rest as a
13-
// string. Styles and merges are out of scope for v1 (the sheet model does not
14-
// store them yet).
13+
// string. Cell styles, column widths / row heights and freeze panes are
14+
// carried over; merges are not (the sheet model does not store them).
1515
func Export(wb *sheet.Workbook) ([]byte, error) {
1616
f := excelize.NewFile()
1717
defer f.Close()
1818

1919
const defaultSheet = "Sheet1"
2020

21+
// Lazily translate pool style ids to excelize style ids (shared per file).
22+
styleIds := map[int]int{}
23+
styleFor := func(id int) (int, error) {
24+
if xid, ok := styleIds[id]; ok {
25+
return xid, nil
26+
}
27+
st, ok := wb.Styles.Get(id)
28+
if !ok || len(st.Props) == 0 {
29+
styleIds[id] = 0
30+
return 0, nil
31+
}
32+
xid, err := f.NewStyle(propsToStyle(st.Props))
33+
if err != nil {
34+
return 0, err
35+
}
36+
styleIds[id] = xid
37+
return xid, nil
38+
}
39+
2140
for i, s := range wb.Sheets {
2241
name := s.Name
2342
if name == "" {
@@ -44,13 +63,52 @@ func Export(wb *sheet.Workbook) ([]byte, error) {
4463
if err := f.SetCellFormula(name, axis, cell.Raw[1:]); err != nil {
4564
return nil, err
4665
}
47-
continue
48-
}
49-
if n, err := strconv.ParseFloat(cell.Raw, 64); err == nil {
66+
} else if n, err := strconv.ParseFloat(cell.Raw, 64); err == nil && cell.Raw != "" {
5067
if err := f.SetCellValue(name, axis, n); err != nil {
5168
return nil, err
5269
}
53-
} else if err := f.SetCellValue(name, axis, cell.Raw); err != nil {
70+
} else if cell.Raw != "" {
71+
if err := f.SetCellValue(name, axis, cell.Raw); err != nil {
72+
return nil, err
73+
}
74+
}
75+
if cell.StyleId != 0 {
76+
xid, err := styleFor(cell.StyleId)
77+
if err != nil {
78+
return nil, err
79+
}
80+
if xid != 0 {
81+
if err := f.SetCellStyle(name, axis, axis, xid); err != nil {
82+
return nil, err
83+
}
84+
}
85+
}
86+
}
87+
88+
for col, px := range s.ColWidths {
89+
cn, err := excelize.ColumnNumberToName(col + 1)
90+
if err != nil {
91+
return nil, err
92+
}
93+
if err := f.SetColWidth(name, cn, cn, pxToColWidth(px)); err != nil {
94+
return nil, err
95+
}
96+
}
97+
for row, px := range s.RowHeights {
98+
if err := f.SetRowHeight(name, row+1, pxToRowHeight(px)); err != nil {
99+
return nil, err
100+
}
101+
}
102+
103+
if s.FrozenRows > 0 || s.FrozenCols > 0 {
104+
topLeft, err := excelize.CoordinatesToCellName(s.FrozenCols+1, s.FrozenRows+1)
105+
if err != nil {
106+
return nil, err
107+
}
108+
if err := f.SetPanes(name, &excelize.Panes{
109+
Freeze: true, XSplit: s.FrozenCols, YSplit: s.FrozenRows,
110+
TopLeftCell: topLeft, ActivePane: "bottomRight",
111+
}); err != nil {
54112
return nil, err
55113
}
56114
}

lib/xlsx/import.go

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,22 @@ package xlsx
22

33
import (
44
"io"
5+
"math"
56

67
"github.com/ether/etherpad-go/lib/sheet"
78
"github.com/xuri/excelize/v2"
89
)
910

11+
// The client grid is fixed at 200x52; dimension scans stop there.
12+
const (
13+
maxRows = 200
14+
maxCols = 52
15+
)
16+
1017
// Import parses an .xlsx into a WorkbookSnapshot. Sheet id == sheet name. Cells
11-
// carry their raw value, or "=<formula>" when a formula is present. Styles and
12-
// merges are skipped in v1 (no error; the sheet model does not store them yet).
18+
// carry their raw value, or "=<formula>" when a formula is present. Cell styles
19+
// (allowlisted props only), column widths / row heights and freeze panes are
20+
// imported; merges are skipped (the sheet model does not store them).
1321
func Import(r io.Reader) (sheet.WorkbookSnapshot, error) {
1422
f, err := excelize.OpenReader(r)
1523
if err != nil {
@@ -18,6 +26,23 @@ func Import(r io.Reader) (sheet.WorkbookSnapshot, error) {
1826
defer f.Close()
1927

2028
wb := sheet.NewWorkbook()
29+
// excelize style idx -> pool id (0 = nothing representable), shared across
30+
// sheets since styles are file-global.
31+
poolIds := map[int]int{}
32+
poolIdFor := func(xid int) int {
33+
if id, ok := poolIds[xid]; ok {
34+
return id
35+
}
36+
id := 0
37+
if st, err := f.GetStyle(xid); err == nil && st != nil {
38+
if props := styleToProps(st); len(props) > 0 {
39+
id = wb.Styles.Put(sheet.Style{Props: props})
40+
}
41+
}
42+
poolIds[xid] = id
43+
return id
44+
}
45+
2146
for _, name := range f.GetSheetList() {
2247
sh := wb.AddSheet(name, name)
2348
rows, err := f.GetRows(name)
@@ -34,10 +59,60 @@ func Import(r io.Reader) (sheet.WorkbookSnapshot, error) {
3459
if formula, ferr := f.GetCellFormula(name, axis); ferr == nil && formula != "" {
3560
raw = "=" + formula
3661
}
37-
if raw == "" {
62+
styleId := 0
63+
if xid, serr := f.GetCellStyle(name, axis); serr == nil && xid != 0 {
64+
styleId = poolIdFor(xid)
65+
}
66+
if raw == "" && styleId == 0 {
3867
continue
3968
}
40-
sh.SetCell(sheet.CellRef{Row: rIdx, Col: cIdx}, sheet.Cell{Raw: raw})
69+
sh.SetCell(sheet.CellRef{Row: rIdx, Col: cIdx}, sheet.Cell{Raw: raw, StyleId: styleId})
70+
}
71+
}
72+
73+
// Styled-but-empty cells never show up in GetRows, and excelize does
74+
// not maintain the sheet dimension attribute, so sweep the whole grid
75+
// for styles on cells not yet seen. ponytail: 200x52 lookups per
76+
// sheet, fine for an import endpoint.
77+
for r := range maxRows {
78+
for c := range maxCols {
79+
ref := sheet.CellRef{Row: r, Col: c}
80+
if _, seen := sh.Cells[ref]; seen {
81+
continue
82+
}
83+
axis, _ := excelize.CoordinatesToCellName(c+1, r+1)
84+
if xid, serr := f.GetCellStyle(name, axis); serr == nil && xid != 0 {
85+
if id := poolIdFor(xid); id != 0 {
86+
sh.SetCell(ref, sheet.Cell{StyleId: id})
87+
}
88+
}
89+
}
90+
}
91+
92+
// Dimensions: excelize returns the sheet default for untouched
93+
// indices, so probe a far-away column/row as the baseline and store
94+
// only deviations. ponytail: scans the fixed grid extent (52/200).
95+
baseW, _ := f.GetColWidth(name, "XFD")
96+
for c := range maxCols {
97+
cn, _ := excelize.ColumnNumberToName(c + 1)
98+
if w, err := f.GetColWidth(name, cn); err == nil && math.Abs(w-baseW) > 0.01 {
99+
sh.ColWidths[c] = colWidthToPx(w)
100+
}
101+
}
102+
baseH, _ := f.GetRowHeight(name, excelize.TotalRows)
103+
for r := range maxRows {
104+
if h, err := f.GetRowHeight(name, r+1); err == nil && math.Abs(h-baseH) > 0.01 {
105+
sh.RowHeights[r] = rowHeightToPx(h)
106+
}
107+
}
108+
109+
// Freeze panes: the model only supports freezing the first row/col.
110+
if panes, err := f.GetPanes(name); err == nil && panes.Freeze {
111+
if panes.YSplit > 0 {
112+
sh.FrozenRows = 1
113+
}
114+
if panes.XSplit > 0 {
115+
sh.FrozenCols = 1
41116
}
42117
}
43118
}

lib/xlsx/import_test.go

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"github.com/ether/etherpad-go/lib/sheet"
8+
"github.com/xuri/excelize/v2"
89
)
910

1011
func TestImportExportRoundTrip(t *testing.T) {
@@ -38,3 +39,117 @@ func TestImportExportRoundTrip(t *testing.T) {
3839
t.Fatalf("B1 = %q", sh.GetCell(sheet.CellRef{Row: 0, Col: 1}).Raw)
3940
}
4041
}
42+
43+
func TestRoundTripStylesDimsAndFreeze(t *testing.T) {
44+
wb := sheet.NewWorkbook()
45+
s := wb.AddSheet("Data", "Data")
46+
props := map[string]string{
47+
"bold": "1", "italic": "1", "underline": "1",
48+
"color": "#ff0000", "bg": "#00ff00", "align": "center",
49+
"border": "all", "wrap": "1",
50+
"fontFamily": "Arial", "fontSize": "14",
51+
"numFmt": "currency:2",
52+
}
53+
styleId := wb.Styles.Put(sheet.Style{Props: props})
54+
s.SetCell(sheet.CellRef{Row: 0, Col: 0}, sheet.Cell{Raw: "42", StyleId: styleId})
55+
// styled but empty cell must survive too
56+
s.SetCell(sheet.CellRef{Row: 1, Col: 1}, sheet.Cell{StyleId: styleId})
57+
s.ColWidths[2] = 150
58+
s.RowHeights[3] = 40
59+
s.FrozenRows, s.FrozenCols = 1, 1
60+
61+
data, err := Export(wb)
62+
if err != nil {
63+
t.Fatalf("Export: %v", err)
64+
}
65+
snap, err := Import(bytes.NewReader(data))
66+
if err != nil {
67+
t.Fatalf("Import: %v", err)
68+
}
69+
got := sheet.WorkbookFromSnapshot(snap)
70+
sh := got.SheetByID("Data")
71+
if sh == nil {
72+
t.Fatal("sheet Data missing")
73+
}
74+
75+
for _, ref := range []sheet.CellRef{{Row: 0, Col: 0}, {Row: 1, Col: 1}} {
76+
c := sh.GetCell(ref)
77+
if c.StyleId == 0 {
78+
t.Fatalf("cell %v lost its style", ref)
79+
}
80+
st, _ := got.Styles.Get(c.StyleId)
81+
for k, want := range props {
82+
if st.Props[k] != want {
83+
t.Errorf("cell %v prop %s = %q, want %q", ref, k, st.Props[k], want)
84+
}
85+
}
86+
}
87+
if px := sh.ColWidths[2]; px < 148 || px > 152 { // unit conversion rounds
88+
t.Errorf("col 2 width = %d, want ~150", px)
89+
}
90+
if px := sh.RowHeights[3]; px < 38 || px > 42 {
91+
t.Errorf("row 3 height = %d, want ~40", px)
92+
}
93+
if sh.FrozenRows != 1 || sh.FrozenCols != 1 {
94+
t.Errorf("freeze = %d/%d, want 1/1", sh.FrozenRows, sh.FrozenCols)
95+
}
96+
if len(sh.ColWidths) != 1 || len(sh.RowHeights) != 1 {
97+
t.Errorf("dims not sparse: cols=%v rows=%v", sh.ColWidths, sh.RowHeights)
98+
}
99+
}
100+
101+
// TestImportForeignFile builds a file the way Excel would (builtin numFmt ids,
102+
// style on a value cell) rather than via our own Export, so mapping gaps can't
103+
// hide behind a symmetric round-trip.
104+
func TestImportForeignFile(t *testing.T) {
105+
f := excelize.NewFile()
106+
styleId, err := f.NewStyle(&excelize.Style{
107+
Font: &excelize.Font{Bold: true, Color: "FF0000"},
108+
NumFmt: 10, // builtin "0.00%"
109+
})
110+
if err != nil {
111+
t.Fatalf("NewStyle: %v", err)
112+
}
113+
if err := f.SetCellValue("Sheet1", "A1", 0.5); err != nil {
114+
t.Fatalf("SetCellValue: %v", err)
115+
}
116+
if err := f.SetCellStyle("Sheet1", "A1", "A1", styleId); err != nil {
117+
t.Fatalf("SetCellStyle: %v", err)
118+
}
119+
buf, err := f.WriteToBuffer()
120+
if err != nil {
121+
t.Fatalf("WriteToBuffer: %v", err)
122+
}
123+
124+
snap, err := Import(bytes.NewReader(buf.Bytes()))
125+
if err != nil {
126+
t.Fatalf("Import: %v", err)
127+
}
128+
wb := sheet.WorkbookFromSnapshot(snap)
129+
c := wb.SheetByID("Sheet1").GetCell(sheet.CellRef{Row: 0, Col: 0})
130+
if c.StyleId == 0 {
131+
t.Fatal("A1 has no style")
132+
}
133+
st, _ := wb.Styles.Get(c.StyleId)
134+
if st.Props["bold"] != "1" || st.Props["color"] != "#ff0000" || st.Props["numFmt"] != "percent:2" {
135+
t.Fatalf("props = %v", st.Props)
136+
}
137+
}
138+
139+
func TestNumFmtCodeMapping(t *testing.T) {
140+
cases := map[string]string{
141+
"@": "text", "m/d/yyyy": "date", "0.00%": "percent:2",
142+
"$#,##0.00": "currency:2", "#,##0": "number:0", "General": "",
143+
}
144+
for code, want := range cases {
145+
if got := codeToNumFmt(code); got != want {
146+
t.Errorf("codeToNumFmt(%q) = %q, want %q", code, got, want)
147+
}
148+
}
149+
// symbolic -> code -> symbolic is stable
150+
for _, nf := range []string{"text", "date", "number:2", "currency:1", "percent:0"} {
151+
if got := codeToNumFmt(numFmtToCode(nf)); got != nf {
152+
t.Errorf("roundtrip %q -> %q", nf, got)
153+
}
154+
}
155+
}

0 commit comments

Comments
 (0)