Error docs and cleanup

[chanced]

This is a continuation of #99 which was a continuation of #93.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 81.22530% with 95 lines in your changes missing coverage. Please review.

Project coverage is 95.8%. Comparing base (bf648a4) to head (3a5c61c).

Files with missing lines	Patch %	Lines
src/resolve.rs	81.7%	30 Missing ⚠️
src/pointer.rs	82.2%	24 Missing ⚠️
src/diagnostic.rs	74.4%	22 Missing ⚠️
src/assign.rs	86.3%	13 Missing ⚠️
src/token.rs	78.2%	5 Missing ⚠️
src/index.rs	66.6%	1 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
src/index.rs	92.2% <66.6%> (-4.4%)	⬇️
src/token.rs	97.6% <78.2%> (-2.4%)	⬇️
src/assign.rs	97.6% <86.3%> (-2.1%)	⬇️
src/diagnostic.rs	74.4% <74.4%> (ø)
src/pointer.rs	95.7% <82.2%> (-2.6%)	⬇️
src/resolve.rs	93.6% <81.7%> (-6.4%)	⬇️

[asmello]

// @chanced

I'm finally giving this all another look. I'm pretty happy with how it all turned out, overall.

My main nit is that there's a bit of an inconsistency now between how assign::Error and RichParseError work. The former produces a sourceless error by default, and requires the Diagnose trait to be made into a Report<_> error, while the latter is Report<_> by default. Apologies if I missed an explanation for why they ended up diverging, but I kinda like the assign::Error approach, so maybe we can switch PointerBuf::parse to this paradigm? I think I have a way to achieve that - I wouldn't push it just for the optimisation aspect, but I do want us to remain consistent. Will propose a PR.

Other follow-ups:

• I don't think the serde flag should be enabled by default (I agree miette shouldn't either)
• diagnostic::Label::new should probably be crate-private
• We should document that validate_bytes must be called with a non-empty slice
• struct NoLeadingSlash is not used anywhere, should be removed
• docs for delete.rs seem out-of-date, they mention Ok(None) as the return value in case of failure (should be just None, right?)
• I think the ResolveError name should be deprecated since AssignError was (or did I miss some context?)
• index::ParseIndexError and token::EncodingError should probably be migrated to the new error system too, as they have a source string to work with.
• Maybe Diagnose should be renamed WithSource and its methods renamed with_source and with_source_from? It's a bit confusing that you can only call .diagnose() on a thing that implements Diagnostic (naively one might expect .diagnose() to produce a Diagnostic implementor instead).
• Relatedly, I think maybe Diagnose should be sealed? Not sure it makes sense for users to implement it for their types.

Phew, I think that covers it. Though I need to revisit the PR I have open on json_patch to remind myself if there's anything blocking that (other than us having a stable API on this end).

EDIT: some of this I've changed my mind about, see #104 and #105

[asmello]

I think we also need a tweak in diagnose_with, as the way it's written it will eagerly call f() 😅