feat: add diagnostic marker #12374
feat: add diagnostic marker #12374
Conversation
| @@ -110,7 +110,7 @@ impl Renderer<'_, '_> { | |||
| (end_col, _) = self.renderer.set_string_truncated( | |||
| self.renderer.viewport.x + draw_col, | |||
| row, | |||
| line, | |||
| format!("{} {}", self.config.diagnostic_marker, line).as_str(), | |||
There was a problem hiding this comment.
These should just be drawn separately. Allocating a string for every diagnostic on every frame is too expensive
There was a problem hiding this comment.
That was quick! It makes sense but I am not sure how to do it, would you have any pointers?
There was a problem hiding this comment.
I guess I would need to change set_string_truncated for the TextRenderer and the Surface or is there a different way that I am missing?
There was a problem hiding this comment.
He probably means that you should first draw the marker and then draw the line as before. You are allocating here to just draw in a column, which could just be done as offsetting the x value along as you draw.
There was a problem hiding this comment.
In the end, I changed set_string_truncated. The reason that I do not simply call draw_decoration is because, while it is true that without making it configurable is easy:
// diagnostic marker
const DIAG: &str = "● ";
// trigger before eol diagnostic at <InlineDiagnostics as Decoration>::render_virt_lines
renderer.draw_decoration(DIAG, eol_diagnostic.severity(), virt_off.col as u16);
col_off = renderer.draw_eol_diagnostic(eol_diagnostic, pos.visual_line, virt_off.col);If DIAG is not const, and it is part of the InlineDiagnostics (state.config), we cannot
make it outlive the TextRenderer (I think), so it would not be possible to borrow the diagnostic marker, given this signature:
impl Decoration for InlineDiagnostics<'_> {
fn render_virt_lines(
&mut self,
renderer: &mut TextRenderer,
pos: LinePos,
virt_off: Position,
) -> Position {}
}It seems like I am a bit rusty, so I might be over complicating this.
|
Thinking about this, perhaps this should not be configurable, in the spirit of other PRs/comments related to symbols. Am I right? What should the default/fixed marker be? |
|
Yeah I feel that this could be combined with (or based on) #12060. For example I have this patch locally as the-mikedavis@5cf96a3 |
|
Makes sense, I'll close this one and leave the implementation for that PR (or reopen it once that one is merged). |
Related to #11864
Add a configurable marker (● by default) to separate between the line and the end of line diagnostic decoration.
The default is the same as the gutter symbol, maybe it should be changed, but I have seen another PR about changing the gutter, so let me know what you think!