remove ineffectual variable assignments and simplify code

LICENSE

@@ -1,6 +1,6 @@
 BSD 3-Clause License
 
-Copyright (c) 2016-2020, 360 Enterprise Security Group, Endpoint Security, Inc.
+Copyright (c) 2016-2020 The excelize Authors.
 All rights reserved.
 
 Redistribution and use in source and binary forms, with or without

chart.go

@@ -730,28 +730,14 @@ func parseFormatChartSet(formatSet string) (*formatChart, error) {
 //    }
 //
 func (f *File) AddChart(sheet, cell, format string, combo ...string) error {
-	formatSet, err := parseFormatChartSet(format)
-	if err != nil {
-		return err
-	}
-	comboCharts := []*formatChart{}
-	for _, comboFormat := range combo {
-		comboChart, err := parseFormatChartSet(comboFormat)
-		if err != nil {
-			return err
-		}
-		if _, ok := chartValAxNumFmtFormatCode[comboChart.Type]; !ok {
-			return errors.New("unsupported chart type " + comboChart.Type)
-		}
-		comboCharts = append(comboCharts, comboChart)
-	}
 	// Read sheet data.
 	xlsx, err := f.workSheetReader(sheet)
 	if err != nil {
 		return err
 	}
-	if _, ok := chartValAxNumFmtFormatCode[formatSet.Type]; !ok {
-		return errors.New("unsupported chart type " + formatSet.Type)
+	formatSet, comboCharts, err := f.getFormatChart(format, combo)
+	if err != nil {
+		return err
 	}
 	// Add first picture for given sheet, create xl/drawings/ and xl/drawings/_rels/ folder.
 	drawingID := f.countDrawings() + 1
@@ -777,31 +763,18 @@ func (f *File) AddChart(sheet, cell, format string, combo ...string) error {
 func (f *File) AddChartSheet(sheet, format string, combo ...string) error {
 	// Check if the worksheet already exists
 	if f.GetSheetIndex(sheet) != 0 {
-		return errors.New("already existing name worksheet")
+		return errors.New("the same name worksheet already exists")
 	}
-	formatSet, err := parseFormatChartSet(format)
+	formatSet, comboCharts, err := f.getFormatChart(format, combo)
 	if err != nil {
 		return err
 	}
-	comboCharts := []*formatChart{}
-	for _, comboFormat := range combo {
-		comboChart, err := parseFormatChartSet(comboFormat)
-		if err != nil {
-			return err
-		}
-		if _, ok := chartValAxNumFmtFormatCode[comboChart.Type]; !ok {
-			return errors.New("unsupported chart type " + comboChart.Type)
-		}
-		comboCharts = append(comboCharts, comboChart)
-	}
-	if _, ok := chartValAxNumFmtFormatCode[formatSet.Type]; !ok {
-		return errors.New("unsupported chart type " + formatSet.Type)
-	}
 	cs := xlsxChartsheet{
 		SheetViews: []*xlsxChartsheetViews{{
 			SheetView: []*xlsxChartsheetView{{ZoomScaleAttr: 100, ZoomToFitAttr: true}}},
 		},
 	}
+	f.SheetCount++
 	wb := f.workbookReader()
 	sheetID := 0
 	for _, v := range wb.Sheets.Sheet {
@@ -819,10 +792,7 @@ func (f *File) AddChartSheet(sheet, format string, combo ...string) error {
 	drawingID, drawingXML = f.prepareChartSheetDrawing(&cs, drawingID, sheet, drawingXML)
 	drawingRels := "xl/drawings/_rels/drawing" + strconv.Itoa(drawingID) + ".xml.rels"
 	drawingRID := f.addRels(drawingRels, SourceRelationshipChart, "../charts/chart"+strconv.Itoa(chartID)+".xml", "")
-	err = f.addSheetDrawingChart(sheet, drawingXML, formatSet.Dimension.Width, formatSet.Dimension.Height, drawingRID, &formatSet.Format)
-	if err != nil {
-		return err
-	}
+	f.addSheetDrawingChart(drawingXML, drawingRID, &formatSet.Format)
 	f.addChart(formatSet, comboCharts)
 	f.addContentTypePart(chartID, "chart")
 	f.addContentTypePart(sheetID, "chartsheet")
@@ -831,11 +801,35 @@ func (f *File) AddChartSheet(sheet, format string, combo ...string) error {
 	rID := f.addRels("xl/_rels/workbook.xml.rels", SourceRelationshipChartsheet, fmt.Sprintf("chartsheets/sheet%d.xml", sheetID), "")
 	// Update xl/workbook.xml
 	f.setWorkbook(sheet, sheetID, rID)
-	v, _ := xml.Marshal(cs)
-	f.saveFileList(path, replaceRelationshipsBytes(replaceWorkSheetsRelationshipsNameSpaceBytes(v)))
+	chartsheet, _ := xml.Marshal(cs)
+	f.saveFileList(path, replaceRelationshipsBytes(replaceRelationshipsNameSpaceBytes(chartsheet)))
 	return err
 }
 
+// getFormatChart provides a function to check format set of the chart and
+// create chart format.
+func (f *File) getFormatChart(format string, combo []string) (*formatChart, []*formatChart, error) {
+	comboCharts := []*formatChart{}
+	formatSet, err := parseFormatChartSet(format)
+	if err != nil {
+		return formatSet, comboCharts, err
+	}
+	for _, comboFormat := range combo {
+		comboChart, err := parseFormatChartSet(comboFormat)
+		if err != nil {
+			return formatSet, comboCharts, err
+		}
+		if _, ok := chartValAxNumFmtFormatCode[comboChart.Type]; !ok {
+			return formatSet, comboCharts, errors.New("unsupported chart type " + comboChart.Type)
+		}
+		comboCharts = append(comboCharts, comboChart)
+	}
+	if _, ok := chartValAxNumFmtFormatCode[formatSet.Type]; !ok {
+		return formatSet, comboCharts, errors.New("unsupported chart type " + formatSet.Type)
+	}
+	return formatSet, comboCharts, err
+}
+
 // DeleteChart provides a function to delete chart in XLSX by given worksheet
 // and cell name.
 func (f *File) DeleteChart(sheet, cell string) (err error) {

chart_test.go

@@ -198,6 +198,8 @@ func TestAddChart(t *testing.T) {
 		assert.NoError(t, f.AddChart("Combo Charts", axis, fmt.Sprintf(`{"type":"areaStacked","series":[{"name":"Sheet1!$A$30","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$30:$D$30"},{"name":"Sheet1!$A$31","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$31:$D$31"},{"name":"Sheet1!$A$32","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$32:$D$32"},{"name":"Sheet1!$A$33","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$33:$D$33"}],"format":{"x_scale":1.0,"y_scale":1.0,"x_offset":15,"y_offset":10,"print_obj":true,"lock_aspect_ratio":false,"locked":false},"legend":{"position":"left","show_legend_key":false},"title":{"name":"%s"},"plotarea":{"show_bubble_size":true,"show_cat_name":false,"show_leader_lines":false,"show_percent":true,"show_series_name":true,"show_val":true}}`, props[1]), fmt.Sprintf(`{"type":"%s","series":[{"name":"Sheet1!$A$34","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$34:$D$34"},{"name":"Sheet1!$A$35","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$35:$D$35"},{"name":"Sheet1!$A$36","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$36:$D$36"},{"name":"Sheet1!$A$37","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$37:$D$37"}],"format":{"x_scale":1.0,"y_scale":1.0,"x_offset":15,"y_offset":10,"print_obj":true,"lock_aspect_ratio":false,"locked":false},"legend":{"position":"left","show_legend_key":false},"plotarea":{"show_bubble_size":true,"show_cat_name":false,"show_leader_lines":false,"show_percent":true,"show_series_name":true,"show_val":true}}`, props[0])))
 	}
 	assert.NoError(t, f.SaveAs(filepath.Join("test", "TestAddChart.xlsx")))
+	// Test with illegal cell coordinates
+	assert.EqualError(t, f.AddChart("Sheet2", "A", `{"type":"col","series":[{"name":"Sheet1!$A$30","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$30:$D$30"},{"name":"Sheet1!$A$31","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$31:$D$31"},{"name":"Sheet1!$A$32","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$32:$D$32"},{"name":"Sheet1!$A$33","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$33:$D$33"},{"name":"Sheet1!$A$34","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$34:$D$34"},{"name":"Sheet1!$A$35","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$35:$D$35"},{"name":"Sheet1!$A$36","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$36:$D$36"},{"name":"Sheet1!$A$37","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$37:$D$37"}],"format":{"x_scale":1.0,"y_scale":1.0,"x_offset":15,"y_offset":10,"print_obj":true,"lock_aspect_ratio":false,"locked":false},"legend":{"position":"left","show_legend_key":false},"title":{"name":"2D Column Chart"},"plotarea":{"show_bubble_size":true,"show_cat_name":false,"show_leader_lines":false,"show_percent":true,"show_series_name":true,"show_val":true},"show_blanks_as":"zero"}`), `cannot convert cell "A" to coordinates: invalid cell name "A"`)
 	// Test with unsupported chart type
 	assert.EqualError(t, f.AddChart("Sheet2", "BD32", `{"type":"unknown","series":[{"name":"Sheet1!$A$30","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$30:$D$30"},{"name":"Sheet1!$A$31","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$31:$D$31"},{"name":"Sheet1!$A$32","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$32:$D$32"},{"name":"Sheet1!$A$33","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$33:$D$33"},{"name":"Sheet1!$A$34","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$34:$D$34"},{"name":"Sheet1!$A$35","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$35:$D$35"},{"name":"Sheet1!$A$36","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$36:$D$36"},{"name":"Sheet1!$A$37","categories":"Sheet1!$B$29:$D$29","values":"Sheet1!$B$37:$D$37"}],"format":{"x_scale":1.0,"y_scale":1.0,"x_offset":15,"y_offset":10,"print_obj":true,"lock_aspect_ratio":false,"locked":false},"legend":{"position":"left","show_legend_key":false},"title":{"name":"Bubble 3D Chart"},"plotarea":{"show_bubble_size":true,"show_cat_name":false,"show_leader_lines":false,"show_percent":true,"show_series_name":true,"show_val":true},"show_blanks_as":"zero"}`), "unsupported chart type unknown")
 	// Test add combo chart with invalid format set
@@ -217,8 +219,20 @@ func TestAddChartSheet(t *testing.T) {
 		assert.NoError(t, f.SetCellValue("Sheet1", k, v))
 	}
 	assert.NoError(t, f.AddChartSheet("Chart1", `{"type":"col3DClustered","series":[{"name":"Sheet1!$A$2","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$2:$D$2"},{"name":"Sheet1!$A$3","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$3:$D$3"},{"name":"Sheet1!$A$4","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$4:$D$4"}],"title":{"name":"Fruit 3D Clustered Column Chart"}}`))
-
-	assert.EqualError(t, f.AddChartSheet("Sheet1", `{"type":"col3DClustered","series":[{"name":"Sheet1!$A$2","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$2:$D$2"},{"name":"Sheet1!$A$3","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$3:$D$3"},{"name":"Sheet1!$A$4","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$4:$D$4"}],"title":{"name":"Fruit 3D Clustered Column Chart"}}`), "already existing name worksheet")
+	// Test set the chartsheet as active sheet
+	var sheetID int
+	for idx, sheetName := range f.GetSheetMap() {
+		if sheetName != "Chart1" {
+			continue
+		}
+		sheetID = idx
+	}
+	f.SetActiveSheet(sheetID)
+
+	// Test cell value on chartsheet
+	assert.EqualError(t, f.SetCellValue("Chart1", "A1", true), "sheet Chart1 is chart sheet")
+	// Test add chartsheet on already existing name sheet
+	assert.EqualError(t, f.AddChartSheet("Sheet1", `{"type":"col3DClustered","series":[{"name":"Sheet1!$A$2","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$2:$D$2"},{"name":"Sheet1!$A$3","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$3:$D$3"},{"name":"Sheet1!$A$4","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$4:$D$4"}],"title":{"name":"Fruit 3D Clustered Column Chart"}}`), "the same name worksheet already exists")
 	// Test with unsupported chart type
 	assert.EqualError(t, f.AddChartSheet("Chart2", `{"type":"unknown","series":[{"name":"Sheet1!$A$2","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$2:$D$2"},{"name":"Sheet1!$A$3","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$3:$D$3"},{"name":"Sheet1!$A$4","categories":"Sheet1!$B$1:$D$1","values":"Sheet1!$B$4:$D$4"}],"title":{"name":"Fruit 3D Clustered Column Chart"}}`), "unsupported chart type unknown")
 

drawing.go

@@ -1232,10 +1232,7 @@ func (f *File) addDrawingChart(sheet, drawingXML, cell string, width, height, rI
 // addSheetDrawingChart provides a function to add chart graphic frame for
 // chartsheet by given sheet, drawingXML, width, height, relationship index
 // and format sets.
-func (f *File) addSheetDrawingChart(sheet, drawingXML string, width, height, rID int, formatSet *formatPicture) (err error) {
-	width = int(float64(width) * formatSet.XScale)
-	height = int(float64(height) * formatSet.YScale)
-
+func (f *File) addSheetDrawingChart(drawingXML string, rID int, formatSet *formatPicture) {
 	content, cNvPrID := f.drawingParser(drawingXML)
 	absoluteAnchor := xdrCellAnchor{
 		EditAs: formatSet.Positioning,
@@ -1269,7 +1266,7 @@ func (f *File) addSheetDrawingChart(sheet, drawingXML string, width, height, rID
 	}
 	content.AbsoluteAnchor = append(content.AbsoluteAnchor, &absoluteAnchor)
 	f.Drawings[drawingXML] = content
-	return err
+	return
 }
 
 // deleteDrawing provides a function to delete chart graphic frame by given by

excelize.go

@@ -156,6 +156,10 @@ func (f *File) workSheetReader(sheet string) (xlsx *xlsxWorksheet, err error) {
 		return
 	}
 	if xlsx = f.Sheet[name]; f.Sheet[name] == nil {
+		if strings.HasPrefix(name, "xl/chartsheets") {
+			err = fmt.Errorf("sheet %s is chart sheet", sheet)
+			return
+		}
 		xlsx = new(xlsxWorksheet)
 		if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readXML(name)))).
 			Decode(xlsx); err != nil && err != io.EOF {
@@ -227,9 +231,9 @@ func (f *File) addRels(relPath, relType, target, targetMode string) int {
 	return rID
 }
 
-// replaceWorkSheetsRelationshipsNameSpaceBytes provides a function to replace
+// replaceRelationshipsNameSpaceBytes provides a function to replace
 // XML tags to self-closing for compatible Microsoft Office Excel 2007.
-func replaceWorkSheetsRelationshipsNameSpaceBytes(contentMarshal []byte) []byte {
+func replaceRelationshipsNameSpaceBytes(contentMarshal []byte) []byte {
 	var oldXmlns = []byte(` xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main">`)
 	var newXmlns = []byte(templateNamespaceIDMap)
 	contentMarshal = bytes.Replace(contentMarshal, oldXmlns, newXmlns, -1)

rows.go

@@ -174,7 +174,7 @@ func (f *File) Rows(sheet string) (*Rows, error) {
 	if f.Sheet[name] != nil {
 		// flush data
 		output, _ := xml.Marshal(f.Sheet[name])
-		f.saveFileList(name, replaceWorkSheetsRelationshipsNameSpaceBytes(output))
+		f.saveFileList(name, replaceRelationshipsNameSpaceBytes(output))
 	}
 	var (
 		err       error

sheet.go

@@ -119,7 +119,7 @@ func (f *File) workSheetWriter() {
 				f.Sheet[p].SheetData.Row[k].C = trimCell(v.C)
 			}
 			output, _ := xml.Marshal(sheet)
-			f.saveFileList(p, replaceRelationshipsBytes(replaceWorkSheetsRelationshipsNameSpaceBytes(output)))
+			f.saveFileList(p, replaceRelationshipsBytes(replaceRelationshipsNameSpaceBytes(output)))
 			ok := f.checked[p]
 			if ok {
 				delete(f.Sheet, p)
@@ -190,7 +190,7 @@ func (f *File) relsWriter() {
 		if rel != nil {
 			output, _ := xml.Marshal(rel)
 			if strings.HasPrefix(path, "xl/worksheets/sheet/rels/sheet") {
-				output = replaceWorkSheetsRelationshipsNameSpaceBytes(output)
+				output = replaceRelationshipsNameSpaceBytes(output)
 			}
 			f.saveFileList(path, replaceRelationshipsBytes(output))
 		}
@@ -211,19 +211,6 @@ func replaceRelationshipsBytes(content []byte) []byte {
 	return bytes.Replace(content, oldXmlns, newXmlns, -1)
 }
 
-// replaceRelationshipsNameSpaceBytes; Some tools that read XLSX files have
-// very strict requirements about the structure of the input XML. In
-// particular both Numbers on the Mac and SAS dislike inline XML namespace
-// declarations, or namespace prefixes that don't match the ones that Excel
-// itself uses. This is a problem because the Go XML library doesn't multiple
-// namespace declarations in a single element of a document. This function is
-// a horrible hack to fix that after the XML marshalling is completed.
-func replaceRelationshipsNameSpaceBytes(workbookMarshal []byte) []byte {
-	oldXmlns := []byte(`<workbook xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main">`)
-	newXmlns := []byte(`<workbook` + templateNamespaceIDMap)
-	return bytes.Replace(workbookMarshal, oldXmlns, newXmlns, -1)
-}
-
 // SetActiveSheet provides function to set default active worksheet of XLSX by
 // given index. Note that active index is different from the index returned by
 // function GetSheetMap(). It should be greater than 0 and less than total
@@ -248,7 +235,11 @@ func (f *File) SetActiveSheet(index int) {
 		}
 	}
 	for idx, name := range f.GetSheetMap() {
-		xlsx, _ := f.workSheetReader(name)
+		xlsx, err := f.workSheetReader(name)
+		if err != nil {
+			// Chartsheet
+			return
+		}
 		if xlsx.SheetViews == nil {
 			xlsx.SheetViews = &xlsxSheetViews{
 				SheetView: []xlsxSheetView{{WorkbookViewID: 0}},
@@ -370,8 +361,8 @@ func (f *File) getSheetMap() map[string]string {
 				// Construct a target XML as xl/worksheets/sheet%d by split path, compatible with different types of relative paths in workbook.xml.rels, for example: worksheets/sheet%d.xml and /xl/worksheets/sheet%d.xml
 				pathInfo := strings.Split(rel.Target, "/")
 				pathInfoLen := len(pathInfo)
-				if pathInfoLen > 0 {
-					maps[v.Name] = fmt.Sprintf("xl/worksheets/%s", pathInfo[pathInfoLen-1])
+				if pathInfoLen > 1 {
+					maps[v.Name] = fmt.Sprintf("xl/%s", strings.Join(pathInfo[pathInfoLen-2:], "/"))
 				}
 			}
 		}
@@ -420,7 +411,10 @@ func (f *File) DeleteSheet(name string) {
 				for _, rel := range wbRels.Relationships {
 					if rel.ID == sheet.ID {
 						sheetXML = fmt.Sprintf("xl/%s", rel.Target)
-						rels = strings.Replace(fmt.Sprintf("xl/%s.rels", rel.Target), "xl/worksheets/", "xl/worksheets/_rels/", -1)
+						pathInfo := strings.Split(rel.Target, "/")
+						if len(pathInfo) == 2 {
+							rels = fmt.Sprintf("xl/%s/_rels/%s.rels", pathInfo[0], pathInfo[1])
+						}
 					}
 				}
 			}
@@ -430,6 +424,7 @@ func (f *File) DeleteSheet(name string) {
 			delete(f.sheetMap, sheetName)
 			delete(f.XLSX, sheetXML)
 			delete(f.XLSX, rels)
+			delete(f.Relationships, rels)
 			delete(f.Sheet, sheetXML)
 			f.SheetCount--
 		}
@@ -729,7 +724,7 @@ func (f *File) SearchSheet(sheet, value string, reg ...bool) ([]string, error) {
 	if f.Sheet[name] != nil {
 		// flush data
 		output, _ := xml.Marshal(f.Sheet[name])
-		f.saveFileList(name, replaceWorkSheetsRelationshipsNameSpaceBytes(output))
+		f.saveFileList(name, replaceRelationshipsNameSpaceBytes(output))
 	}
 	return f.searchSheet(name, value, regSearch)
 }

styles.go

@@ -1018,7 +1018,7 @@ func (f *File) stylesReader() *xlsxStyleSheet {
 func (f *File) styleSheetWriter() {
 	if f.Styles != nil {
 		output, _ := xml.Marshal(f.Styles)
-		f.saveFileList("xl/styles.xml", replaceWorkSheetsRelationshipsNameSpaceBytes(output))
+		f.saveFileList("xl/styles.xml", replaceRelationshipsNameSpaceBytes(output))
 	}
 }