This closes qax-os#1204, breaking changes for add comments

- Allowing insert SVG format images
- Unit tests updated

.gitignore

@@ -1,15 +1,15 @@
+.DS_Store
+.idea
+*.json
+*.out
+*.test
 ~$*.xlsx
+test/*.png
+test/BadWorkbook.SaveAsEmptyStruct.xlsx
+test/Encryption*.xlsx
+test/excelize-*
 test/Test*.xlam
 test/Test*.xlsm
 test/Test*.xlsx
 test/Test*.xltm
 test/Test*.xltx
-# generated files
-test/Encryption*.xlsx
-test/BadWorkbook.SaveAsEmptyStruct.xlsx
-test/*.png
-test/excelize-*
-*.out
-*.test
-.idea
-.DS_Store

cell.go

@@ -902,31 +902,7 @@ func getCellRichText(si *xlsxSI) (runs []RichTextRun) {
 			Text: v.T.Val,
 		}
 		if v.RPr != nil {
-			font := Font{Underline: "none"}
-			font.Bold = v.RPr.B != nil
-			font.Italic = v.RPr.I != nil
-			if v.RPr.U != nil {
-				font.Underline = "single"
-				if v.RPr.U.Val != nil {
-					font.Underline = *v.RPr.U.Val
-				}
-			}
-			if v.RPr.RFont != nil && v.RPr.RFont.Val != nil {
-				font.Family = *v.RPr.RFont.Val
-			}
-			if v.RPr.Sz != nil && v.RPr.Sz.Val != nil {
-				font.Size = *v.RPr.Sz.Val
-			}
-			font.Strike = v.RPr.Strike != nil
-			if v.RPr.Color != nil {
-				font.Color = strings.TrimPrefix(v.RPr.Color.RGB, "FF")
-				if v.RPr.Color.Theme != nil {
-					font.ColorTheme = v.RPr.Color.Theme
-				}
-				font.ColorIndexed = v.RPr.Color.Indexed
-				font.ColorTint = v.RPr.Color.Tint
-			}
-			run.Font = &font
+			run.Font = newFont(v.RPr)
 		}
 		runs = append(runs, run)
 	}
@@ -985,6 +961,35 @@ func newRpr(fnt *Font) *xlsxRPr {
 	return &rpr
 }
 
+// newFont create font format by given run properties for the rich text.
+func newFont(rPr *xlsxRPr) *Font {
+	font := Font{Underline: "none"}
+	font.Bold = rPr.B != nil
+	font.Italic = rPr.I != nil
+	if rPr.U != nil {
+		font.Underline = "single"
+		if rPr.U.Val != nil {
+			font.Underline = *rPr.U.Val
+		}
+	}
+	if rPr.RFont != nil && rPr.RFont.Val != nil {
+		font.Family = *rPr.RFont.Val
+	}
+	if rPr.Sz != nil && rPr.Sz.Val != nil {
+		font.Size = *rPr.Sz.Val
+	}
+	font.Strike = rPr.Strike != nil
+	if rPr.Color != nil {
+		font.Color = strings.TrimPrefix(rPr.Color.RGB, "FF")
+		if rPr.Color.Theme != nil {
+			font.ColorTheme = rPr.Color.Theme
+		}
+		font.ColorIndexed = rPr.Color.Indexed
+		font.ColorTint = rPr.Color.Tint
+	}
+	return &font
+}
+
 // setRichText provides a function to set rich text of a cell.
 func setRichText(runs []RichTextRun) ([]xlsxR, error) {
 	var (

comment.go

@@ -13,7 +13,6 @@ package excelize
 
 import (
 	"bytes"
-	"encoding/json"
 	"encoding/xml"
 	"fmt"
 	"io"
@@ -23,17 +22,6 @@ import (
 	"strings"
 )
 
-// parseCommentOptions provides a function to parse the format settings of
-// the comment with default value.
-func parseCommentOptions(opts string) (*commentOptions, error) {
-	options := commentOptions{
-		Author: "Author:",
-		Text:   " ",
-	}
-	err := json.Unmarshal([]byte(opts), &options)
-	return &options, err
-}
-
 // GetComments retrieves all comments and returns a map of worksheet name to
 // the worksheet comments.
 func (f *File) GetComments() (comments map[string][]Comment) {
@@ -53,14 +41,18 @@ func (f *File) GetComments() (comments map[string][]Comment) {
 				if comment.AuthorID < len(d.Authors.Author) {
 					sheetComment.Author = d.Authors.Author[comment.AuthorID]
 				}
-				sheetComment.Ref = comment.Ref
+				sheetComment.Cell = comment.Ref
 				sheetComment.AuthorID = comment.AuthorID
 				if comment.Text.T != nil {
 					sheetComment.Text += *comment.Text.T
 				}
 				for _, text := range comment.Text.R {
 					if text.T != nil {
-						sheetComment.Text += text.T.Val
+						run := RichTextRun{Text: text.T.Val}
+						if text.RPr != nil {
+							run.Font = newFont(text.RPr)
+						}
+						sheetComment.Runs = append(sheetComment.Runs, run)
 					}
 				}
 				sheetComments = append(sheetComments, sheetComment)
@@ -92,12 +84,15 @@ func (f *File) getSheetComments(sheetFile string) string {
 // author length is 255 and the max text length is 32512. For example, add a
 // comment in Sheet1!$A$30:
 //
-//	err := f.AddComment("Sheet1", "A30", `{"author":"Excelize: ","text":"This is a comment."}`)
-func (f *File) AddComment(sheet, cell, opts string) error {
-	options, err := parseCommentOptions(opts)
-	if err != nil {
-		return err
-	}
+//	err := f.AddComment(sheet, excelize.Comment{
+//	    Cell:   "A12",
+//	    Author: "Excelize",
+//	    Runs: []excelize.RichTextRun{
+//	        {Text: "Excelize: ", Font: &excelize.Font{Bold: true}},
+//	        {Text: "This is a comment."},
+//	    },
+//	})
+func (f *File) AddComment(sheet string, comment Comment) error {
 	// Read sheet data.
 	ws, err := f.workSheetReader(sheet)
 	if err != nil {
@@ -122,20 +117,19 @@ func (f *File) AddComment(sheet, cell, opts string) error {
 		f.addSheetLegacyDrawing(sheet, rID)
 	}
 	commentsXML := "xl/comments" + strconv.Itoa(commentID) + ".xml"
-	var colCount int
-	for i, l := range strings.Split(options.Text, "\n") {
-		if ll := len(l); ll > colCount {
-			if i == 0 {
-				ll += len(options.Author)
+	var rows, cols int
+	for _, runs := range comment.Runs {
+		for _, subStr := range strings.Split(runs.Text, "\n") {
+			rows++
+			if chars := len(subStr); chars > cols {
+				cols = chars
 			}
-			colCount = ll
 		}
 	}
-	err = f.addDrawingVML(commentID, drawingVML, cell, strings.Count(options.Text, "\n")+1, colCount)
-	if err != nil {
+	if err = f.addDrawingVML(commentID, drawingVML, comment.Cell, rows+1, cols); err != nil {
 		return err
 	}
-	f.addComment(commentsXML, cell, options)
+	f.addComment(commentsXML, comment)
 	f.addContentTypePart(commentID, "comments")
 	return err
 }
@@ -280,56 +274,59 @@ func (f *File) addDrawingVML(commentID int, drawingVML, cell string, lineCount,
 
 // addComment provides a function to create chart as xl/comments%d.xml by
 // given cell and format sets.
-func (f *File) addComment(commentsXML, cell string, opts *commentOptions) {
-	a := opts.Author
-	t := opts.Text
-	if len(a) > MaxFieldLength {
-		a = a[:MaxFieldLength]
+func (f *File) addComment(commentsXML string, comment Comment) {
+	if comment.Author == "" {
+		comment.Author = "Author"
 	}
-	if len(t) > 32512 {
-		t = t[:32512]
+	if len(comment.Author) > MaxFieldLength {
+		comment.Author = comment.Author[:MaxFieldLength]
 	}
-	comments := f.commentsReader(commentsXML)
-	authorID := 0
+	comments, authorID := f.commentsReader(commentsXML), 0
 	if comments == nil {
-		comments = &xlsxComments{Authors: xlsxAuthor{Author: []string{opts.Author}}}
+		comments = &xlsxComments{Authors: xlsxAuthor{Author: []string{comment.Author}}}
 	}
-	if inStrSlice(comments.Authors.Author, opts.Author, true) == -1 {
-		comments.Authors.Author = append(comments.Authors.Author, opts.Author)
+	if inStrSlice(comments.Authors.Author, comment.Author, true) == -1 {
+		comments.Authors.Author = append(comments.Authors.Author, comment.Author)
 		authorID = len(comments.Authors.Author) - 1
 	}
-	defaultFont := f.GetDefaultFont()
-	bold := ""
-	cmt := xlsxComment{
-		Ref:      cell,
+	defaultFont, chars, cmt := f.GetDefaultFont(), 0, xlsxComment{
+		Ref:      comment.Cell,
 		AuthorID: authorID,
-		Text: xlsxText{
-			R: []xlsxR{
-				{
-					RPr: &xlsxRPr{
-						B:  &bold,
-						Sz: &attrValFloat{Val: float64Ptr(9)},
-						Color: &xlsxColor{
-							Indexed: 81,
-						},
-						RFont:  &attrValString{Val: stringPtr(defaultFont)},
-						Family: &attrValInt{Val: intPtr(2)},
-					},
-					T: &xlsxT{Val: a},
-				},
-				{
-					RPr: &xlsxRPr{
-						Sz: &attrValFloat{Val: float64Ptr(9)},
-						Color: &xlsxColor{
-							Indexed: 81,
-						},
-						RFont:  &attrValString{Val: stringPtr(defaultFont)},
-						Family: &attrValInt{Val: intPtr(2)},
-					},
-					T: &xlsxT{Val: t},
+		Text:     xlsxText{R: []xlsxR{}},
+	}
+	if comment.Text != "" {
+		if len(comment.Text) > TotalCellChars {
+			comment.Text = comment.Text[:TotalCellChars]
+		}
+		cmt.Text.T = stringPtr(comment.Text)
+		chars += len(comment.Text)
+	}
+	for _, run := range comment.Runs {
+		if chars == TotalCellChars {
+			break
+		}
+		if chars+len(run.Text) > TotalCellChars {
+			run.Text = run.Text[:TotalCellChars-chars]
+		}
+		chars += len(run.Text)
+		r := xlsxR{
+			RPr: &xlsxRPr{
+				Sz: &attrValFloat{Val: float64Ptr(9)},
+				Color: &xlsxColor{
+					Indexed: 81,
 				},
+				RFont:  &attrValString{Val: stringPtr(defaultFont)},
+				Family: &attrValInt{Val: intPtr(2)},
 			},
-		},
+			T: &xlsxT{Val: run.Text, Space: xml.Attr{
+				Name:  xml.Name{Space: NameSpaceXML, Local: "space"},
+				Value: "preserve",
+			}},
+		}
+		if run.Font != nil {
+			r.RPr = newRpr(run.Font)
+		}
+		cmt.Text.R = append(cmt.Text.R, r)
 	}
 	comments.CommentList.Comment = append(comments.CommentList.Comment, cmt)
 	f.Comments[commentsXML] = comments

comment_test.go

@@ -26,14 +26,14 @@ func TestAddComments(t *testing.T) {
 		t.FailNow()
 	}
 
-	s := strings.Repeat("c", 32768)
-	assert.NoError(t, f.AddComment("Sheet1", "A30", `{"author":"`+s+`","text":"`+s+`"}`))
-	assert.NoError(t, f.AddComment("Sheet2", "B7", `{"author":"Excelize: ","text":"This is a comment."}`))
+	s := strings.Repeat("c", TotalCellChars+1)
+	assert.NoError(t, f.AddComment("Sheet1", Comment{Cell: "A30", Author: s, Text: s, Runs: []RichTextRun{{Text: s}, {Text: s}}}))
+	assert.NoError(t, f.AddComment("Sheet2", Comment{Cell: "B7", Author: "Excelize", Text: s[:TotalCellChars-1], Runs: []RichTextRun{{Text: "Excelize: ", Font: &Font{Bold: true}}, {Text: "This is a comment."}}}))
 
 	// Test add comment on not exists worksheet.
-	assert.EqualError(t, f.AddComment("SheetN", "B7", `{"author":"Excelize: ","text":"This is a comment."}`), "sheet SheetN does not exist")
+	assert.EqualError(t, f.AddComment("SheetN", Comment{Cell: "B7", Author: "Excelize", Runs: []RichTextRun{{Text: "Excelize: ", Font: &Font{Bold: true}}, {Text: "This is a comment."}}}), "sheet SheetN does not exist")
 	// Test add comment on with illegal cell reference
-	assert.EqualError(t, f.AddComment("Sheet1", "A", `{"author":"Excelize: ","text":"This is a comment."}`), newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
+	assert.EqualError(t, f.AddComment("Sheet1", Comment{Cell: "A", Author: "Excelize", Runs: []RichTextRun{{Text: "Excelize: ", Font: &Font{Bold: true}}, {Text: "This is a comment."}}}), newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
 	if assert.NoError(t, f.SaveAs(filepath.Join("test", "TestAddComments.xlsx"))) {
 		assert.Len(t, f.GetComments(), 2)
 	}
@@ -52,12 +52,12 @@ func TestDeleteComment(t *testing.T) {
 		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.AddComment("Sheet2", "C41", `{"author":"Excelize: ","text":"This is a comment3-1."}`))
-	assert.NoError(t, f.AddComment("Sheet2", "C42", `{"author":"Excelize: ","text":"This is a comment4."}`))
-	assert.NoError(t, f.AddComment("Sheet2", "C41", `{"author":"Excelize: ","text":"This is a comment3-2."}`))
+	assert.NoError(t, f.AddComment("Sheet2", Comment{Cell: "A40", Text: "Excelize: This is a comment1."}))
+	assert.NoError(t, f.AddComment("Sheet2", Comment{Cell: "A41", Runs: []RichTextRun{{Text: "Excelize: ", Font: &Font{Bold: true}}, {Text: "This is a comment2."}}}))
+	assert.NoError(t, f.AddComment("Sheet2", Comment{Cell: "C41", Runs: []RichTextRun{{Text: "Excelize: ", Font: &Font{Bold: true}}, {Text: "This is a comment3."}}}))
+	assert.NoError(t, f.AddComment("Sheet2", Comment{Cell: "C41", Runs: []RichTextRun{{Text: "Excelize: ", Font: &Font{Bold: true}}, {Text: "This is a comment3-1."}}}))
+	assert.NoError(t, f.AddComment("Sheet2", Comment{Cell: "C42", Runs: []RichTextRun{{Text: "Excelize: ", Font: &Font{Bold: true}}, {Text: "This is a comment4."}}}))
+	assert.NoError(t, f.AddComment("Sheet2", Comment{Cell: "C41", Runs: []RichTextRun{{Text: "Excelize: ", Font: &Font{Bold: true}}, {Text: "This is a comment2."}}}))
 
 	assert.NoError(t, f.DeleteComment("Sheet2", "A40"))
 

excelize_test.go

@@ -946,8 +946,7 @@ func TestSetDeleteSheet(t *testing.T) {
 			t.FailNow()
 		}
 		f.DeleteSheet("Sheet1")
-		assert.EqualError(t, f.AddComment("Sheet1", "A1", ""), "unexpected end of JSON input")
-		assert.NoError(t, f.AddComment("Sheet1", "A1", `{"author":"Excelize: ","text":"This is a comment."}`))
+		assert.NoError(t, f.AddComment("Sheet1", Comment{Cell: "A1", Author: "Excelize", Runs: []RichTextRun{{Text: "Excelize: ", Font: &Font{Bold: true}}, {Text: "This is a comment."}}}))
 		assert.NoError(t, f.SaveAs(filepath.Join("test", "TestSetDeleteSheet.TestBook4.xlsx")))
 	})
 }

picture.go

@@ -183,7 +183,7 @@ func (f *File) AddPictureFromBytes(sheet, cell, opts, name, extension string, fi
 		drawingHyperlinkRID = f.addRels(drawingRels, SourceRelationshipHyperLink, options.Hyperlink, hyperlinkType)
 	}
 	ws.Unlock()
-	err = f.addDrawingPicture(sheet, drawingXML, cell, name, img.Width, img.Height, drawingRID, drawingHyperlinkRID, options)
+	err = f.addDrawingPicture(sheet, drawingXML, cell, name, ext, drawingRID, drawingHyperlinkRID, img, options)
 	if err != nil {
 		return err
 	}
@@ -263,11 +263,12 @@ func (f *File) countDrawings() (count int) {
 // addDrawingPicture provides a function to add picture by given sheet,
 // drawingXML, cell, file name, width, height relationship index and format
 // sets.
-func (f *File) addDrawingPicture(sheet, drawingXML, cell, file string, width, height, rID, hyperlinkRID int, opts *pictureOptions) error {
+func (f *File) addDrawingPicture(sheet, drawingXML, cell, file, ext string, rID, hyperlinkRID int, img image.Config, opts *pictureOptions) error {
 	col, row, err := CellNameToCoordinates(cell)
 	if err != nil {
 		return err
 	}
+	width, height := img.Width, img.Height
 	if opts.Autofit {
 		width, height, col, row, err = f.drawingResize(sheet, cell, float64(width), float64(height), opts)
 		if err != nil {
@@ -308,6 +309,19 @@ func (f *File) addDrawingPicture(sheet, drawingXML, cell, file string, width, he
 	}
 	pic.BlipFill.Blip.R = SourceRelationship.Value
 	pic.BlipFill.Blip.Embed = "rId" + strconv.Itoa(rID)
+	if ext == ".svg" {
+		pic.BlipFill.Blip.ExtList = &xlsxEGOfficeArtExtensionList{
+			Ext: []xlsxCTOfficeArtExtension{
+				{
+					URI: ExtURISVG,
+					SVGBlip: xlsxCTSVGBlip{
+						XMLNSaAVG: NameSpaceDrawing2016SVG.Value,
+						Embed:     pic.BlipFill.Blip.Embed,
+					},
+				},
+			},
+		}
+	}
 	pic.SpPr.PrstGeom.Prst = "rect"
 
 	twoCellAnchor.Pic = &pic
@@ -362,7 +376,10 @@ func (f *File) addMedia(file []byte, ext string) string {
 // setContentTypePartImageExtensions provides a function to set the content
 // type for relationship parts and the Main Document part.
 func (f *File) setContentTypePartImageExtensions() {
-	imageTypes := map[string]string{"jpeg": "image/", "png": "image/", "gif": "image/", "tiff": "image/", "emf": "image/x-", "wmf": "image/x-", "emz": "image/x-", "wmz": "image/x-"}
+	imageTypes := map[string]string{
+		"jpeg": "image/", "png": "image/", "gif": "image/", "svg": "image/", "tiff": "image/",
+		"emf": "image/x-", "wmf": "image/x-", "emz": "image/x-", "wmz": "image/x-",
+	}
 	content := f.contentTypesReader()
 	content.Lock()
 	defer content.Unlock()