Fix #424, refactor merged cells adjuster

adjust.go

@@ -9,7 +9,10 @@
 
 package excelize
 
-import "strings"
+import (
+	"errors"
+	"strings"
+)
 
 type adjustDirection bool
 
@@ -140,46 +143,85 @@ func (f *File) adjustAutoFilter(xlsx *xlsxWorksheet, dir adjustDirection, num, o
 		return nil
 	}
 
-	rng := strings.Split(xlsx.AutoFilter.Ref, ":")
-	firstCell := rng[0]
-	lastCell := rng[1]
-
-	firstCol, firstRow, err := CellNameToCoordinates(firstCell)
+	coordinates, err := f.areaRefToCoordinates(xlsx.AutoFilter.Ref)
 	if err != nil {
 		return err
 	}
+	x1, y1, x2, y2 := coordinates[0], coordinates[1], coordinates[2], coordinates[3]
 
-	lastCol, lastRow, err := CellNameToCoordinates(lastCell)
-	if err != nil {
-		return err
-	}
-
-	if (dir == rows && firstRow == num && offset < 0) || (dir == columns && firstCol == num && lastCol == num) {
+	if (dir == rows && y1 == num && offset < 0) || (dir == columns && x1 == num && x2 == num) {
 		xlsx.AutoFilter = nil
 		for rowIdx := range xlsx.SheetData.Row {
 			rowData := &xlsx.SheetData.Row[rowIdx]
-			if rowData.R > firstRow && rowData.R <= lastRow {
+			if rowData.R > y1 && rowData.R <= y2 {
 				rowData.Hidden = false
 			}
 		}
 		return nil
 	}
 
+	coordinates = f.adjustAutoFilterHelper(dir, coordinates, num, offset)
+	x1, y1, x2, y2 = coordinates[0], coordinates[1], coordinates[2], coordinates[3]
+
+	if xlsx.AutoFilter.Ref, err = f.coordinatesToAreaRef([]int{x1, y1, x2, y2}); err != nil {
+		return err
+	}
+	return nil
+}
+
+// adjustAutoFilterHelper provides a function for adjusting auto filter to
+// compare and calculate cell axis by the given adjust direction, operation
+// axis and offset.
+func (f *File) adjustAutoFilterHelper(dir adjustDirection, coordinates []int, num, offset int) []int {
 	if dir == rows {
-		if firstRow >= num {
-			firstCell, _ = CoordinatesToCellName(firstCol, firstRow+offset)
+		if coordinates[1] >= num {
+			coordinates[1] += offset
 		}
-		if lastRow >= num {
-			lastCell, _ = CoordinatesToCellName(lastCol, lastRow+offset)
+		if coordinates[3] >= num {
+			coordinates[3] += offset
 		}
 	} else {
-		if lastCol >= num {
-			lastCell, _ = CoordinatesToCellName(lastCol+offset, lastRow)
+		if coordinates[2] >= num {
+			coordinates[2] += offset
 		}
 	}
+	return coordinates
+}
 
-	xlsx.AutoFilter.Ref = firstCell + ":" + lastCell
-	return nil
+// areaRefToCoordinates provides a function to convert area reference to a
+// pair of coordinates.
+func (f *File) areaRefToCoordinates(ref string) ([]int, error) {
+	coordinates := make([]int, 4)
+	rng := strings.Split(ref, ":")
+	firstCell := rng[0]
+	lastCell := rng[1]
+	var err error
+	coordinates[0], coordinates[1], err = CellNameToCoordinates(firstCell)
+	if err != nil {
+		return coordinates, err
+	}
+	coordinates[2], coordinates[3], err = CellNameToCoordinates(lastCell)
+	if err != nil {
+		return coordinates, err
+	}
+	return coordinates, err
+}
+
+// coordinatesToAreaRef provides a function to convert a pair of coordinates
+// to area reference.
+func (f *File) coordinatesToAreaRef(coordinates []int) (string, error) {
+	if len(coordinates) != 4 {
+		return "", errors.New("coordinates length must be 4")
+	}
+	firstCell, err := CoordinatesToCellName(coordinates[0], coordinates[1])
+	if err != nil {
+		return "", err
+	}
+	lastCell, err := CoordinatesToCellName(coordinates[2], coordinates[3])
+	if err != nil {
+		return "", err
+	}
+	return firstCell + ":" + lastCell, err
 }
 
 // adjustMergeCells provides a function to update merged cells when inserting
@@ -190,59 +232,56 @@ func (f *File) adjustMergeCells(xlsx *xlsxWorksheet, dir adjustDirection, num, o
 	}
 
 	for i, areaData := range xlsx.MergeCells.Cells {
-		rng := strings.Split(areaData.Ref, ":")
-		firstCell := rng[0]
-		lastCell := rng[1]
-
-		firstCol, firstRow, err := CellNameToCoordinates(firstCell)
+		coordinates, err := f.areaRefToCoordinates(areaData.Ref)
 		if err != nil {
 			return err
 		}
-
-		lastCol, lastRow, err := CellNameToCoordinates(lastCell)
-		if err != nil {
-			return err
-		}
-
-		adjust := func(v int) int {
-			if v >= num {
-				v += offset
-				if v < 1 {
-					return 1
-				}
-				return v
-			}
-			return v
-		}
-
+		x1, y1, x2, y2 := coordinates[0], coordinates[1], coordinates[2], coordinates[3]
 		if dir == rows {
-			firstRow = adjust(firstRow)
-			lastRow = adjust(lastRow)
+			if y1 == num && y2 == num && offset < 0 {
+				f.deleteMergeCell(xlsx, i)
+			}
+			y1 = f.adjustMergeCellsHelper(y1, num, offset)
+			y2 = f.adjustMergeCellsHelper(y2, num, offset)
 		} else {
-			firstCol = adjust(firstCol)
-			lastCol = adjust(lastCol)
-		}
-
-		if firstCol == lastCol && firstRow == lastRow {
-			if len(xlsx.MergeCells.Cells) > 1 {
-				xlsx.MergeCells.Cells = append(xlsx.MergeCells.Cells[:i], xlsx.MergeCells.Cells[i+1:]...)
-				xlsx.MergeCells.Count = len(xlsx.MergeCells.Cells)
-			} else {
-				xlsx.MergeCells = nil
+			if x1 == num && x2 == num && offset < 0 {
+				f.deleteMergeCell(xlsx, i)
 			}
+			x1 = f.adjustMergeCellsHelper(x1, num, offset)
+			x2 = f.adjustMergeCellsHelper(x2, num, offset)
 		}
-
-		if firstCell, err = CoordinatesToCellName(firstCol, firstRow); err != nil {
+		if x1 == x2 && y1 == y2 {
+			f.deleteMergeCell(xlsx, i)
+		}
+		if areaData.Ref, err = f.coordinatesToAreaRef([]int{x1, y1, x2, y2}); err != nil {
 			return err
 		}
+	}
+	return nil
+}
 
-		if lastCell, err = CoordinatesToCellName(lastCol, lastRow); err != nil {
-			return err
+// adjustMergeCellsHelper provides a function for adjusting merge cells to
+// compare and calculate cell axis by the given pivot, operation axis and
+// offset.
+func (f *File) adjustMergeCellsHelper(pivot, num, offset int) int {
+	if pivot >= num {
+		pivot += offset
+		if pivot < 1 {
+			return 1
 		}
+		return pivot
+	}
+	return pivot
+}
 
-		areaData.Ref = firstCell + ":" + lastCell
+// deleteMergeCell provides a function to delete merged cell by given index.
+func (f *File) deleteMergeCell(sheet *xlsxWorksheet, idx int) {
+	if len(sheet.MergeCells.Cells) > 1 {
+		sheet.MergeCells.Cells = append(sheet.MergeCells.Cells[:idx], sheet.MergeCells.Cells[idx+1:]...)
+		sheet.MergeCells.Count = len(sheet.MergeCells.Cells)
+	} else {
+		sheet.MergeCells = nil
 	}
-	return nil
 }
 
 // adjustCalcChain provides a function to update the calculation chain when

adjust_test.go

@@ -27,6 +27,24 @@ func TestAdjustMergeCells(t *testing.T) {
 			},
 		},
 	}, rows, 0, 0), `cannot convert cell "B" to coordinates: invalid cell name "B"`)
+	assert.NoError(t, f.adjustMergeCells(&xlsxWorksheet{
+		MergeCells: &xlsxMergeCells{
+			Cells: []*xlsxMergeCell{
+				{
+					Ref: "A1:B1",
+				},
+			},
+		},
+	}, rows, 1, -1))
+	assert.NoError(t, f.adjustMergeCells(&xlsxWorksheet{
+		MergeCells: &xlsxMergeCells{
+			Cells: []*xlsxMergeCell{
+				{
+					Ref: "A1:A2",
+				},
+			},
+		},
+	}, columns, 1, -1))
 }
 
 func TestAdjustAutoFilter(t *testing.T) {
@@ -83,3 +101,16 @@ func TestAdjustCalcChain(t *testing.T) {
 	f.CalcChain = nil
 	assert.NoError(t, f.InsertCol("Sheet1", "A"))
 }
+
+func TestCoordinatesToAreaRef(t *testing.T) {
+	f := NewFile()
+	ref, err := f.coordinatesToAreaRef([]int{})
+	assert.EqualError(t, err, "coordinates length must be 4")
+	ref, err = f.coordinatesToAreaRef([]int{1, -1, 1, 1})
+	assert.EqualError(t, err, "invalid cell coordinates [1, -1]")
+	ref, err = f.coordinatesToAreaRef([]int{1, 1, 1, -1})
+	assert.EqualError(t, err, "invalid cell coordinates [1, -1]")
+	ref, err = f.coordinatesToAreaRef([]int{1, 1, 1, 1})
+	assert.NoError(t, err)
+	assert.EqualValues(t, ref, "A1:A1")
+}

cell.go

@@ -401,31 +401,27 @@ func (f *File) SetCellHyperLink(sheet, axis, link, linkType string) error {
 // If you create a merged cell that overlaps with another existing merged cell,
 // those merged cells that already exist will be removed.
 func (f *File) MergeCell(sheet, hcell, vcell string) error {
-	hcol, hrow, err := CellNameToCoordinates(hcell)
+	coordinates, err := f.areaRefToCoordinates(hcell + ":" + vcell)
 	if err != nil {
 		return err
 	}
+	x1, y1, x2, y2 := coordinates[0], coordinates[1], coordinates[2], coordinates[3]
 
-	vcol, vrow, err := CellNameToCoordinates(vcell)
-	if err != nil {
-		return err
-	}
-
-	if hcol == vcol && hrow == vrow {
+	if x1 == x2 && y1 == y2 {
 		return err
 	}
 
 	// Correct the coordinate area, such correct C1:B3 to B1:C3.
-	if vcol < hcol {
-		hcol, vcol = vcol, hcol
+	if x2 < x1 {
+		x1, x2 = x2, x1
 	}
 
-	if vrow < hrow {
-		hrow, vrow = vrow, hrow
+	if y2 < y1 {
+		y1, y2 = y2, y1
 	}
 
-	hcell, _ = CoordinatesToCellName(hcol, hrow)
-	vcell, _ = CoordinatesToCellName(vcol, vrow)
+	hcell, _ = CoordinatesToCellName(x1, y1)
+	vcell, _ = CoordinatesToCellName(x2, y2)
 
 	xlsx, err := f.workSheetReader(sheet)
 	if err != nil {

rows.go

@@ -421,7 +421,7 @@ func (f *File) RemoveRow(sheet string, row int) error {
 		return err
 	}
 	if row > len(xlsx.SheetData.Row) {
-		return nil
+		return f.adjustHelper(sheet, rows, row, -1)
 	}
 	for rowIdx := range xlsx.SheetData.Row {
 		if xlsx.SheetData.Row[rowIdx].R == row {

table.go

@@ -115,29 +115,24 @@ func (f *File) addSheetTable(sheet string, rID int) {
 
 // addTable provides a function to add table by given worksheet name,
 // coordinate area and format set.
-func (f *File) addTable(sheet, tableXML string, hcol, hrow, vcol, vrow, i int, formatSet *formatTable) error {
+func (f *File) addTable(sheet, tableXML string, x1, y1, x2, y2, i int, formatSet *formatTable) error {
 	// Correct the minimum number of rows, the table at least two lines.
-	if hrow == vrow {
-		vrow++
+	if y1 == y2 {
+		y2++
 	}
 
 	// Correct table reference coordinate area, such correct C1:B3 to B1:C3.
-	hcell, err := CoordinatesToCellName(hcol, hrow)
+	ref, err := f.coordinatesToAreaRef([]int{x1, y1, x2, y2})
 	if err != nil {
 		return err
 	}
-	vcell, err := CoordinatesToCellName(vcol, vrow)
-	if err != nil {
-		return err
-	}
-	ref := hcell + ":" + vcell
 
 	var tableColumn []*xlsxTableColumn
 
 	idx := 0
-	for i := hcol; i <= vcol; i++ {
+	for i := x1; i <= x2; i++ {
 		idx++
-		cell, err := CoordinatesToCellName(i, hrow)
+		cell, err := CoordinatesToCellName(i, y1)
 		if err != nil {
 			return err
 		}

xmlDecodeDrawing.go

@@ -11,19 +11,55 @@ package excelize
 
 import "encoding/xml"
 
-// decodeCellAnchor directly maps the oneCellAnchor (One Cell Anchor Shape Size)
-// and twoCellAnchor (Two Cell Anchor Shape Size). This element specifies a two
-// cell anchor placeholder for a group, a shape, or a drawing element. It moves
-// with cells and its extents are in EMU units.
+// decodeCellAnchor directly maps the oneCellAnchor (One Cell Anchor Shape
+// Size) and twoCellAnchor (Two Cell Anchor Shape Size). This element
+// specifies a two cell anchor placeholder for a group, a shape, or a drawing
+// element. It moves with cells and its extents are in EMU units.
 type decodeCellAnchor struct {
-	EditAs  string `xml:"editAs,attr,omitempty"`
-	Content string `xml:",innerxml"`
+	EditAs     string            `xml:"editAs,attr,omitempty"`
+	From       *decodeFrom       `xml:"from"`
+	To         *decodeTo         `xml:"to"`
+	Sp         *decodeSp         `xml:"sp"`
+	ClientData *decodeClientData `xml:"clientData"`
+	Content    string            `xml:",innerxml"`
+}
+
+// xdrSp (Shape) directly maps the sp element. This element specifies the
+// existence of a single shape. A shape can either be a preset or a custom
+// geometry, defined using the SpreadsheetDrawingML framework. In addition to
+// a geometry each shape can have both visual and non-visual properties
+// attached. Text and corresponding styling information can also be attached
+// to a shape. This shape is specified along with all other shapes within
+// either the shape tree or group shape elements.
+type decodeSp struct {
+	NvSpPr *decodeNvSpPr `xml:"nvSpPr"`
+	SpPr   *decodeSpPr   `xml:"spPr"`
+}
+
+// decodeSp (Non-Visual Properties for a Shape) directly maps the nvSpPr
+// element. This element specifies all non-visual properties for a shape. This
+// element is a container for the non-visual identification properties, shape
+// properties and application properties that are to be associated with a
+// shape. This allows for additional information that does not affect the
+// appearance of the shape to be stored.
+type decodeNvSpPr struct {
+	CNvPr   *decodeCNvPr   `xml:"cNvPr"`
+	ExtLst  *decodeExt     `xml:"extLst"`
+	CNvSpPr *decodeCNvSpPr `xml:"cNvSpPr"`
+}
+
+// decodeCNvSpPr (Connection Non-Visual Shape Properties) directly maps the
+// cNvSpPr element. This element specifies the set of non-visual properties
+// for a connection shape. These properties specify all data about the
+// connection shape which do not affect its display within a spreadsheet.
+type decodeCNvSpPr struct {
+	TxBox bool `xml:"txBox,attr"`
 }
 
 // decodeWsDr directly maps the root element for a part of this content type
-// shall wsDr. In order to solve the problem that the label structure is changed
-// after serialization and deserialization, two different structures are
-// defined. decodeWsDr just for deserialization.
+// shall wsDr. In order to solve the problem that the label structure is
+// changed after serialization and deserialization, two different structures
+// are defined. decodeWsDr just for deserialization.
 type decodeWsDr struct {
 	A             string              `xml:"xmlns a,attr"`
 	Xdr           string              `xml:"xmlns xdr,attr"`
@@ -34,9 +70,9 @@ type decodeWsDr struct {
 }
 
 // decodeTwoCellAnchor directly maps the oneCellAnchor (One Cell Anchor Shape
-// Size) and twoCellAnchor (Two Cell Anchor Shape Size). This element specifies
-// a two cell anchor placeholder for a group, a shape, or a drawing element. It
-// moves with cells and its extents are in EMU units.
+// Size) and twoCellAnchor (Two Cell Anchor Shape Size). This element
+// specifies a two cell anchor placeholder for a group, a shape, or a drawing
+// element. It moves with cells and its extents are in EMU units.
 type decodeTwoCellAnchor struct {
 	From       *decodeFrom       `xml:"from"`
 	To         *decodeTo         `xml:"to"`
@@ -46,7 +82,8 @@ type decodeTwoCellAnchor struct {
 
 // decodeCNvPr directly maps the cNvPr (Non-Visual Drawing Properties). This
 // element specifies non-visual canvas properties. This allows for additional
-// information that does not affect the appearance of the picture to be stored.
+// information that does not affect the appearance of the picture to be
+// stored.
 type decodeCNvPr struct {
 	ID    int    `xml:"id,attr"`
 	Name  string `xml:"name,attr"`
@@ -55,8 +92,8 @@ type decodeCNvPr struct {
 }
 
 // decodePicLocks directly maps the picLocks (Picture Locks). This element
-// specifies all locking properties for a graphic frame. These properties inform
-// the generating application about specific properties that have been
+// specifies all locking properties for a graphic frame. These properties
+// inform the generating application about specific properties that have been
 // previously locked and thus should not be changed.
 type decodePicLocks struct {
 	NoAdjustHandles    bool `xml:"noAdjustHandles,attr,omitempty"`
@@ -82,9 +119,9 @@ type decodeBlip struct {
 	R      string `xml:"r,attr"`
 }
 
-// decodeStretch directly maps the stretch element. This element specifies that
-// a BLIP should be stretched to fill the target rectangle. The other option is
-// a tile where a BLIP is tiled to fill the available area.
+// decodeStretch directly maps the stretch element. This element specifies
+// that a BLIP should be stretched to fill the target rectangle. The other
+// option is a tile where a BLIP is tiled to fill the available area.
 type decodeStretch struct {
 	FillRect string `xml:"fillRect"`
 }
@@ -128,12 +165,12 @@ type decodeCNvPicPr struct {
 	PicLocks decodePicLocks `xml:"picLocks"`
 }
 
-// directly maps the nvPicPr (Non-Visual Properties for a Picture). This element
-// specifies all non-visual properties for a picture. This element is a
-// container for the non-visual identification properties, shape properties and
-// application properties that are to be associated with a picture. This allows
-// for additional information that does not affect the appearance of the picture
-// to be stored.
+// directly maps the nvPicPr (Non-Visual Properties for a Picture). This
+// element specifies all non-visual properties for a picture. This element is
+// a container for the non-visual identification properties, shape properties
+// and application properties that are to be associated with a picture. This
+// allows for additional information that does not affect the appearance of
+// the picture to be stored.
 type decodeNvPicPr struct {
 	CNvPr    decodeCNvPr    `xml:"cNvPr"`
 	CNvPicPr decodeCNvPicPr `xml:"cNvPicPr"`
@@ -148,20 +185,20 @@ type decodeBlipFill struct {
 	Stretch decodeStretch `xml:"stretch"`
 }
 
-// decodeSpPr directly maps the spPr (Shape Properties). This element specifies
-// the visual shape properties that can be applied to a picture. These are the
-// same properties that are allowed to describe the visual properties of a shape
-// but are used here to describe the visual appearance of a picture within a
-// document.
+// decodeSpPr directly maps the spPr (Shape Properties). This element
+// specifies the visual shape properties that can be applied to a picture.
+// These are the same properties that are allowed to describe the visual
+// properties of a shape but are used here to describe the visual appearance
+// of a picture within a document.
 type decodeSpPr struct {
-	Xfrm     decodeXfrm     `xml:"a:xfrm"`
-	PrstGeom decodePrstGeom `xml:"a:prstGeom"`
+	Xfrm     decodeXfrm     `xml:"xfrm"`
+	PrstGeom decodePrstGeom `xml:"prstGeom"`
 }
 
-// decodePic elements encompass the definition of pictures within the DrawingML
-// framework. While pictures are in many ways very similar to shapes they have
-// specific properties that are unique in order to optimize for picture-
-// specific scenarios.
+// decodePic elements encompass the definition of pictures within the
+// DrawingML framework. While pictures are in many ways very similar to shapes
+// they have specific properties that are unique in order to optimize for
+// picture- specific scenarios.
 type decodePic struct {
 	NvPicPr  decodeNvPicPr  `xml:"nvPicPr"`
 	BlipFill decodeBlipFill `xml:"blipFill"`
@@ -184,8 +221,8 @@ type decodeTo struct {
 	RowOff int `xml:"rowOff"`
 }
 
-// decodeClientData directly maps the clientData element. An empty element which
-// specifies (via attributes) certain properties related to printing and
+// decodeClientData directly maps the clientData element. An empty element
+// which specifies (via attributes) certain properties related to printing and
 // selection of the drawing object. The fLocksWithSheet attribute (either true
 // or false) determines whether to disable selection when the sheet is
 // protected, and fPrintsWithSheet attribute (either true or false) determines