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

Use more standard representation of boolean values in cells #62

Merged
merged 2 commits into from
May 2, 2023

Conversation

pineapplemachine
Copy link

See https://bugs.documentfoundation.org/show_bug.cgi?id=155046

Unfortunately the OOXML ECMA-376 spec does not ever seem to explicitly mention what is expected for a <v> cell value element within a <c> cell element where t="b" indicates a boolean value type. But it seems that excel4node's writing the text "true" and "false" instead of the text "0" and "1" within <v> elements representing boolean values is uncommon behavior.

With this change, boolean cells now display as expected in LibreOffice Calc and Google Sheets, where previously they did not.

I verified on my system that npm run test and node sample.js && ./validate.sh Excel.xlsx both run successfully after building.

This is an improved implementation of the same fix as in #59.

See https://bugs.documentfoundation.org/show_bug.cgi?id=155046

Unfortunately the OOXML ECMA-376 spec does not ever seem to explicitly mention what is expected for a `<v>` element within a `<c> element with `t="b"`. But it seems that excel4node's writing the text "true" and "false" instead of the text "0" and "1" within `<v>` elements representing boolean values is uncommon behavior.
@jrohland
Copy link

jrohland commented May 2, 2023

This looks good. I added additional boolean cells to the sample.js to verify the output: 4572571

Comparing the output from main to this pull request in google docs shows the correct values now being rendered. Left is current version, right is this pull request.
image

LibreOffice Calc is also now rendering boolean true values correctly:
image

@jrohland jrohland merged commit f66bc70 into advisr-io:main May 2, 2023
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.

2 participants