DRAFT: Handle and return more errors

[MJacred]

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 using continue 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

• Docs change / refactoring / dependency upgrade
• Breaking change (fix or feature that would cause existing functionality to change)
  • because of new error returns

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
  • in code comments, add info regarding firstError returned
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• cleanup commits once everything else is fine: Sign the work, commit message lingua

Misc

Open to feedback

[xuri]

Thanks for your PR. This PR contains a lot of code and I need some time to review. The failure unit test case should be fixed.

[MJacred]

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

[MJacred]

@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.

[MJacred]

@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

1. adding the New*() funcs
2. new error messages + getSheetNameBySheetXMLPath() (plus its unit test)
3. the changes this brings to all other funcs
4. all unit tests
5. updated Book1.xlsx

the commit message contents will be based off the current ones