Skip to content

Commit 9bc3fd7

Browse files
chengcxyxinyao.cheng
chengcxy
and
xinyao.cheng
authored
This optimize the code, simplify unit test and drawing object position calculation (#1561)
Co-authored-by: xinyao.cheng <[email protected]>
1 parent 8e891b5 commit 9bc3fd7

File tree

7 files changed

+84
-101
lines changed

7 files changed

+84
-101
lines changed

‎col.go

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -604,20 +604,21 @@ func flatCols(col xlsxCol, cols []xlsxCol, replacer func(fc, c xlsxCol) xlsxCol)
604604
// width # Width of object frame.
605605
// height # Height of object frame.
606606
func (f *File) positionObjectPixels(sheet string, col, row, x1, y1, width, height int) (int, int, int, int, int, int) {
607+
colIdx, rowIdx := col-1, row-1
607608
// Adjust start column for offsets that are greater than the col width.
608-
for x1 >= f.getColWidth(sheet, col+1) {
609-
x1 -= f.getColWidth(sheet, col+1)
610-
col++
609+
for x1 >= f.getColWidth(sheet, colIdx+1) {
610+
colIdx++
611+
x1 -= f.getColWidth(sheet, colIdx)
611612
}
612613

613614
// Adjust start row for offsets that are greater than the row height.
614-
for y1 >= f.getRowHeight(sheet, row+1) {
615-
y1 -= f.getRowHeight(sheet, row+1)
616-
row++
615+
for y1 >= f.getRowHeight(sheet, rowIdx+1) {
616+
rowIdx++
617+
y1 -= f.getRowHeight(sheet, rowIdx)
617618
}
618619

619620
// Initialized end cell to the same as the start cell.
620-
colEnd, rowEnd := col, row
621+
colEnd, rowEnd := colIdx, rowIdx
621622

622623
width += x1
623624
height += y1
@@ -635,9 +636,7 @@ func (f *File) positionObjectPixels(sheet string, col, row, x1, y1, width, heigh
635636
}
636637

637638
// The end vertices are whatever is left from the width and height.
638-
x2 := width
639-
y2 := height
640-
return col, row, colEnd, rowEnd, x2, y2
639+
return colIdx, rowIdx, colEnd, rowEnd, width, height
641640
}
642641

643642
// getColWidth provides a function to get column width in pixels by given

‎drawing.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1297,12 +1297,9 @@ func (f *File) addDrawingChart(sheet, drawingXML, cell string, width, height, rI
12971297
if err != nil {
12981298
return err
12991299
}
1300-
colIdx := col - 1
1301-
rowIdx := row - 1
1302-
13031300
width = int(float64(width) * opts.ScaleX)
13041301
height = int(float64(height) * opts.ScaleY)
1305-
colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, colIdx, rowIdx, opts.OffsetX, opts.OffsetY, width, height)
1302+
colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, col, row, opts.OffsetX, opts.OffsetY, width, height)
13061303
content, cNvPrID, err := f.drawingParser(drawingXML)
13071304
if err != nil {
13081305
return err

‎merge_test.go

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,18 @@ func TestMergeCell(t *testing.T) {
1313
t.FailNow()
1414
}
1515
assert.EqualError(t, f.MergeCell("Sheet1", "A", "B"), newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
16-
assert.NoError(t, f.MergeCell("Sheet1", "D9", "D9"))
17-
assert.NoError(t, f.MergeCell("Sheet1", "D9", "E9"))
18-
assert.NoError(t, f.MergeCell("Sheet1", "H14", "G13"))
19-
assert.NoError(t, f.MergeCell("Sheet1", "C9", "D8"))
20-
assert.NoError(t, f.MergeCell("Sheet1", "F11", "G13"))
21-
assert.NoError(t, f.MergeCell("Sheet1", "H7", "B15"))
22-
assert.NoError(t, f.MergeCell("Sheet1", "D11", "F13"))
23-
assert.NoError(t, f.MergeCell("Sheet1", "G10", "K12"))
16+
for _, cells := range [][]string{
17+
{"D9", "D9"},
18+
{"D9", "E9"},
19+
{"H14", "G13"},
20+
{"C9", "D8"},
21+
{"F11", "G13"},
22+
{"H7", "B15"},
23+
{"D11", "F13"},
24+
{"G10", "K12"},
25+
} {
26+
assert.NoError(t, f.MergeCell("Sheet1", cells[0], cells[1]))
27+
}
2428
assert.NoError(t, f.SetCellValue("Sheet1", "G11", "set value in merged cell"))
2529
assert.NoError(t, f.SetCellInt("Sheet1", "H11", 100))
2630
assert.NoError(t, f.SetCellValue("Sheet1", "I11", 0.5))
@@ -39,32 +43,29 @@ func TestMergeCell(t *testing.T) {
3943

4044
_, err = f.NewSheet("Sheet3")
4145
assert.NoError(t, err)
42-
assert.NoError(t, f.MergeCell("Sheet3", "D11", "F13"))
43-
assert.NoError(t, f.MergeCell("Sheet3", "G10", "K12"))
44-
45-
assert.NoError(t, f.MergeCell("Sheet3", "B1", "D5")) // B1:D5
46-
assert.NoError(t, f.MergeCell("Sheet3", "E1", "F5")) // E1:F5
47-
48-
assert.NoError(t, f.MergeCell("Sheet3", "H2", "I5"))
49-
assert.NoError(t, f.MergeCell("Sheet3", "I4", "J6")) // H2:J6
50-
51-
assert.NoError(t, f.MergeCell("Sheet3", "M2", "N5"))
52-
assert.NoError(t, f.MergeCell("Sheet3", "L4", "M6")) // L2:N6
53-
54-
assert.NoError(t, f.MergeCell("Sheet3", "P4", "Q7"))
55-
assert.NoError(t, f.MergeCell("Sheet3", "O2", "P5")) // O2:Q7
5646

57-
assert.NoError(t, f.MergeCell("Sheet3", "A9", "B12"))
58-
assert.NoError(t, f.MergeCell("Sheet3", "B7", "C9")) // A7:C12
59-
60-
assert.NoError(t, f.MergeCell("Sheet3", "E9", "F10"))
61-
assert.NoError(t, f.MergeCell("Sheet3", "D8", "G12"))
62-
63-
assert.NoError(t, f.MergeCell("Sheet3", "I8", "I12"))
64-
assert.NoError(t, f.MergeCell("Sheet3", "I10", "K10"))
65-
66-
assert.NoError(t, f.MergeCell("Sheet3", "M8", "Q13"))
67-
assert.NoError(t, f.MergeCell("Sheet3", "N10", "O11"))
47+
for _, cells := range [][]string{
48+
{"D11", "F13"},
49+
{"G10", "K12"},
50+
{"B1", "D5"}, // B1:D5
51+
{"E1", "F5"}, // E1:F5
52+
{"H2", "I5"},
53+
{"I4", "J6"}, // H2:J6
54+
{"M2", "N5"},
55+
{"L4", "M6"}, // L2:N6
56+
{"P4", "Q7"},
57+
{"O2", "P5"}, // O2:Q7
58+
{"A9", "B12"},
59+
{"B7", "C9"}, // A7:C12
60+
{"E9", "F10"},
61+
{"D8", "G12"},
62+
{"I8", "I12"},
63+
{"I10", "K10"},
64+
{"M8", "Q13"},
65+
{"N10", "O11"},
66+
} {
67+
assert.NoError(t, f.MergeCell("Sheet3", cells[0], cells[1]))
68+
}
6869

6970
// Test merge cells on not exists worksheet
7071
assert.EqualError(t, f.MergeCell("SheetN", "N10", "O11"), "sheet SheetN does not exist")

‎picture.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -332,16 +332,13 @@ func (f *File) addDrawingPicture(sheet, drawingXML, cell, ext string, rID, hyper
332332
}
333333
width, height := img.Width, img.Height
334334
if opts.AutoFit {
335-
width, height, col, row, err = f.drawingResize(sheet, cell, float64(width), float64(height), opts)
336-
if err != nil {
335+
if width, height, col, row, err = f.drawingResize(sheet, cell, float64(width), float64(height), opts); err != nil {
337336
return err
338337
}
339338
} else {
340339
width = int(float64(width) * opts.ScaleX)
341340
height = int(float64(height) * opts.ScaleY)
342341
}
343-
col--
344-
row--
345342
colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, col, row, opts.OffsetX, opts.OffsetY, width, height)
346343
content, cNvPrID, err := f.drawingParser(drawingXML)
347344
if err != nil {

‎picture_test.go

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,9 @@ func TestAddPicture(t *testing.T) {
6262
// Test add picture to worksheet from bytes with illegal cell reference
6363
assert.EqualError(t, f.AddPictureFromBytes("Sheet1", "A", &Picture{Extension: ".png", File: file, Format: &GraphicOptions{AltText: "Excel Logo"}}), newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
6464

65-
assert.NoError(t, f.AddPicture("Sheet1", "Q8", filepath.Join("test", "images", "excel.gif"), nil))
66-
assert.NoError(t, f.AddPicture("Sheet1", "Q15", filepath.Join("test", "images", "excel.jpg"), nil))
67-
assert.NoError(t, f.AddPicture("Sheet1", "Q22", filepath.Join("test", "images", "excel.tif"), nil))
68-
assert.NoError(t, f.AddPicture("Sheet1", "Q28", filepath.Join("test", "images", "excel.bmp"), nil))
65+
for cell, ext := range map[string]string{"Q8": "gif", "Q15": "jpg", "Q22": "tif", "Q28": "bmp"} {
66+
assert.NoError(t, f.AddPicture("Sheet1", cell, filepath.Join("test", "images", fmt.Sprintf("excel.%s", ext)), nil))
67+
}
6968

7069
// Test write file to given path
7170
assert.NoError(t, f.SaveAs(filepath.Join("test", "TestAddPicture1.xlsx")))
@@ -99,15 +98,10 @@ func TestAddPictureErrors(t *testing.T) {
9998
// Test add picture with custom image decoder and encoder
10099
decode := func(r io.Reader) (image.Image, error) { return nil, nil }
101100
decodeConfig := func(r io.Reader) (image.Config, error) { return image.Config{Height: 100, Width: 90}, nil }
102-
image.RegisterFormat("emf", "", decode, decodeConfig)
103-
image.RegisterFormat("wmf", "", decode, decodeConfig)
104-
image.RegisterFormat("emz", "", decode, decodeConfig)
105-
image.RegisterFormat("wmz", "", decode, decodeConfig)
106-
image.RegisterFormat("svg", "", decode, decodeConfig)
107-
assert.NoError(t, f.AddPicture("Sheet1", "Q1", filepath.Join("test", "images", "excel.emf"), nil))
108-
assert.NoError(t, f.AddPicture("Sheet1", "Q7", filepath.Join("test", "images", "excel.wmf"), nil))
109-
assert.NoError(t, f.AddPicture("Sheet1", "Q13", filepath.Join("test", "images", "excel.emz"), nil))
110-
assert.NoError(t, f.AddPicture("Sheet1", "Q19", filepath.Join("test", "images", "excel.wmz"), nil))
101+
for cell, ext := range map[string]string{"Q1": "emf", "Q7": "wmf", "Q13": "emz", "Q19": "wmz"} {
102+
image.RegisterFormat(ext, "", decode, decodeConfig)
103+
assert.NoError(t, f.AddPicture("Sheet1", cell, filepath.Join("test", "images", fmt.Sprintf("excel.%s", ext)), nil))
104+
}
111105
assert.NoError(t, f.AddPicture("Sheet1", "Q25", "excelize.svg", &GraphicOptions{ScaleX: 2.8}))
112106
assert.NoError(t, f.SaveAs(filepath.Join("test", "TestAddPicture2.xlsx")))
113107
assert.NoError(t, f.Close())

‎shape.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -326,13 +326,10 @@ func (f *File) addDrawingShape(sheet, drawingXML, cell string, opts *Shape) erro
326326
if err != nil {
327327
return err
328328
}
329-
colIdx := fromCol - 1
330-
rowIdx := fromRow - 1
331-
332329
width := int(float64(opts.Width) * opts.Format.ScaleX)
333330
height := int(float64(opts.Height) * opts.Format.ScaleY)
334331

335-
colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, colIdx, rowIdx, opts.Format.OffsetX, opts.Format.OffsetY,
332+
colStart, rowStart, colEnd, rowEnd, x2, y2 := f.positionObjectPixels(sheet, fromCol, fromRow, opts.Format.OffsetX, opts.Format.OffsetY,
336333
width, height)
337334
content, cNvPrID, err := f.drawingParser(drawingXML)
338335
if err != nil {

‎stream_test.go

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -337,11 +337,9 @@ func TestStreamSetRowWithStyle(t *testing.T) {
337337

338338
ws, err := file.workSheetReader("Sheet1")
339339
assert.NoError(t, err)
340-
assert.Equal(t, grayStyleID, ws.SheetData.Row[0].C[0].S)
341-
assert.Equal(t, zeroStyleID, ws.SheetData.Row[0].C[1].S)
342-
assert.Equal(t, zeroStyleID, ws.SheetData.Row[0].C[2].S)
343-
assert.Equal(t, blueStyleID, ws.SheetData.Row[0].C[3].S)
344-
assert.Equal(t, blueStyleID, ws.SheetData.Row[0].C[4].S)
340+
for colIdx, expected := range []int{grayStyleID, zeroStyleID, zeroStyleID, blueStyleID, blueStyleID} {
341+
assert.Equal(t, expected, ws.SheetData.Row[0].C[colIdx].S)
342+
}
345343
}
346344

347345
func TestStreamSetCellValFunc(t *testing.T) {
@@ -352,25 +350,29 @@ func TestStreamSetCellValFunc(t *testing.T) {
352350
sw, err := f.NewStreamWriter("Sheet1")
353351
assert.NoError(t, err)
354352
c := &xlsxC{}
355-
assert.NoError(t, sw.setCellValFunc(c, 128))
356-
assert.NoError(t, sw.setCellValFunc(c, int8(-128)))
357-
assert.NoError(t, sw.setCellValFunc(c, int16(-32768)))
358-
assert.NoError(t, sw.setCellValFunc(c, int32(-2147483648)))
359-
assert.NoError(t, sw.setCellValFunc(c, int64(-9223372036854775808)))
360-
assert.NoError(t, sw.setCellValFunc(c, uint(128)))
361-
assert.NoError(t, sw.setCellValFunc(c, uint8(255)))
362-
assert.NoError(t, sw.setCellValFunc(c, uint16(65535)))
363-
assert.NoError(t, sw.setCellValFunc(c, uint32(4294967295)))
364-
assert.NoError(t, sw.setCellValFunc(c, uint64(18446744073709551615)))
365-
assert.NoError(t, sw.setCellValFunc(c, float32(100.1588)))
366-
assert.NoError(t, sw.setCellValFunc(c, 100.1588))
367-
assert.NoError(t, sw.setCellValFunc(c, " Hello"))
368-
assert.NoError(t, sw.setCellValFunc(c, []byte(" Hello")))
369-
assert.NoError(t, sw.setCellValFunc(c, time.Now().UTC()))
370-
assert.NoError(t, sw.setCellValFunc(c, time.Duration(1e13)))
371-
assert.NoError(t, sw.setCellValFunc(c, true))
372-
assert.NoError(t, sw.setCellValFunc(c, nil))
373-
assert.NoError(t, sw.setCellValFunc(c, complex64(5+10i)))
353+
for _, val := range []interface{}{
354+
128,
355+
int8(-128),
356+
int16(-32768),
357+
int32(-2147483648),
358+
int64(-9223372036854775808),
359+
uint(128),
360+
uint8(255),
361+
uint16(65535),
362+
uint32(4294967295),
363+
uint64(18446744073709551615),
364+
float32(100.1588),
365+
100.1588,
366+
" Hello",
367+
[]byte(" Hello"),
368+
time.Now().UTC(),
369+
time.Duration(1e13),
370+
true,
371+
nil,
372+
complex64(5 + 10i),
373+
} {
374+
assert.NoError(t, sw.setCellValFunc(c, val))
375+
}
374376
}
375377

376378
func TestStreamWriterOutlineLevel(t *testing.T) {
@@ -389,14 +391,10 @@ func TestStreamWriterOutlineLevel(t *testing.T) {
389391

390392
file, err = OpenFile(filepath.Join("test", "TestStreamWriterSetRowOutlineLevel.xlsx"))
391393
assert.NoError(t, err)
392-
level, err := file.GetRowOutlineLevel("Sheet1", 1)
393-
assert.NoError(t, err)
394-
assert.Equal(t, uint8(1), level)
395-
level, err = file.GetRowOutlineLevel("Sheet1", 2)
396-
assert.NoError(t, err)
397-
assert.Equal(t, uint8(7), level)
398-
level, err = file.GetRowOutlineLevel("Sheet1", 3)
399-
assert.NoError(t, err)
400-
assert.Equal(t, uint8(0), level)
394+
for rowIdx, expected := range []uint8{1, 7, 0} {
395+
level, err := file.GetRowOutlineLevel("Sheet1", rowIdx+1)
396+
assert.NoError(t, err)
397+
assert.Equal(t, expected, level)
398+
}
401399
assert.NoError(t, file.Close())
402400
}

0 commit comments

Comments
 (0)