Per review feedback on PR #4943:
- Remove the $this->ns property from Reader/Xlsx/Sparklines and the
getNamespaces() call that populated it. The reader now navigates the XML
with the fixed namespace URIs (Namespaces::DATA_VALIDATIONS1 for x14 and
Namespaces::DATA_VALIDATIONS2 for xm) via SimpleXMLElement::children(),
so it no longer depends on the prefixes a third-party writer happens to
use.
- Move the SPARKLINE_URI constant to the Namespaces class (alongside
STYLE_CHECKBOX_URI) and reference it from both the reader and writer.
Add tests for previously-uncovered SparklineGroup setters (axis types,
manual limits, additional display options and colours) and a hand-crafted
Xlsx fixture exercising the reader's edge cases (non-sparkline ext, group
without sparklines, sparkline with empty sqref, missing colour elements).
Harden Reader/Xlsx/Sparklines against malformed input: guard the
sparklineGroup and sparkline iterations against a null children set so an
empty <x14:sparklineGroups> or <x14:sparklines> element no longer emits a
PHP warning.
Implements native sparkline support (line, column, and win/loss) in the
Xlsx reader and writer, resolving issue #4941.
- New model classes under Worksheet/Sparkline: Sparkline, SparklineGroup,
and the SparklineType enum.
- Worksheet gains a sparkline group collection with addSparkline(),
addSparklineGroup(), getSparklineGroupCollection(), and
removeSparklineGroupCollection(), plus deep-clone support.
- Reader/Xlsx/Sparklines parses x14:sparklineGroups from the sheet extLst.
- Writer/Xlsx/Worksheet emits x14:sparklineGroups; writeExtLst is
refactored so conditional-formatting data bars and sparklines share a
single extLst.
- Adds unit tests, an Xlsx round-trip test, a runnable sample, and docs.
Fix#943, which went stale a long time ago, and is now reopened. I believe that the specific problem in that issue was actually mostly resolved some time ago. However, `highestRow/Column` produces questionable results when the delete range overlaps the highest row/column. That is fixed by this PR.
In addition, `removeRow/Column` allow the specification of a non-negative value for `numberOfRows/Columns`. But the expected results in those cases are not defined, and the actual results probably do not meet user expectations. I believe that very few, if any, users are taking advantage of this "feature", but we may as well straighten it out. For 0 rows/columns, I think it makes perfect sense to do nothing. As for negative - if `removeRow(5, 2)` says to remove 2 rows *starting* with 5, I think `removeRow(5, -2)` ought to remove (up to) 2 rows *ending* with 5. The undefined behaviors are changed to act as described in this paragraph.
A number of interesting performance ideas have been submitted by @kemo. PR #4833 optimizes Xls Reader in a number of different ways. One of those, slightly modified, is to use the protected `readFilter` property which is available to all classes which inherit from `BaseReader` (which all our supported readers do), rather than call the `getReadFilter` method to obtain it (often in a loop). That applies to all readers, not just Xls, and is such a good idea that I am fast-tracking it ahead of 4833. Gnumeric, Ods, Xls, Xlsx, and Xml will all receive small performance boosts from it. Csv already does this, so no change is needed there. Html and Slk do not appear to do any filtering; possibly work for another day.
Feroz
oleibman
dependabot[bot]