excelize
excelize copied to clipboard
DRAFT: Handle and return more errors
PR Details
Description
Several errors are never checked against
Changes regarding error handling
- some getter funcs now just return a pointer, there are new
New*()
funcs to handle initialization; full list:-
NewCalcChainReader()
-
NewContentTypesReader()
-
NewStylesReader()
-
NewThemeReader()
-
NewWorkbookReader()
- NOTE: they are exported for now, but can un-export them if requested
- The reason they were separated in getters and
New*()
is to prevent more funcs that return errors where it's not necessary. Especially as those funcs are used pretty heavily
-
- in some cases, no error was returned, because existing code already handled error -> new code in same area of said funcs respect the existing handling
- in some cases, it seems errors should be fine, in those cases I want to at least return the first error (see
firstError
). And if an error occurred, handle it usingcontinue
or whatever approach fits best the respective context
new internals
-
getSheetNameBySheetXMLPath()
: get worksheet name // by checking the sheet map against the given XML file path- plus its unit test
- errors
-
ErrIncompleteFileSetup
-
ErrRangeLength
-
newRangeLengthError
(for further detail on error above)
-
changed excels
- Book1.xlsx (see comment)
Minor changes picked up on:
- in NewSheet(), don't call DeleteSheet() -> not needed, because if sheet exists, it is returned
- in OpenReader() also set: ContentTypes, WorkBook
Related Issue
closes #1267
Motivation and Context
- fixing potential crashes
- enable library user to react and potentially handle errors
- currently, some funcs fail silently or could crash
How Has This Been Tested
- running package tests locally (using golang version 1.16)
Types of changes
- [x] Docs change / refactoring / dependency upgrade
- [x] Breaking change (fix or feature that would cause existing functionality to change)
- because of new error returns
Checklist
- [x] My code follows the code style of this project.
- [x] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- in code comments, add info regarding
firstError
returned
- in code comments, add info regarding
- [x] I have read the CONTRIBUTING document.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.
- [ ] cleanup commits once everything else is fine: Sign the work, commit message lingua
Misc
Open to feedback
Hi, Yes, I'm working on the failing unit tests.
Would appreciate help with errors
- ~~
error in sheet "Sheet2": overlapRange: invalid range length: range of "A6" is smaller than minimum range length of 2
~~ see next comment - ~~
xml decode error: XML syntax error on line 1: invalid UTF-8
~~- seems like the error checks were wrong, fixed with newest commit
@xuri:
ok, it seems Book1.xlsx had cell A6 flagged as "merged cell". A cell merge of one cell makes no sense
Therefore, I deleted cell A6 in the excel
or was that intentional? Guessing because of this line
value, err = f.GetCellValue("Sheet2", "A6") // Merged cell ref is single coordinate.
If it was intentional, it would make sense to delete incorrect merges via code, then save corrected result.
@xuri: I'm basically finished, except if any action has to be taken because of situation stated in previous comment.
EDIT: and regarding the currently exported New*()
funcs: Should I "un-export" them?
If no action is required, I'd cleanup my commits
- add / extend func comments (aka documentation)
- redo the commits so that all are signed
I'm thinking of doing 5 commits
- adding the
New*()
funcs - new error messages +
getSheetNameBySheetXMLPath()
(plus its unit test) - the changes this brings to all other funcs
- all unit tests
- updated Book1.xlsx
the commit message contents will be based off the current ones