Zero length span

[Nahor]

Ideally, this PR would just be commit d63b2bb and c7cbb07[^1] stacked on top of PR #354, but Github doesn't support PR stacking, so this also contains the commits from #354.

The first is to add a "error line only" mode, where one does not want extra context line but still get the line with the error, which is necessary when using zero-length span without context. The second adds support for the zero-length span at the end of the source or the context.

This is a API breaking change. It changes the usize to Option<usize> in SourceCode::read_span().

GraphicalReportHandler and NarratableReportHandler supports the new capability but in a non-API breaking way, by adding with_opt_context_lines instead of changing the signature of with_context_lines. The latter's behavior is also unchanged (where 0 means None in the new function rather than Some(0))

[^1]: and commit 8e4849e to fix new miri errors.

[Nahor]

I fixed the Miri error by removing the unused structs since it doesn't look like they have ever been used.

[Nahor]

... and mark the unused structs in the tests as dead_code

[Nahor]

I don't know why the CI got aborted, I'm guessing because it failed on my fork. There it failed because of an update of the backtrace crate (rust-lang/backtrace-rs#597).

[workingjubilee]

Sorry about that, backtrace 0.3.71 is out now! Though uhh Windows backtraces may be... interesting, still, on the current stable. Sorry about that!

[Nahor]

FYI, my branch has passed the checks with backtrace 0.3.71

[zkat]

I'll have to think about this one for a bit: it's a pretty significant API breakage, and I'm trying to limit things that change the API this significantly. The only thing that I know I want to change about miette that would be notably semver-breaking is that I have some plans for #242 that might involve replacing/deprecating Report.

[Nahor]

Not the most significant API change I was thinking about 😜. A bigger one was to change the SpanContents::data to an array of lines, but I haven't dared yet 😓

[zkat]

Yeah there's a certain extent to which, considering how widely used Miette is, it needs to be "mostly done", and we don't really have the ability to make major changes to it. It's ok to make "breaking" changes that only affect the small minority of folks who use certain niche APIs, I think, but the upgrade path for 99% of people should be clean, even across semver-major boundaries. It can otherwise be a lot of work to upgrade.

[Nahor]

Agreed, which is why I hesitated doing the "vector of lines", and why I chose to add with_opt_context_line rather than fix with_context_lines for the Option<usize> change.

And in that regard, I don't think this Option<usize> is a big change (I'm biased of course). For most people, nothing changes. Pure "users" (those who don't impl anything) won't be affected beside a recompile.

The main group of affected people would be the one having their own SourceCode implementation (it's hard to know how big that group is though, so I don't know if it's a minority or not). I think most implementations would be trivial to fix since it's not really a new feature is "just" a matter of renaming 0 to None and extending non-zero case to also cover 0. As an example, see my own fix for source_impl.rs, which is just a couple of is_some and unwrap_or (although it relies on some big prior cleanup I did for other unrelated reasons, see PR #354).

Another group of affected people would be the one implementing their own Report, but that group should be very small, and quite likely, the change should be very trivial too (see my changes to GraphicalReportHandler and NarratableReportHandler).

Maybe to you the change looks bigger to you than it to you is to you because, as mentioned earlier, this PR also includes some unrelated commits (PR #354) due to the lack of stacked PR support in GitHub? See commit d63b2bb for what actually changed to support Option<usize>.