Make Errors more "narrow"

[RedPhoenixQ]

This was discussed in #810 here.

This splits up error types so that there is almost one type for each module, which narrows the amount of error variants that is returned from each function.

For the functions where two or more error types may be returned the new combined_error! macro is used to create a new error enum that holds these variants with most error impl's automatically done.

The old errors::Error is still present and public. All other new errors implement From<_> for errors::Error so that the provided errors::Result type can still be used to try (?) any function from this crate.

I haven't spent much time adding/editing docs for these changes since most of them still apply without changes. Maybe the docs for errors::Error should be changed to make it clear that it will not be given out as an error from anywhere directly. There are also some names of the new error types that might need changing

[codecov-commenter]

:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 48.20513% with 101 lines in your changes missing coverage. Please review.

Project coverage is 60.14%. Comparing base (39b5905) to head (00a0962). Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
src/errors.rs	4.25%	45 Missing :warning:
src/name.rs	69.13%	25 Missing :warning:
src/encoding.rs	47.05%	18 Missing :warning:
src/events/mod.rs	57.14%	6 Missing :warning:
examples/read_nodes.rs	0.00%	4 Missing :warning:
src/escape.rs	0.00%	3 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #811 +/- ## ========================================== + Coverage 60.08% 60.14% +0.05% ========================================== Files 41 41 Lines 15975 15985 +10 ========================================== + Hits 9599 9614 +15 + Misses 6376 6371 -5 ``` | [Flag](https://app.codecov.io/gh/tafia/quick-xml/pull/811/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/tafia/quick-xml/pull/811/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `60.14% <48.20%> (+0.05%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[RedPhoenixQ]

Sorry for the force pushes, it keeps running my usage example as a doc test and the annotations (like '''rust) mean different things in different rust version apparently

[RedPhoenixQ]

Some doc links became broken, that need to be fixed

Compiling the docs was an oversight. Will fix.

I'm fine with the first 3 commits, but I'm unsure about the 4th (other two commits just the follow-ups fixing the compilation errors). I'm not sure that such a fine breakdown of errors will be convenient for work. In most cases, you will not be interested in a specific type of error and it will still be thrown higher, but the presence of many types of errors will complicate such throwing (it's not for nothing that things like anyhow::Error appeared).

Do you have opposite experience?

I'm mostly interested in not having irrelevant options when calling lower level apis. I will agree that the attributes methods are inconvenient when returning ReadError (like the read_node example). I still think that having these methods return AttrError is better for when only using those apis.

I'm torn on whether this is better, especially when it's common for ReadError and AttrError appear in the same function. I still prefer returning AttrError but I agree it may not be worth it.

If this split beyond EncodingError and NamespaceError is not desired, the combined_error! macro could also be removed entirely.

You can fix things and make a force push, GitHub UI provides a way to compare between force pushes, so it is fine). I would prefer to have a history without fix-up commits, so it would be great if each commit:

• in a compilable state on CI (which means it is compiled fine with all CI tested combinations of flags, in practice usually check of cargo test and cargo test --all-features is enough)

• cargo doc --all-features does not report warnings

• each commit updates changelog with the relevant changes (so it is easely to understand later in which commit that change was made)

Absolutely. The docs was an oversight and I know that one commit doesn't pass the tests. I will go back and rework all of these commits.

[RedPhoenixQ]

When the errors are being changed anyway, there's an opportunity to be consistent about whether Error enum variants should be named SomeError::Io or SomeError::IoError. Any preference here?

[Mingun]

I still think that having these methods return AttrError is better for when only using those apis.

When we declare in API a more wide error that is really could happen, I'm fine to narrow the result type, but introducing new fine-granulated error types for that, I think, would be overkill.

The problem is that the API may not be well-established, and with the introduction of validation checks, it may turn out that some functions will return more errors. Some Linux package maintainers already complained, that quick-xml API changes too quickly :).

If this split beyond EncodingError and NamespaceError is not desired, the combined_error! macro could also be removed entirely.

Yes. So for now please left only the changes from the first 3 commits. Maybe in time the 4th commit also would be welcomed, who knows :)?

Any preference here?

SomeError::Io

[RedPhoenixQ]

I belive I have fixed all issues from the previous review. The changelog is now incrementally updated every commit and all commits pass cargo test and cargo doc (with --all-features).

I have still included the AttrError change as I didn't understand if it should be discarded or not. Will remove it if you want.

[Mingun]

Thanks!