This closes qax-os#849, add new function DeleteComment for delete c…

…omment (qax-os#1317)

- Update unit tests for the delete comment
- Add 3 errors function for error messages

comment.go

@@ -140,6 +140,39 @@ func (f *File) AddComment(sheet, cell, format string) error {
 	return err
 }
 
+// DeleteComment provides the method to delete comment in a sheet by given
+// worksheet. For example, delete the comment in Sheet1!$A$30:
+//
+//	err := f.DeleteComment("Sheet1", "A30")
+func (f *File) DeleteComment(sheet, cell string) (err error) {
+	sheetXMLPath, ok := f.getSheetXMLPath(sheet)
+	if !ok {
+		err = newNoExistSheetError(sheet)
+		return
+	}
+	commentsXML := f.getSheetComments(filepath.Base(sheetXMLPath))
+	if !strings.HasPrefix(commentsXML, "/") {
+		commentsXML = "xl" + strings.TrimPrefix(commentsXML, "..")
+	}
+	commentsXML = strings.TrimPrefix(commentsXML, "/")
+	if comments := f.commentsReader(commentsXML); comments != nil {
+		for i, cmt := range comments.CommentList.Comment {
+			if cmt.Ref == cell {
+				if len(comments.CommentList.Comment) > 1 {
+					comments.CommentList.Comment = append(
+						comments.CommentList.Comment[:i],
+						comments.CommentList.Comment[i+1:]...,
+					)
+					continue
+				}
+				comments.CommentList.Comment = nil
+			}
+		}
+		f.Comments[commentsXML] = comments
+	}
+	return
+}
+
 // addDrawingVML provides a function to create comment as
 // xl/drawings/vmlDrawing%d.vml by given commit ID and cell.
 func (f *File) addDrawingVML(commentID int, drawingVML, cell string, lineCount, colCount int) error {

comment_test.go

@@ -46,6 +46,32 @@ func TestAddComments(t *testing.T) {
 	assert.EqualValues(t, len(NewFile().GetComments()), 0)
 }
 
+func TestDeleteComment(t *testing.T) {
+	f, err := prepareTestBook1()
+	if !assert.NoError(t, err) {
+		t.FailNow()
+	}
+
+	assert.NoError(t, f.AddComment("Sheet2", "A40", `{"author":"Excelize: ","text":"This is a comment1."}`))
+	assert.NoError(t, f.AddComment("Sheet2", "A41", `{"author":"Excelize: ","text":"This is a comment2."}`))
+	assert.NoError(t, f.AddComment("Sheet2", "C41", `{"author":"Excelize: ","text":"This is a comment3."}`))
+
+	assert.NoError(t, f.DeleteComment("Sheet2", "A40"))
+
+	assert.EqualValues(t, 2, len(f.GetComments()["Sheet2"]))
+	assert.EqualValues(t, len(NewFile().GetComments()), 0)
+
+	// Test delete all comments in a worksheet
+	assert.NoError(t, f.DeleteComment("Sheet2", "A41"))
+	assert.NoError(t, f.DeleteComment("Sheet2", "C41"))
+	assert.EqualValues(t, 0, len(f.GetComments()["Sheet2"]))
+	// Test delete comment on not exists worksheet
+	assert.EqualError(t, f.DeleteComment("SheetN", "A1"), "sheet SheetN is not exist")
+	// Test delete comment with worksheet part
+	f.Pkg.Delete("xl/worksheets/sheet1.xml")
+	assert.NoError(t, f.DeleteComment("Sheet1", "A22"))
+}
+
 func TestDecodeVMLDrawingReader(t *testing.T) {
 	f := NewFile()
 	path := "xl/drawings/vmlDrawing1.xml"

docProps.go

@@ -14,7 +14,6 @@ package excelize
 import (
 	"bytes"
 	"encoding/xml"
-	"fmt"
 	"io"
 	"reflect"
 )
@@ -76,7 +75,7 @@ func (f *File) SetAppProps(appProperties *AppProperties) (err error) {
 	app = new(xlsxProperties)
 	if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readXML(defaultXMLPathDocPropsApp)))).
 		Decode(app); err != nil && err != io.EOF {
-		err = fmt.Errorf("xml decode error: %s", err)
+		err = newDecodeXMLError(err)
 		return
 	}
 	fields = []string{"Application", "ScaleCrop", "DocSecurity", "Company", "LinksUpToDate", "HyperlinksChanged", "AppVersion"}
@@ -103,7 +102,7 @@ func (f *File) GetAppProps() (ret *AppProperties, err error) {
 	app := new(xlsxProperties)
 	if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readXML(defaultXMLPathDocPropsApp)))).
 		Decode(app); err != nil && err != io.EOF {
-		err = fmt.Errorf("xml decode error: %s", err)
+		err = newDecodeXMLError(err)
 		return
 	}
 	ret, err = &AppProperties{
@@ -181,7 +180,7 @@ func (f *File) SetDocProps(docProperties *DocProperties) (err error) {
 	core = new(decodeCoreProperties)
 	if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readXML(defaultXMLPathDocPropsCore)))).
 		Decode(core); err != nil && err != io.EOF {
-		err = fmt.Errorf("xml decode error: %s", err)
+		err = newDecodeXMLError(err)
 		return
 	}
 	newProps, err = &xlsxCoreProperties{
@@ -236,7 +235,7 @@ func (f *File) GetDocProps() (ret *DocProperties, err error) {
 
 	if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readXML(defaultXMLPathDocPropsCore)))).
 		Decode(core); err != nil && err != io.EOF {
-		err = fmt.Errorf("xml decode error: %s", err)
+		err = newDecodeXMLError(err)
 		return
 	}
 	ret, err = &DocProperties{

drawing.go

@@ -14,7 +14,6 @@ package excelize
 import (
 	"bytes"
 	"encoding/xml"
-	"fmt"
 	"io"
 	"log"
 	"reflect"
@@ -1322,7 +1321,7 @@ func (f *File) deleteDrawing(col, row int, drawingXML, drawingType string) (err
 		deTwoCellAnchor = new(decodeTwoCellAnchor)
 		if err = f.xmlNewDecoder(strings.NewReader("<decodeTwoCellAnchor>" + wsDr.TwoCellAnchor[idx].GraphicFrame + "</decodeTwoCellAnchor>")).
 			Decode(deTwoCellAnchor); err != nil && err != io.EOF {
-			err = fmt.Errorf("xml decode error: %s", err)
+			err = newDecodeXMLError(err)
 			return
 		}
 		if err = nil; deTwoCellAnchor.From != nil && decodeTwoCellAnchorFuncs[drawingType](deTwoCellAnchor) {

errors.go

@@ -70,6 +70,23 @@ func newCellNameToCoordinatesError(cell string, err error) error {
 	return fmt.Errorf("cannot convert cell %q to coordinates: %v", cell, err)
 }
 
+// newNoExistSheetError defined the error message on receiving the not exist
+// sheet name.
+func newNoExistSheetError(name string) error {
+	return fmt.Errorf("sheet %s is not exist", name)
+}
+
+// newNotWorksheetError defined the error message on receiving a sheet which
+// not a worksheet.
+func newNotWorksheetError(name string) error {
+	return fmt.Errorf("sheet %s is not a worksheet", name)
+}
+
+// newDecodeXMLError defined the error message on decode XML error.
+func newDecodeXMLError(err error) error {
+	return fmt.Errorf("xml decode error: %s", err)
+}
+
 var (
 	// ErrStreamSetColWidth defined the error message on set column width in
 	// stream writing mode.

excelize.go

@@ -231,7 +231,7 @@ func (f *File) workSheetReader(sheet string) (ws *xlsxWorksheet, err error) {
 		ok   bool
 	)
 	if name, ok = f.getSheetXMLPath(sheet); !ok {
-		err = fmt.Errorf("sheet %s is not exist", sheet)
+		err = newNoExistSheetError(sheet)
 		return
 	}
 	if worksheet, ok := f.Sheet.Load(name); ok && worksheet != nil {
@@ -240,7 +240,7 @@ func (f *File) workSheetReader(sheet string) (ws *xlsxWorksheet, err error) {
 	}
 	for _, sheetType := range []string{"xl/chartsheets", "xl/dialogsheet", "xl/macrosheet"} {
 		if strings.HasPrefix(name, sheetType) {
-			err = fmt.Errorf("sheet %s is not a worksheet", sheet)
+			err = newNotWorksheetError(sheet)
 			return
 		}
 	}
@@ -251,7 +251,7 @@ func (f *File) workSheetReader(sheet string) (ws *xlsxWorksheet, err error) {
 	}
 	if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readBytes(name)))).
 		Decode(ws); err != nil && err != io.EOF {
-		err = fmt.Errorf("xml decode error: %s", err)
+		err = newDecodeXMLError(err)
 		return
 	}
 	err = nil
@@ -424,7 +424,7 @@ func (f *File) UpdateLinkedValue() error {
 	for _, name := range f.GetSheetList() {
 		ws, err := f.workSheetReader(name)
 		if err != nil {
-			if err.Error() == fmt.Sprintf("sheet %s is not a worksheet", trimSheetName(name)) {
+			if err.Error() == newNotWorksheetError(name).Error() {
 				continue
 			}
 			return err

picture.go

@@ -15,7 +15,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"encoding/xml"
-	"fmt"
 	"image"
 	"io"
 	"io/ioutil"
@@ -554,15 +553,15 @@ func (f *File) getPicture(row, col int, drawingXML, drawingRelationships string)
 	deWsDr = new(decodeWsDr)
 	if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readXML(drawingXML)))).
 		Decode(deWsDr); err != nil && err != io.EOF {
-		err = fmt.Errorf("xml decode error: %s", err)
+		err = newDecodeXMLError(err)
 		return
 	}
 	err = nil
 	for _, anchor := range deWsDr.TwoCellAnchor {
 		deTwoCellAnchor = new(decodeTwoCellAnchor)
 		if err = f.xmlNewDecoder(strings.NewReader("<decodeTwoCellAnchor>" + anchor.Content + "</decodeTwoCellAnchor>")).
 			Decode(deTwoCellAnchor); err != nil && err != io.EOF {
-			err = fmt.Errorf("xml decode error: %s", err)
+			err = newDecodeXMLError(err)
 			return
 		}
 		if err = nil; deTwoCellAnchor.From != nil && deTwoCellAnchor.Pic != nil {

stream.go

@@ -92,7 +92,7 @@ type StreamWriter struct {
 func (f *File) NewStreamWriter(sheet string) (*StreamWriter, error) {
 	sheetID := f.getSheetID(sheet)
 	if sheetID == -1 {
-		return nil, fmt.Errorf("sheet %s is not exist", sheet)
+		return nil, newNoExistSheetError(sheet)
 	}
 	sw := &StreamWriter{
 		File:    f,