move locus calculation to separate function
This moves the locus calculation from RichDiagnostic::render to Diagnostic::locuses. Some questions to answer:
-
[ ] Is the implementation impact acceptable? The way I implemented this has some impacts on the existing API though: Because this way uses a
BTreeMapkeyed with theFileId, theFileIdmust now implementOrdinstead of justPartialEq.This was the simplest and least overhead accumulating way to enable deduplication by file. Alternatively we could derive another arbirary ordering of the
FileIds, e.g. by indices from a deduplicated Vec.I personally think this change is justified because
- many library users make use of
SimpleFiles, which is compatible and - any sensible candidates for
FileIdthat I can think of and implementPartialEqalso implementOrd.
- many library users make use of
-
[ ] Should this new function be added to the public API? I did not yet implement it this way, but it was suggested in #333. This would go towards better enabling "alternative" renderers.
Please note the CI is failing because of dependencies not being compatible with the oldest tested against Rust version (1.40.0).
I'm wondering if there is as way to do this without needing to allocate an intermediate BTreeMap for this? 🤔 That's the one thing that would give me pause I guess.
We might also be able to solve this better if we had some sort of intermediate 'resolved diagnostic' - something I'm hoping we can get to at some stage!
The reason for using a BTreeMap was this comment https://github.com/brendanzab/codespan/blob/629d35a4c9f30236a2adbc5773c756d23d1775d7/codespan-reporting/src/term/views.rs#L98-L100
I would not call it an "intermediate BTreeMap" because its used afterwards, like the vector the above comment is referencing.
I don't see a way without allocating some data structure similar to this. We could do with a Vec but that does not come with the deduplication built in, and no advantages that I am aware of.