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

Incorrect Attribute Value for forceFullCalc in writeCalcPr Method #4108

Closed
heikyomuraichi opened this issue Jul 23, 2024 · 2 comments
Closed

Comments

@heikyomuraichi
Copy link

$objWriter->writeAttribute('forceFullCalc', ($recalcRequired) ? '0' : '1');

Issue Summary
The writeCalcPr method in the class contains an incorrect attribute value for forceFullCalc. The current implementation sets forceFullCalc to 0 when $recalcRequired is true, and to 1 when $recalcRequired is false. This seems to be the opposite of the intended behavior.

Suggested Fix
The attribute forceFullCalc should be set to 1 when $recalcRequired is true, and to 0 when $recalcRequired is false. This would correctly reflect the need for a full recalculation when recalcRequired is true.

$objWriter->writeAttribute('forceFullCalc', ($recalcRequired) ? '1' : '0'); 
@oleibman
Copy link
Collaborator

I believe PhpSpreadsheet is producing the desired result, but the parameter is, unfortunately, misnamed. When preCalculateFormulas (and therefore recalcRequired) is false, I see the following in the output:

<calcPr calcId="999999" calcMode="auto" calcCompleted="0" fullCalcOnLoad="1" forceFullCalc="1"/>

When it's true, I see the following:

<calcPr calcId="999999" calcMode="auto" calcCompleted="1" fullCalcOnLoad="0" forceFullCalc="0"/>

I think that both are as they should be. Do you have an example where you feel otherwise?

@heikyomuraichi
Copy link
Author

Thank you for the clarification. I understand now that the actual behavior of the writeCalcPr method is correct and that the naming of the parameter might be a bit misleading. I don't have an example where the current implementation produces incorrect results. It was a misunderstanding on my part.

Thank you for your assistance and explanation.

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Jul 25, 2024
See issue PHPOffice#4108. Parameter recalcRequired is a complete misnomer. While we can get along without changing it, I think we should strive for accuracy. Execution logic is unchanged, and existing tests are adequate. This isn't really intended as a public interface, and, even if is being used by someone out there, it will be a problem only for people calling the parameter by name. So the chances of a problem are very low, and the workaround is very easy (call positionally).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants