Thanks @pfnsec for working on this feature! I like the general idea and mostly had some small design questions and thoughts on how we might be able to better test the spans.

@pfnsec #567 has now been merged

rustc-LLVM ERROR: IO failure on output stream: No space left on device
error: could not compile `ron` (test "449_tagged_enum")

egads! :P

@pfnsec Perhaps you could add a cargo clean between each step of the CI check job?

I meant further above, lines 77, 80, and 85 where the check, clippy, and test steps are. Could you add a clean step between those?

Ah, my bad. Like this?

I wonder if we could also add similar span tests to the following tests:

• https://github.com/ron-rs/ron/blob/master/tests/256_comma_error.rs
• https://github.com/ron-rs/ron/blob/master/tests/393_serde_errors.rs
• https://github.com/ron-rs/ron/blob/master/tests/203_error_positions.rs

Functionality-wise, I'm happy with the changes and so now we just need to do a bit of cleanup

Hopefully that should fix all the lints. As for the additional span tests you mentioned, we'd have to include unicode-segmentation as a dependency proper as mentioned above. It would be an easy addition if you're fine with the extra dep.

Hopefully that should fix all the lints. As for the additional span tests you mentioned, we'd have to include unicode-segmentation as a dependency proper as mentioned above. It would be an easy addition if you're fine with the extra dep.

Since we'd only use it in the tests, would it still not just be a test dependency? If that doesn't work, maybe we could move the span text extraction code into its own module and add the module to those tests as well (using an explicit module path attribute)? It would be a bit of a hack but should be good enough for the three files.

Unfortunately integration tests only test the public interface of the library, so gating modules through #[cfg(test)] won't work. We could have a non-default feature, test-span-substring, that triggers an optional dependency on unicode-segmentation and gate every relevant integration test through that, but that may be enough plumbing to warrant a separate PR.

Oh, now I follow, you mean doing something like

[#path = "../src/util/span_substr.rs"]
pub mod span_substr;

, right? I'll take a stab at it tomorrow.

Oh, now I follow, you mean doing something like

[#path = "../src/util/span_substr.rs"]
pub mod span_substr;

, right? I'll take a stab at it tomorrow.

Yes, if that doesn’t work I’ve opened #571 and we can do it in a separate PR - if it does we can close the issue immediately :)

Alas, it doesn't work - by importing it that way with an explicit path we end up implementing a fn for the foreign types ron::de::Position and ron::de::Span, along with other crate:: import path mismatches. I've split it off into a separate module to do the optional-feature plumbing mentioned above. I think it may be the cleanest option.

Hopefully this is clean enough for our liking :)
unicode-segmentation is retained as a dev dependency for unit tests. It's also no-std friendly, so it shouldn't interfere with the work there. The existing error checking asserts are retained as well along with the feature-gated span checks.

Hold up, I think I got my wires crossed here with ron::util mod layout. Will fix in the morning...

Alas...

System.IO.IOException: No space left on device : '/home/runner/actions-runner/cached/...'

@pfnsec I'm happy to merge now unless there's anything else from your end?

Nope, nothing else on my end! Thanks :D

Thanks @pfnsec for all of your work on this feature!