Bug 56892 - [PATCH] Ignoring number-stored-as-text warnings requires going via CTWorksheet
Summary: [PATCH] Ignoring number-stored-as-text warnings requires going via CTWorksheet
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: unspecified
Hardware: PC Linux
: P2 enhancement (vote)
Target Milestone: ---
Assignee: POI Developers List
Keywords: PatchAvailable
Depends on:
Blocks: 46136 58641
  Show dependency tree
Reported: 2014-08-27 11:30 UTC by Chris Boyle
Modified: 2016-10-29 08:33 UTC (History)
1 user (show)

Patch to add XSSFSheet.addNumberStoredAsTextIgnoredError (3.49 KB, patch)
2014-08-27 11:31 UTC, Chris Boyle
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Boyle 2014-08-27 11:30:05 UTC

Comment 1 Chris Boyle 2014-08-27 11:31:44 UTC
Created attachment 31948 [details]
Patch to add XSSFSheet.addNumberStoredAsTextIgnoredError

The attached patch adds methods on XSSFSheet to set number-stored-as-text warnings as ignored for a region without requiring the caller to dig into CTWorksheet.
Comment 2 Nick Burch 2014-08-27 12:02:43 UTC
Thanks for this

Looking at the code and unit test, I wonder if it might be worth making it a bit more general?

For example, define an enum of the kinds of errors that can be ignored (taken from the ones CTIgnoredError defines), then have the method be more like:

addIgnoredError(CellRangeAddress, IgnoredErrorType)

(Or does that need to be a list/array of ignored types? Does Excel support setting multiple?)

Bonus marks (=possible pint of your choice in a handy Oxford pub!) if you could also provide a nice getter for these ignored errors too :)
Comment 3 David North 2015-06-12 15:41:10 UTC
See also bug 54868, bug 46136
Comment 4 David North 2016-01-13 17:16:52 UTC
Excel supports multiple ignoredError elements. Furthermore, the flags on each one are not mutually exclusive, so if a given cell range has the same error(s) they can be expressed by one element.

I'll work on generalising the API as suggested.
Comment 5 David North 2016-01-13 17:54:59 UTC
I've checked in an attempt at a generalised API in SVN r1724469

Nick, is that approximately what you had in mind?

Whether this can be generalised to cover HSSF too is best left for bug 46136, I think. It's unclear whether all the types of ignored error are applicable to the HSSF format.
Comment 6 Nick Burch 2016-01-14 09:44:06 UTC
For the HSSF stuff, look at FeatFormulaErr2

Looks at first glance like the list is almost the same. Assuming that's really the case, it'd probably be best to put the IgnoredErrorType interface into the common SS usermodel, but without the CT stuff. Add a helper / child interface in xssf that does that part

It might be good to create test .xls and .xlsx files, with some ignored errors in them, then write a XSSF-specific getter test for it

I think that writing a HSSF getter for existing ignores should be quite quick, as we have the record code needed. We could then use the same test to check it

Writing the HSSF add will take a bit more, as you need to create / update the right records in the right order
Comment 7 Nick Burch 2016-02-11 15:18:00 UTC
David - are you happy to make these changes, or do you need someone else to try?
Comment 8 Nick Burch 2016-02-15 15:01:25 UTC
As of r1730543, I've moved the enum into the main ss package, and documented those entries that I can from the XLS spec. That leaves a couple needing descriptions that I think are XLSX-only.

I haven't added the matching HSSF support using FeatFormulaErr2, that remains for #46136