Prevent whitespace from rendering inside inlay hints #6312
Conversation
|
An empty file named |
helix-term/src/ui/document.rs
Outdated
| @@ -287,9 +287,11 @@ pub fn render_text<'t>( | |||
| style_span.0 | |||
| }; | |||
|
|
|||
| let virt = grapheme.is_virtual().clone(); | |||
There was a problem hiding this comment.
| let virt = grapheme.is_virtual().clone(); | |
| let virt = grapheme.is_virtual(); |
Doesn't really make a difference but cloning a bool is kind of pointless
There was a problem hiding this comment.
Had this as a relic when I was working with &bool
helix-term/src/ui/document.rs
Outdated
| @@ -405,14 +408,16 @@ impl<'a> TextRenderer<'a> { | |||
| } | |||
|
|
|||
| let width = grapheme.width(); | |||
| let space = if is_virtual { " " } else { &self.space }; | |||
| let nbsp = if is_virtual { " " } else { &self.nbsp }; | |||
| let grapheme = match grapheme { | |||
| Grapheme::Tab { width } => { | |||
| let grapheme_tab_width = char_to_byte_idx(&self.tab, width); | |||
There was a problem hiding this comment.
To properly render tabs you need to add a second field virtual_tab to which gets filled with white spaces in new instead of using whitespace chars.
There was a problem hiding this comment.
Gotcha. I believe its a bit verbose inside the Grapheme::Tab match, but it should now handle the tab rendering
There was a problem hiding this comment.
Looks good now but I would prefer if you could move the if condition outside the match and handle it just like the other variables (so let tab = ..).
I thought it was quite clean how you did it for the other whitespace chars so it would be great if tab was the same
There was a problem hiding this comment.
That makes sense, done.
pascalkuthe
added
E-easy
|
ah actually you need to fix the clippy lint before we can merge, it's just a smalmt Hong tough (chebk the ci run for details or run |
pascalkuthe
added
S-waiting-on-author
| @@ -313,6 +315,7 @@ pub struct TextRenderer<'a> { | |||
| pub nbsp: String, | |||
| pub space: String, | |||
| pub tab: String, | |||
| pub virtual_tab: String, | |||
There was a problem hiding this comment.
Must this be String? Can it be integer instead?
There was a problem hiding this comment.
No we need the allocation to be able to hand out &str to it, just like the tab field. The tab width is not the interesting part here, we already know that from the grapheme.
* fix spaces and nbsps showing in inlay hints * remove origin * virtual tab + fix unneeded clone * update virtual tab determining location * fix clippy lint
* fix spaces and nbsps showing in inlay hints * remove origin * virtual tab + fix unneeded clone * update virtual tab determining location * fix clippy lint
* fix spaces and nbsps showing in inlay hints * remove origin * virtual tab + fix unneeded clone * update virtual tab determining location * fix clippy lint
Addresses most of #6297.
Would like some advice on what should be done about tab rendering. Is it even needed to be addressed because I don't think the inlay hints would draw a tab?