ExcelOpenSpoutExporter: add support for non-string values #359
Conversation
We encountered an issue where we had a `render` callable which returned an int. That broke the exporter because I assumed that the value is always a string. I had the idea (when looking at e.g. NumberColumn) that a callable given to `render` should always return a string. But that's not very clear in the documentation. But I also changed our `render` function to return a string.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #359 +/- ##
=======================================
Coverage 94.69% 94.69%
=======================================
Files 37 37
Lines 999 999
=======================================
Hits 946 946
Misses 53 53 ☔ View full report in Codecov by Sentry. |
|
|
|
Okay that clears things up and makes sense, thanks! This PR will not ignore an integer if it encounters one. It will pass the value directly to OpenSpout using Cell::fromValue(). OpenSpout will then write the value as a Number type in XSLX. If a value is passed that has a type which is not supported by the function above, there will likely be a TypeError. I could change this behaviour to instead try to convert the value to a string, and if that doesn't work, just silently ignore the value In case of strings, they will be truncated and stripped of HTML. Other types will not be modified and passed to OpenSpout as-is. |
mhvis
changed the title
|
Oops, Github diff view hid that you're still inserting it in line 62 indeed. Then it's fine like this indeed. |
We encountered an issue where we had a
rendercallable which returned an int. That broke the exporter because I assumed that the value is always a string.I had the idea (when looking at e.g. NumberColumn) that a callable given to
rendershould always return a string. But that's not very clear in the documentation. But I also changed ourrenderfunction to return a string.