search

roo-rb / roo-xls

Roo::Xls add support for legacy Excel file standards to Roo.
MIT License
82 stars 82 forks source link

[Excel 2003 XML] Add support for font colors + required fixes #45

Closed ms-ati closed 6 years ago

ms-ati commented 6 years ago

As mentioned in #42, all I'd needed was to add support for the color of a given cell. But unfortunately, I discovered that other pieces of the puzzle were broken along the way.

This PR attempts to fix all that, adding the support for fonts and colors via #font.

Adds RSpec coverage via the following test files (screenshot included for those without Excel): font_colors.xml.zip

font_colors_screenshot_in_mac_excel_16 10

Fixes #41, #42, #43, #44

Also fixes #33 I believe

ms-ati commented 6 years ago

Following semantic versioning, I recommend bumping a MINOR version, to 1.2.0 after this PR is merged, as this code adds new capability (font name, size, and color) into an existing API, and fixes existing bugs.

However -- there may be users depending on the mis-parsing of the cells, especially row and column numbers around merged cells. Therefore, you may need to bump a MAJOR version, to 2.0.0, to reflect incompatible changes for some users.

Does that make sense? Depends how you all are thinking about it.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+1.2%) to 80.977% when pulling 5a06c76e535348e4c737274e6acfaad72f49d74c on ms-ati:excel-2003-xml-support-font-colors into 00ccb4c9c10f028846eb9b0cb1c640b798c944a7 on roo-rb:master.

ms-ati commented 6 years ago

@Empact et all -- any chance of getting eyes on this PR?

Empact commented 6 years ago

This looks great. Thanks! Keep the PRs coming. :)

ms-ati commented 6 years ago

Hurray! Thanks @Empact. I might try to tackle the streaming api question, now that you mention it. In the meantime -- our team would really appreciate it if you could cut a gem release that contains this PR?

Cheers!

-Marc

ms-ati commented 6 years ago

Ping on request for a roo-xls gem release @Empact ?

Empact commented 6 years ago

@ms-ati Released! Could you give it a try? https://rubygems.org/gems/roo-xls/versions/1.2.0

Also a github release https://github.com/roo-rb/roo-xls/releases/tag/v1.2.0

ms-ati commented 6 years ago

Nice @Empact ! Thank you :)