Resolve #345, fix missing comments by GetComments

chart.go

@@ -789,7 +789,7 @@ func (f *File) AddChartSheet(sheet, format string, combo ...string) error {
 	drawingID := f.countDrawings() + 1
 	chartID := f.countCharts() + 1
 	drawingXML := "xl/drawings/drawing" + strconv.Itoa(drawingID) + ".xml"
-	drawingID, drawingXML = f.prepareChartSheetDrawing(&cs, drawingID, sheet, drawingXML)
+	f.prepareChartSheetDrawing(&cs, drawingID, sheet)
 	drawingRels := "xl/drawings/_rels/drawing" + strconv.Itoa(drawingID) + ".xml.rels"
 	drawingRID := f.addRels(drawingRels, SourceRelationshipChart, "../charts/chart"+strconv.Itoa(chartID)+".xml", "")
 	f.addSheetDrawingChart(drawingXML, drawingRID, &formatSet.Format)

comment.go

@@ -16,6 +16,7 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"path/filepath"
 	"strconv"
 	"strings"
 )
@@ -35,8 +36,8 @@ func parseFormatCommentsSet(formatSet string) (*formatComment, error) {
 // the worksheet comments.
 func (f *File) GetComments() (comments map[string][]Comment) {
 	comments = map[string][]Comment{}
-	for n := range f.sheetMap {
-		if d := f.commentsReader("xl" + strings.TrimPrefix(f.getSheetComments(f.GetSheetIndex(n)), "..")); d != nil {
+	for n, path := range f.sheetMap {
+		if d := f.commentsReader("xl" + strings.TrimPrefix(f.getSheetComments(filepath.Base(path)), "..")); d != nil {
 			sheetComments := []Comment{}
 			for _, comment := range d.CommentList.Comment {
 				sheetComment := Comment{}
@@ -60,9 +61,9 @@ func (f *File) GetComments() (comments map[string][]Comment) {
 }
 
 // getSheetComments provides the method to get the target comment reference by
-// given worksheet index.
-func (f *File) getSheetComments(sheetID int) string {
-	var rels = "xl/worksheets/_rels/sheet" + strconv.Itoa(sheetID) + ".xml.rels"
+// given worksheet file path.
+func (f *File) getSheetComments(sheetFile string) string {
+	var rels = "xl/worksheets/_rels/" + sheetFile + ".rels"
 	if sheetRels := f.relsReader(rels); sheetRels != nil {
 		for _, v := range sheetRels.Relationships {
 			if v.Type == SourceRelationshipComments {
@@ -107,7 +108,6 @@ func (f *File) AddComment(sheet, cell, format string) error {
 		f.addSheetLegacyDrawing(sheet, rID)
 	}
 	commentsXML := "xl/comments" + strconv.Itoa(commentID) + ".xml"
-	f.addComment(commentsXML, cell, formatSet)
 	var colCount int
 	for i, l := range strings.Split(formatSet.Text, "\n") {
 		if ll := len(l); ll > colCount {
@@ -121,6 +121,7 @@ func (f *File) AddComment(sheet, cell, format string) error {
 	if err != nil {
 		return err
 	}
+	f.addComment(commentsXML, cell, formatSet)
 	f.addContentTypePart(commentID, "comments")
 	return err
 }

comment_test.go

@@ -29,10 +29,17 @@ func TestAddComments(t *testing.T) {
 
 	// Test add comment on not exists worksheet.
 	assert.EqualError(t, f.AddComment("SheetN", "B7", `{"author":"Excelize: ","text":"This is a comment."}`), "sheet SheetN is not exist")
-
+	// Test add comment on with illegal cell coordinates
+	assert.EqualError(t, f.AddComment("Sheet1", "A", `{"author":"Excelize: ","text":"This is a comment."}`), `cannot convert cell "A" to coordinates: invalid cell name "A"`)
 	if assert.NoError(t, f.SaveAs(filepath.Join("test", "TestAddComments.xlsx"))) {
 		assert.Len(t, f.GetComments(), 2)
 	}
+
+	f.Comments["xl/comments2.xml"] = nil
+	f.XLSX["xl/comments2.xml"] = []byte(`<?xml version="1.0" encoding="UTF-8" standalone="yes"?><comments xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"><authors><author>Excelize: </author></authors><commentList><comment ref="B7" authorId="0"><text><t>Excelize: </t></text></comment></commentList></comments>`)
+	comments := f.GetComments()
+	assert.EqualValues(t, 2, len(comments["Sheet1"]))
+	assert.EqualValues(t, 1, len(comments["Sheet2"]))
 }
 
 func TestDecodeVMLDrawingReader(t *testing.T) {

drawing.go

@@ -40,22 +40,15 @@ func (f *File) prepareDrawing(xlsx *xlsxWorksheet, drawingID int, sheet, drawing
 
 // prepareChartSheetDrawing provides a function to prepare drawing ID and XML
 // by given drawingID, worksheet name and default drawingXML.
-func (f *File) prepareChartSheetDrawing(xlsx *xlsxChartsheet, drawingID int, sheet, drawingXML string) (int, string) {
+func (f *File) prepareChartSheetDrawing(xlsx *xlsxChartsheet, drawingID int, sheet string) {
 	sheetRelationshipsDrawingXML := "../drawings/drawing" + strconv.Itoa(drawingID) + ".xml"
-	if xlsx.Drawing != nil {
-		// The worksheet already has a picture or chart relationships, use the relationships drawing ../drawings/drawing%d.xml.
-		sheetRelationshipsDrawingXML = f.getSheetRelationshipsTargetByID(sheet, xlsx.Drawing.RID)
-		drawingID, _ = strconv.Atoi(strings.TrimSuffix(strings.TrimPrefix(sheetRelationshipsDrawingXML, "../drawings/drawing"), ".xml"))
-		drawingXML = strings.Replace(sheetRelationshipsDrawingXML, "..", "xl", -1)
-	} else {
-		// Add first picture for given sheet.
-		sheetRels := "xl/chartsheets/_rels/" + strings.TrimPrefix(f.sheetMap[trimSheetName(sheet)], "xl/chartsheets/") + ".rels"
-		rID := f.addRels(sheetRels, SourceRelationshipDrawingML, sheetRelationshipsDrawingXML, "")
-		xlsx.Drawing = &xlsxDrawing{
-			RID: "rId" + strconv.Itoa(rID),
-		}
+	// Only allow one chart in a chartsheet.
+	sheetRels := "xl/chartsheets/_rels/" + strings.TrimPrefix(f.sheetMap[trimSheetName(sheet)], "xl/chartsheets/") + ".rels"
+	rID := f.addRels(sheetRels, SourceRelationshipDrawingML, sheetRelationshipsDrawingXML, "")
+	xlsx.Drawing = &xlsxDrawing{
+		RID: "rId" + strconv.Itoa(rID),
 	}
-	return drawingID, drawingXML
+	return
 }
 
 // addChart provides a function to create chart as xl/charts/chart%d.xml by

drawing_test.go

@@ -0,0 +1,27 @@
+// Copyright 2016 - 2020 The excelize Authors. All rights reserved. Use of
+// this source code is governed by a BSD-style license that can be found in
+// the LICENSE file.
+//
+// Package excelize providing a set of functions that allow you to write to
+// and read from XLSX files. Support reads and writes XLSX file generated by
+// Microsoft Excel™ 2007 and later. Support save file without losing original
+// charts of XLSX. This library needs Go version 1.10 or later.
+
+package excelize
+
+import (
+	"testing"
+)
+
+func TestDrawingParser(t *testing.T) {
+	f := File{
+		Drawings: make(map[string]*xlsxWsDr),
+		XLSX: map[string][]byte{
+			"charset": MacintoshCyrillicCharset,
+			"wsDr":    []byte(`<?xml version="1.0" encoding="UTF-8" standalone="yes"?><xdr:wsDr xmlns:xdr="http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing"><xdr:oneCellAnchor><xdr:graphicFrame/></xdr:oneCellAnchor></xdr:wsDr>`)},
+	}
+	// Test with one cell anchor
+	f.drawingParser("wsDr")
+	// Test with unsupport charset
+	f.drawingParser("charset")
+}

excelize_test.go

@@ -934,7 +934,7 @@ func TestCopySheetError(t *testing.T) {
 
 func TestGetSheetComments(t *testing.T) {
 	f := NewFile()
-	assert.Equal(t, "", f.getSheetComments(0))
+	assert.Equal(t, "", f.getSheetComments("sheet0"))
 }
 
 func TestSetActiveSheet(t *testing.T) {