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

Add recalc to xlsxC.getValueFrom() #1823

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mikeschinkel
Copy link

@mikeschinkel mikeschinkel commented Feb 24, 2024

PR Details

Adds a maybeRecalc() method to address the issue mentioned discussion #1822.

Description

Adds a maybeRecalc() method to xlsxc to be called immediately after entering the getFromValue() method.
This calls File.CalcCellValue() when xlsxc.F != nil and xlsxc.R != "" and xlsxc.V not one of "", 0, or 0.0.

This is a draft PR with many TODOs that bring up questions I had about if I was doing this the best way. I also ask questions to ensure I am using the right internal features and if I am following the vision of the project.

NOTE, when I was about to submit this I read the checklist and then went through the very thorough contributing guidelines.

Alas, I realizes there are many things I did not do — and since I am already running bind on my other deadlines I don't have time to do them all right now.

So I could have chosen either to not submit this as a draft PR — as I wrote it to solve my own current issues — or submit it so that it could be reviewed in connection with the referenced discussion and possibly updated to turn into a non-draft PR that could be merged later.

I chose the latter. I hope that you agree with me that I made the right choice.

Related Issue

Discussion #1822.

Motivation and Context

Ensures that zero values are not returned for sheets where there is a formula and a zero value, and while Excel shows a value excelize otherwise returns a zero value because of what is in the <v> element.

This issue surfaces when calling File.GetRows(), and maybe other methods.

I wrote this fix to meet my own needs, and am submitting it as a draft PR rather than just keeping it to myself.

How Has This Been Tested

I have only done manual testing as it is only a draft PR meant to spur discussion.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document.

@xuri xuri added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 24, 2024
@xuri xuri force-pushed the master branch 2 times, most recently from 79958aa to 0c3dfb1 Compare May 25, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants