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

Include sheet state in listWorksheetInfo #4345

Closed
christian-fries opened this issue Feb 7, 2025 · 3 comments · Fixed by #4366
Closed

Include sheet state in listWorksheetInfo #4345

christian-fries opened this issue Feb 7, 2025 · 3 comments · Fixed by #4366

Comments

@christian-fries
Copy link

The method listWorksheetInfo is great for analyzing an Excel workbook without loading it (and therefore consuming a lot of memory). Sadly, it is missing the sheet state of the worksheets. Adding the sheet state to the output of listWorksheetInfo would make reasoning about workbooks and their worksheets more flexibel.

@oleibman
Copy link
Collaborator

Would you need sheetState returned all the time, or just in cases where it isn't visible? The latter would probably be easier for me, and less likely to be viewed as a breaking change (existing results would change, though it's hard for me to imagine what might break as a result, but sometimes I don't have enough imagination).

@christian-fries
Copy link
Author

For our use cases, we would need a reliable way to detect if a worksheet is visible or not. If the sheetState property is only set if the sheet is not visible, it means that whenever the property is not available, the sheet is visible. Doesn't sound like the best design but it would work for us.

@oleibman
Copy link
Collaborator

Thank you for the input. I am still considering the best approach. The fact that four different input types (csv, html, slk, and xml) don't support any state other than 'visible' is a factor.

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Feb 15, 2025
Fix PHPOffice#4345. Add a new item to the output array. Although the output is changed, this does not seem like a breaking change to me.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants