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

Exporting FormatStyle structs #470

Closed
wants to merge 6 commits into from
Closed

Conversation

trende-jp
Copy link

PR Details

Description

Exporting FormatStyles.

Motivation and Context

Much friendlier for any user of the library.

How Has This Been Tested

Using existing tests.

Types of changes

  • 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.
  • All new and existing tests passed.

@@ -1,4 +1,4 @@
module github.com/360EntSecGroup-Skylar/excelize/v2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you are developing locally with go modules you can use replace to change where the code is pulled from to test your local changes instead of renaming the module.

styles.go Outdated
@@ -1885,13 +1885,9 @@ func parseFormatStyleSet(style string) (*formatStyle, error) {
//
// Cell Sheet1!A6 in the Excel Application: martes, 04 de Julio de 2017
//
func (f *File) NewStyle(style string) (int, error) {
func (f *File) NewStyle(fs *FormatStyle) (int, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should add a new function that takes the newly exposed style struct instead of changing the signature of NewStyle.

As it stands, we would have to release v3 of the library since you are making a breaking change to how people would consume it. If you add a new function to take the struct like NewStyleStruct or AddStyle (or something...) then this can go out as v2.1.0.

All the private functions you changed seem fine since consumers can't see those.

@mlh758
Copy link
Contributor

mlh758 commented Aug 9, 2019

You could use iota to give names to the numbers used in border styles and such which would further help users consume this.

@trende-jp
Copy link
Author

Reverted to something api compatible. As for iota, I will leave that alone for now.

@trende-jp
Copy link
Author

Is it possible to get this approved? I am running into performance issues, but my code is based on the templates being passed objects rather than strings.

@xuri xuri added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 18, 2019
@xuri xuri closed this in 9e2318c Mar 9, 2020
@xuri
Copy link
Member

xuri commented Mar 9, 2020

I exported Style structs to allow create the style for cells by given JSON or structure.

nullfy pushed a commit to nullfy/excelize that referenced this pull request Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants