Add virtual text support

[sudormrfbin]

Closes #411, called text annotations in code.

TODO

• End of line annotations
• Overwrite with virtual text

Future Works

• Inline annotations

POC using diagnostics (click to expand patch):

diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs
index b04eef0..9018af7 100644
--- a/helix-term/src/application.rs
+++ b/helix-term/src/application.rs
@@ -422,8 +422,37 @@ impl Application {
                                         // source
                                     })
                                 })
-                                .collect();
-
+                                .collect::<Vec<_>>();
+
+                            doc.remove_text_annotations(|vt| vt.scope == "diagnostics");
+
+                            use helix_core::diagnostic::Severity;
+                            use helix_view::decorations::{TextAnnotation, TextAnnotationKind};
+                            use helix_view::graphics::{Color, Style};
+                            let style = Style::default().bg(Color::Gray);
+                            doc.extend_text_annotations(
+                                diagnostics
+                                    .iter()
+                                    .map(|d| TextAnnotation {
+                                        scope: "diagnostics".into(),
+                                        text: d.message.clone(),
+                                        style: d
+                                            .severity
+                                            .as_ref()
+                                            .map(|s| match s {
+                                                Severity::Error => style.clone().fg(Color::Red),
+                                                Severity::Warning => {
+                                                    style.clone().fg(Color::Yellow)
+                                                }
+                                                Severity::Info => style.clone().fg(Color::Blue),
+                                                Severity::Hint => style.clone().fg(Color::Green),
+                                            })
+                                            .unwrap_or(style.clone().fg(Color::Yellow)),
+                                        line: d.line,
+                                        kind: TextAnnotationKind::Eol,
+                                    })
+                                    .collect(),
+                            );
                             doc.set_diagnostics(diagnostics);
                         }
                     }

[cessen]

I think for an initial implementation, just end-of-line annotations are fine. As I mentioned in the linked issue, any rendering work you do here is likely to be redundant with planned future work. So keeping it to the simple case probably makes the most sense.

I'm also a little concerned that doing inline rendering right now will be a bit delicate, as the current rendering architecture isn't really set up to support it, nor is the cursor navigation system, etc. Inline annotations will need to touch a lot of systems to work properly.

[sudormrfbin]

In that case I'll drop the inline rendering plan and keep it as is. Cursor and selection rendering for inline annotations will definitely look hacky with the current architecture.

[pickfire]

I'm also a little concerned that doing inline rendering right now will be a bit delicate, as the current rendering architecture isn't really set up to support it, nor is the cursor navigation system, etc. Inline annotations will need to touch a lot of systems to work properly.

Yeah, kak-lsp did the same and it is quite broken in kakoune too.

[sudormrfbin]

As discussed on Matrix, this PR has been revived (yay !), and I have rebased the original changes on top of current master.

[pickfire]

Missing update in https://github.com/helix-editor/helix/blob/master/helix-core/src/position.rs I think, the visual_coords_at_pos should be updated.

[korken89]

This is awesome, thanks for adding support!
Is anyone using this and have made a snippet for enabling rust-analyzer type hints?

[sudormrfbin]

This PR as of now doesn't implement inlayed virtual text, i.e. virtual text in between normal text, which is what we would need to properly add support for rust-analyzer's type hints. Also the type hint API isn't part of the LSP spec yet, so we would have to probably wait on that too.

[korken89]

Ah, I see - that's unfortunate. The type hint API seems to work in vim and vs-code, what is it that they do differently?
Do you know?

[sudormrfbin]

They usually have dedicated plugins that do the setup for you, for example I use https://github.com/nvim-lua/lsp_extensions.nvim for Neovim which sets up inlay hints for rust. VSCode's rust extension also hooks into the decoration API provided by VSC and renders them.

[archseer]

Also the type hint API isn't part of the LSP spec yet, so we would have to probably wait on that too.

The new spec seems final, rust-analyzer recently switched over: https://rust-analyzer.github.io/thisweek/2022/03/14/changelog-120.html#new-features

[korken89]

This seems to now be part of the LSP spec: microsoft/language-server-protocol#956

[kellpossible]

This PR as of now doesn't implement inlayed virtual text, i.e. virtual text in between normal text

@sudormrfbin I quite like the inlay hints from https://github.com/simrat39/rust-tools.nvim#inlay-hints

[2brownc]

Is it possible to make the hints show up below/above the line? If the hint is long it gets cut off when it is displayed beside the line.

[EriKWDev]

+1
Would love for this PR to land, even if behind an experimental config flag :)
(it hasn't already landed experimentally right...?)

[dekuraan]

Does this branch have working type hints?

[erasin]

LSP 3.17 has support for inlay hints https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_inlayHint

[email protected](0.93.1) has support InlayHint

language support:

• rust-analyzer
• golang gopls
• typescript
• c/c++
• java

[poliorcetics]

LSP 3.17 has support for inlay hints https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_inlayHint

That should be done in another PR I think

[dead10ck]

I don't have a very solid understanding of the rendering code, but this looks reasonable from what I can tell.

[dead10ck]

Why was this not necessary before, but is now?

[pickfire]

I believe because it is due to the addition of virtual text.

But this += 1 does not seemed to support double width characters like CJK, otherwise I think it shouldn't always += 1 and sometimes it should += 2.

[pascalkuthe]

I think this works as intended in all cases.

visual_x point is placed after the last character (always incremented by the correct grapheme width in the else clause). The visual_x += 1 only moves the virtual text to the right by one cell so there is space between the annotation and the content of the line. It might be nice to configure the amount of space in the future but I think that can be done in a followup PR.

This was not required before this PR because visual_x is reset back to 0 right after this condition (as a new line starts) on master. However this PR still requires the visual_x (plus the space) to render the EOL annotations.

[dead10ck]

Still needed?

[pascalkuthe]

Apart from two minor nits this looks good to me

[pascalkuthe]

Suggested change

	// It's slightly more efficient to produce a full RopeSlice from the Rope, then slice that a bunch
	// of times than it is to always call Rope::slice/get_slice (it will internally always hit RSEnum::Light).
	let text = text.slice(..);

this is a noop (text is already a RopeSlice

[pascalkuthe]

Might be more performant to implement this as a BTreeMap in tne future with the lines as keys.
This would make iterating a small range of lines (during rendering) more efficient when there are a large amount of annotations.
Can probably be done in a followup PR tough

[evanrichter]

Just as a helix user, I'd really love to see this PR merged! Thanks for your time on this. I had time today to check out your branch and POC for LSP diagnostic virtual text, and it works as expected on my end.

The POC was a little outdated, but here's an updated one you can try (after merging the PR branch to master)

diff --git a/helix-term/src/application.rs b/helix- 
term/src/application.rs
index c7d98fce..4f24b4b9 100644
--- a/helix-term/src/application.rs
+++ b/helix-term/src/application.rs
@@ -633,7 +633,36 @@ pub async fn handle_language_server_message(
                                         source: diagnostic.source.clone()
                                     })
                                 })
-                                .collect();
+                                .collect::<Vec<_>>();
+
+                            doc.clear_text_annotations("diagnostics");
+
+                            use helix_core::diagnostic::Severity;
+                            use helix_view::decorations::{TextAnnotation, TextAnnotationKind};
+                            use helix_view::graphics::{Color, Style};
+                            let style = Style::default().bg(Color::Gray);
+                            doc.push_text_annotations(
+                                "diagnostics",
+                                diagnostics
+                                    .iter()
+                                    .map(|d| TextAnnotation {
+                                        text: d.message.clone().into(),
+                                        style: d
+                                            .severity
+                                            .as_ref()
+                                            .map(|s| match s {
+                                                Severity::Error => style.clone().fg(Color::Red),
+                                                Severity::Warning => {
+                                                    style.clone().fg(Color::Yellow)
+                                                }
+                                                Severity::Info => style.clone().fg(Color::Blue),
+                                                Severity::Hint => style.clone().fg(Color::Green),
+                                            })
+                                            .unwrap_or(style.clone().fg(Color::Yellow)),
+                                        line: d.line,
+                                        kind: TextAnnotationKind::Eol,
+                                    })
+                            );

                             doc.set_diagnostics(diagnostics);
                         }

[evanrichter]

Suggested change

	/// Namespaces and identifes similar annotations
	/// Namespaces to identify similar annotations

[evanrichter]

I think this could be an enum instead of a string, since it will be known at compile time anyway.

pub enum TextAnnotationGroup {
    Diagnostic,
    JumpMode,
}

Then hashing or b-tree look up & filtering will be fast, or possibly even faster would be to store annotations directly into a named field:

struct DocumentAnnotations {
    diagnostics: Vec<TextAnnotation>,
    jump_mode: Vec<TextAnnotation>,
}

pub struct Document {
    // -----8<-----
    text_annotations: DocumentAnnotations,
}

[evanrichter]

ah, another review note: Document implements Debug manually, so consider adding the text_annotations field there as well

https://github.com/helix-editor/helix/pull/417/files#diff-36a64f6c432dc47d11c27851879a0b67459d908edfaa70262aefc21171dc62b9R129

[Omnikar]

I think making a TextAnnotationGroup enum would make it hard to use annotations from a plugin, once a plugin system is implemented.

[evanrichter]

that's a fair point! it's certainly not a blocking issue.

to reap benefits (which may be slight?) of the enum approach and allow plugins to add annotations, something like this would work:

/// first-party annotation groups (only used to differentiate access to the Vecs below)
pub enum TextAnnotationGroup {
    Diagnostic,
    JumpMode,
}

/// plugin annotation groups
/// (might have to reserve "diagnostic" and "jump_mode" strs?
pub type TextAnnotationPluginGroup = &'static str;

struct DocumentAnnotations {
    diagnostics: Vec<TextAnnotation>,
    jump_mode: Vec<TextAnnotation>,
    from_plugins: BTreeMap<&'static str, TextAnnotation>,
}

pub struct Document {
    // -----8<-----
    text_annotations: DocumentAnnotations,
}

[filipdutescu]

This could also be extended to debugging, so it displays the values of variable and objects, which would be of tremendous help.

[abseee]

Update on this? Code folding and inlays are quite useful features currently blocked on this PR.

[pascalkuthe]

An update for everyone interested in this: After discussion in the matrix chat, I will be taking over work on virtual text.
However after discussion with @kirawi I plan to use a different approach than this PR that will build on his work in #3268 and #2184 so that the efforts on soft-wrapping and virtual text do not end up conflicting.
This will involve a generic update to the rendering system that @kirawi and I are planning to develop in cooperation.