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

Allow column style to be set in addition to column width #81

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Allow column style to be set in addition to column width #81

wants to merge 3 commits into from

Conversation

ryanhart2
Copy link
Contributor

This PR replaces the sheet.col_widths field with a cols field. Each value in cols is a map, which can include multiple attributes for a column, including width, bg_color, num_format etc. The compiler saves column styles in the same way that cell styles are saved, and the relevant style id is included in the xml output for each column. The creation and writing of a file, with only the width set on a column, runs at the same speed as before this PR.

Fixes: #48

The cols field in sheet is a map, which can contain a width value as well as other values e.g. bg_color, numfmt etc.
The compiler registers styles based on properties stored in sheet.cols. The registration method is the same as registering cell styles. The xml template generator looks up the style id and includes in the xml definition for a column.
@xou
Copy link
Owner

xou commented Feb 28, 2019

Thank you very much for all your previous optimizations! I am in the middle of a move and tax season, so my time is a bit limited at the moment, I'll try to have a look at the changes and the merge conflict on the weekend. Thank you!

@ryanhart2
Copy link
Contributor Author

No worries. Thank you for this library! I think that the merge conflict will be a bit tricky because I made substantial changes to make_cols/1 which conflicts with the recent changes made to the same function in #74 which allows for outline level (grouping) support. Let me know what you think once you have had a chance to look. It may be easier for me to update my repo and prepare another PR.

@xou
Copy link
Owner

xou commented Mar 24, 2019

Hey, really sorry about this but I currently can't find the time to work on the merge conflicts. Could you have another look if you don't mind? Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants