add QSyntaxHighlighter to Span inspector #1368
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This highlights all output tokens that share the same span as the text cursor is hovering over.
Expansion erros are now shown directly in the output textfield.
- Fix incorrect calculation of target span - Fix false highlighting caused by Pretty Please code manipulation
- Prevents issues with special characters like '<' and '>' breaking the output HTML
These tokens are now ignored during parsing because prettyplease inserts them automatically in certain situations.
…fresh bug - Make output text area scrollable - Adjust text color to match the theme - Fix edge case where text change did not trigger refresh if cursor position stayed the same
- Before tests were failing due to an outdated Cargo.lock
- Tests were failing due to an outdated Cargo.lock
added 13 commits
November 12, 2025 18:25
- renamed finalFormats to PendingHighlights - improve comment for PendingHighlights - added PendingHighlights to field of SyntaxHighlighter
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1368 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 75 75
Lines 13108 13108
=========================================
Hits 13108 13108 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
added 2 commits
December 10, 2025 14:59
Collaborator
|
Hm, I managed to get the span inspector to panic: |
LeonMatthesKDAB
requested changes
Dec 22, 2025
Collaborator
LeonMatthesKDAB
left a comment
LeonMatthesKDAB
left a comment
There was a problem hiding this comment.
Overall, this looks surprisingly clean 🥳
I had expected a custom QSyntaxHighlighter to be much more annoying to implement.
The background coloring for spans is also really easy to parse visually.
Good job, thank you 👍
Just some smaller nitpicks remaining.
Comment on lines
+203
to
+222
| impl cxx_qt::Constructor<(*mut QTextDocument,)> for qobject::SyntaxHighlighter { | ||
| type BaseArguments = (*mut QTextDocument,); | ||
| type InitializeArguments = (); | ||
| type NewArguments = (); | ||
|
|
||
| fn route_arguments( | ||
| args: (*mut QTextDocument,), | ||
| ) -> ( | ||
| Self::NewArguments, | ||
| Self::BaseArguments, | ||
| Self::InitializeArguments, | ||
| ) { | ||
| ((), args, ()) | ||
| } | ||
|
|
||
| fn new(_: ()) -> SyntaxHighlighterRust { | ||
| SyntaxHighlighterRust::default() | ||
| } | ||
| } | ||
|
|
Collaborator
There was a problem hiding this comment.
Yay, all the work to implement the Constructor stuff is paying off 🥳
| ); | ||
| let expand_result = Self::expand(&text.to_string()); | ||
|
|
||
| let (formated_rust, char_flags) = match expand_result.clone() { |
Collaborator
There was a problem hiding this comment.
Suggested change
| let (formated_rust, char_flags) = match expand_result.clone() { | |
| let (formatted_rust, char_flags) = match expand_result.clone() { |
nit-pick: typo
| .map_err(|err| eprintln!("Parsing error: {err}")) | ||
| .unwrap(); | ||
|
|
||
| let formated_rust = prettyplease::unparse(&file); |
Collaborator
There was a problem hiding this comment.
Suggested change
| let formated_rust = prettyplease::unparse(&file); | |
| let formatted_rust = prettyplease::unparse(&file); |
typo
Comment on lines
300
to
303
| self.as_mut() | ||
| .rust_mut() | ||
| .output_highlighter | ||
| .pin_mut() |
Collaborator
There was a problem hiding this comment.
This is duplicated boilerplate from the statement above.
Please extract this into an output_highlighter local variable.
Comment on lines
+111
to
+114
| unsafe extern "C++Qt" { | ||
| include!(<QSyntaxHighlighter>); | ||
| #[qobject] | ||
| type QSyntaxHighlighter; |
Collaborator
There was a problem hiding this comment.
Please split the CXX-Qt bridge into two, where all of this goes into the syntax_highlighter.rs .
That would clean this module up quite a bit.
Contributor
Author
There was a problem hiding this comment.
It does not seem possible for me to split this up, because both parts need to use the same types, but they are defined in different namespaces.
For example:
error[E0308]: mismatched types
--> examples/span-inspector/src/inspector.rs:161:43
|
161 | make_q_syntax_highlighter(output.text_document());
| ------------------------- ^^^^^^^^^^^^^^^^^^^^^^ expected `syntax_highlighter_ffi::QTextDocument`, found `qobject::QTextDocument`
| |
| arguments to this function are incorrect
|
= note: `qobject::QTextDocument` and `syntax_highlighter_ffi::QTextDocument` have similar names, but are actually distinct types
| (None, State::Literal) | ||
| } | ||
|
|
||
| (State::Default, "#[") => { |
Collaborator
There was a problem hiding this comment.
#![ can also mark the start of a macro (which then applies to module scope)
examples/span-inspector/build.rs
Outdated
| .qt_module("Network") | ||
| .qt_module("Quick") | ||
| .file("src/inspector.rs") | ||
| .qobject_header("include/helper.h") |
Collaborator
There was a problem hiding this comment.
This will need to use ccp_file now that 0.8 is out :) (needs rebasing/merging of main)
examples/span-inspector/build.rs
Outdated
| .file("src/inspector.rs") | ||
| .qobject_header("include/helper.h") | ||
| .build(); | ||
| println!("cargo:rerun-if-changed=include/helper.h"); |
Collaborator
There was a problem hiding this comment.
This is not needed, cpp_file will do this automatically :)
| proc-macro2.workspace = true | ||
| prettyplease = { version = "0.2", features = ["verbatim"] } | ||
| syn.workspace = true | ||
| fancy-regex = "0.16.2" |
Collaborator
There was a problem hiding this comment.
Any reason you used fancy-regex and not regex?
Contributor
Author
There was a problem hiding this comment.
It seems that the regex crate does not support lookahead or lookbehind.
| for i in 0..block_length { | ||
| let color = match flags[(block_position + i) as usize] { | ||
| TokenFlag::Original => QColor::from_rgba(0, 100, 155, 170), | ||
| TokenFlag::Generated => QColor::from_rgba(0, 255, 0, 15), |
Collaborator
There was a problem hiding this comment.
I think this green has a little bit too much blue in it, which visually makes it appeary related to the "Original" coloring.
I guess you can also try adding an equal amount of red to balance it and just make it brighter.
But overall, I really like the approach of using the background color to indicate this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a major rewrite of the Span inspector: