Fix diagnostics messages (correct filename and output is cleared when compilation is successful) #56
Conversation
src/main.rs
Outdated
| for (uri, message, range) in messages.into_iter() { | ||
| self.client | ||
| .publish_diagnostics( | ||
| uri, | ||
| vec![Diagnostic { | ||
| range, | ||
| severity: Some(DiagnosticSeverity::ERROR), | ||
| message, | ||
| ..Default::default() | ||
| }) | ||
| .collect(), | ||
| None, | ||
| ) | ||
| .await; | ||
| }], | ||
| None, | ||
| ) | ||
| .await; | ||
| } |
There was a problem hiding this comment.
Can we use something like join_all so we don't have to wait for each promise individually?
| for (uri, message, range) in messages.into_iter() { | ||
| self.client | ||
| .publish_diagnostics( | ||
| uri, |
There was a problem hiding this comment.
Can we merge diagnostics per URI so everything for each file is published at once? Otherwise, we may send diagnostics many times to the same file, which is both slow and will cause the last sent diagnostic to be overwritten each time.
Ideally, this would also handle the above added block to clear diagnostics by sending an empty set to files which we have checked but found no errors.
There was a problem hiding this comment.
Yes I was thinking about it. First I thought that there was a single diagnostic per file but I did not realized that previous diagnostics were just overwritten.
I am fixing it now.
|
Would also close #57 |
|
I pushed a new version where diagnostics are grouped per file, and are sent together with join_all as suggested. However I am unable to find a document where typst reports several errors, to try if it works as expected. Can typst return several error per file? it seems like it stops after the first error (hence always on a single file). |
|
The function prototype certainly suggests it could return multiple errors. Even if that's not a common occurence right now, we have to assume it can be a possibility in the future. LGTM! |
… compilation is successful)
nvarner
force-pushed
the
fix-diagnostics
branch
from
1919442 to
f303e41
Compare
There was a problem hiding this comment.
A few things could be made more idiomatic. In general, it's better to avoid mut and instead build our collections from iterators. However, this looks pretty good, and I need to do some general refactoring anyway. If it turns out to be possible to make it nicer, that can be done then.
|
Thank you! |
fixes #44