Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

value fields xlsxPatternFill.FgColor & xlsxPatternFill.BgColor cause … #770

Merged
merged 2 commits into from
Jan 28, 2021
Merged

value fields xlsxPatternFill.FgColor & xlsxPatternFill.BgColor cause … #770

merged 2 commits into from
Jan 28, 2021

Conversation

eaglexiang
Copy link
Contributor

Value fields xlsxPatternFill.FgColor & xlsxPatternFill.BgColor cause ineffective omitempty tags

PR Details

omitempty tag is used at fields xlsxPatternFill.FgColor and xlsxPatternFill.BgColor, but they are both value fields but not pointer fields, which means they are not ignored while xml marshaling.

So I fixed it to pointer fields.

Description

Below is defined at ECMA-376:

18.8.32 patternFill (Pattern)

This element is used to specify cell fill information for pattern and solid color cell fills. For solid cell fills (no
pattern), fgColor is used. For cell fills with patterns specified, then the cell fill color is specified by the bgColor
element.

fgColor is used for solid cell, and fgColor is used for cell filled with color.

Tag omitempty is used to ignore color elements not in need, where the BgColor and FgColor are both value fields but not pointer fields. It means blank xml element will be built.

sample in standard styles.xml:

<fill>
    <patternFill patternType="gray125"/>
</fill>

sample in excelize styles.xml:

<fill>
    <patternFill patternType="gray125">
        <fgColor></fgColor>
        <bgColor></bgColor>
    </patternFill>
</fill>

So what? Files with useless color elements may be not rendered normally in Google Sheets.

Related Issue

#769

Motivation and Context

How Has This Been Tested

package main

import (
	"fmt"
	"strconv"

	"github.com/360EntSecGroup-Skylar/excelize/v2"
)

func main() {
	values := [][]string{
		{
			"1", "1", "1", "1",
		},
		{
			"1", "1", "1", "1",
		},
	}

	f := excelize.NewFile()

	// new sheet
	sheetName := "Sheet1"
	sheet := f.NewSheet(sheetName)

	// write data
	for i, row := range values {
		axis := "A" + strconv.FormatInt(int64(i+1), 10)

		f.SetSheetRow(sheetName, axis, &row)
	}

	// set color
	//
	// styleID, err := f.NewStyle(`{"fill":{"type":"pattern","pattern":1,"color":["#000000"]}}`)
	// if !assert.NoError(t, err) {
	// 	return
	// }
	// f.SetCellStyle("Sheet1", "A1", "B2", styleID)

	f.SetActiveSheet(sheet)

	// Save spreadsheet by the given path.
	if err := f.SaveAs("file.xlsx"); err != nil {
		fmt.Println(err)
	}
}

I wrote a sample code to build test xlsx files. Files built with new code work well in both Google Sheets and MS Office.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@xuri xuri added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 28, 2021
@xuri
Copy link
Member

xuri commented Jan 28, 2021

Hi @eaglexiang, thanks for your PR, I think omitempty tag also can be removed for the pointer fields.

@eaglexiang
Copy link
Contributor Author

Hi @eaglexiang, thanks for your PR, I think omitempty tag also can be removed for the pointer fields.

@xuri You are right

@xuri xuri merged commit 219add2 into qax-os:master Jan 28, 2021
@xuri
Copy link
Member

xuri commented Jan 28, 2021

LGTM, thanks for your great work @eaglexiang

jenbonzhang pushed a commit to jenbonzhang/excelize that referenced this pull request Oct 22, 2023
qax-os#770)

* value fields xlsxPatternFill.FgColor & xlsxPatternFill.BgColor cause ineffective omitempty tags

* remove useless omitempty tag on xlsxPatternFill.FgColor and xlsxPatternFill.BgColor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants