Skip to content
This repository has been archived by the owner on Nov 5, 2024. It is now read-only.

Add related information to typst diagnostics #317

Merged

Conversation

NickAcPT
Copy link
Contributor

@NickAcPT NickAcPT commented Oct 2, 2023

Adds related information to the diagnostics based on the trace points provided by typst itself.

This is my first time interacting with this codebase so if you spot anything that could be improved, please do let me know!

Before the change:
Before

After the change:
After

@nvarner
Copy link
Owner

nvarner commented Oct 10, 2023

This looks great, thank you! Pending the GH Actions checks, I'd tentatively like to merge.

I tend to write Rust in a functional style, so the biggest code improvement to match the rest of the code base would be to adopt that kind of approach and use iterators/streams where possible. That said, it's not essential, and gets difficult when you have streams with Result and Option (as you do), so it's not a particularly big deal.

Overall, code looks good, split into reasonable and codebase-idiomatic functions.

@nvarner
Copy link
Owner

nvarner commented Oct 10, 2023

@NickAcPT can you fix the clippy errors? I'd be happy to merge after that

@NickAcPT
Copy link
Contributor Author

@nvarner whooosies! I forgot to run clippy, my bad.
I'll fix it as soon as I can and I'll let you know! 👍

@NickAcPT NickAcPT force-pushed the feature/relatedinformation-tracepoint branch from e621000 to 98a18e2 Compare October 12, 2023 18:48
@NickAcPT
Copy link
Contributor Author

NickAcPT commented Oct 12, 2023

@nvarner Apologies for not fixing it sooner!
I think it should be good to go, please let me know if there's anything else you'd want me to fix. ^-^

@nvarner
Copy link
Owner

nvarner commented Oct 12, 2023

No big deal. Looks good now. Thanks for the improvements!

@nvarner nvarner merged commit 94af0c1 into nvarner:master Oct 12, 2023
5 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants