Ods Reader Sheet Names with Period in Addresses and Formulas #4313
+109
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix PHPOffice#4311. Period is a valid character in a sheet name. When a sheet with such a name is referenced in Ods format, the sheet name must be enclosed in apostrophes, because Ods uses period to separate sheet name from cell address. (Excel uses exclamation point so doesn't necessarily need to enclose the sheet name in apostrophes.) This causes a problem for the Ods Reader whenever it tries to parse such an address; however, the problem showed up specifically for auto filters, because the Ods xml for those specifies *'sheetname'.startcell:'sheetname'.endcell* (Excel omits sheetname). Ods Reader translates these addresses in 2 different methods in FormulaTranslator. I had a relatively elegant method for handling this situation in convertToExcelAddressValue, but I could not make it work in convertToExcelFormulaValue. A kludgier method works for Formula, and also for Address. I decided it's better to be consistent, so I'm going with the kludgier method for both. It would not surprise me in the least if there are similar problems lying in wait for other special characters in sheet names, and for other formats besides Ods. For now, I will limit myself to fixing the known problem.
|
Scrutinizer "new issue" is a false positive, and is now suppressed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #4311. Period is a valid character in a sheet name. When a sheet with such a name is referenced in Ods format, the sheet name must be enclosed in apostrophes, because Ods uses period to separate sheet name from cell address. (Excel uses exclamation point so doesn't necessarily need to enclose the sheet name in apostrophes.) This causes a problem for the Ods Reader whenever it tries to parse such an address; however, the problem showed up specifically for auto filters, because the Ods xml for those specifies 'sheetname'.startcell:'sheetname'.endcell (Excel omits sheetname).
Ods Reader translates these addresses in 2 different methods in FormulaTranslator. I had a relatively elegant method for handling this situation in convertToExcelAddressValue, but I could not make it work in convertToExcelFormulaValue. A kludgier method works for Formula, and also for Address. I decided it's better to be consistent, so I'm going with the kludgier method for both.
It would not surprise me in the least if there are similar problems lying in wait for other special characters in sheet names, and for other formats besides Ods. For now, I will limit myself to fixing the known problem.
This is:
Checklist:
Why this change is needed?
Provide an explanation of why this change is needed, with links to any Issues (if appropriate).
If this is a bugfix or a new feature, and there are no existing Issues, then please also create an issue that will make it easier to track progress with this PR.