Prior integration patches went about it in the most simple and direct
way, which is unfortunately completely different than how it _should_
look. Some bad prior art:
* Checking was done during each render only on the current viewport
* Dictionary loading was hard-coded and done during `Editor::new`
* The UX for suggestions was not hooked into code actions
* Same for "Add {word} to dictionary"
Ultimately this is still very unbaked. Big parts still to do:
* Run a tree-sitter query to discover parts of the document that need to
be checked. Look at the queries used in Codebook - I believe we want
to follow that strategy at least partially. It uses different captures
to control the strategy used to parse the captured content. (For
example capturing strings)
* Support multiple dictionaries at once. Not totally sure what this
looks like yet, other than `dictionaries.iter().any(..)`.
* Figure out how many configuration levers we need. Again, Codebook is
likely to be good inspiration here.
This introduces another `Range` type which is an enum: either the
(character) indices in a rope or a wrapper around `lsp::Range` - the
positions in the document according to the position encoding.
Internally we always use character (typically) or byte indices into the
text to represent positions into the text. LSP however uses row and
column counts where the column meaning depends on the position encoding
negotiated with the server. This makes it difficult to have concepts
like `Diagnostic` be generic between an internal feature (for example
spell checking errors) and LSP. The solution here is direct: use an enum
that represents either format of describing a range of positions.
This change introduces that `Range` and uses it for two purposes:
* `Diagnostic` has been rewritten and moved from helix-core to
helix-view. The diagnostic type in `helix_view::document` is now a
wrapper around `helix_view::Diagnostic`, tracking the actual ranges
into the document.
* The `Location` type in `commands::lsp` has been refactored to use this
range instead of directly using `lsp::Range`.
The point of this is to support emitting features like diagnostics and
symbols using internal features like a spell checker and tree-sitter
(respectively). Now the spell checking integration can attach
diagnostics itself, roughly like so:
let provider = DiagnosticProvider::Spelling;
let diagnostics = /* find spelling mistakes */
.map(|(word, range)| {
helix_view::Diagnostic {
message: format!("Possible spelling mistake '{word}'"),
severity: Some(Severity::Hint),
range: helix_view::Range::Document(range),
provider: provider.clone(),
..Default::default()
}
})
.collect();
editor.handle_diagnostics(
provider,
uri,
Some(doc_version),
diagnostics,
);
In addition we can use this to build tree-sitter based symbol pickers
(also see <https://redirect.github.com/helix-editor/helix/pull/12275>).
This change moves the LSP code actions handling code into helix-view
and introduces a generic action type. This encapsulates all of the LSP
code action details in the new `action` module and paves the way for
future internal actions. (Note that we could alternatively encapsulate
the LSP details in `handlers/lsp.rs`.)
For example, the spell checking integration will be able to write
actions for a spelling mistake like so:
let mut actions = Vec::new()
for suggestion in suggestions {
actions.push(Action::new(
format!("Replace '{word}' with '{suggestion}'"),
PRIORITY,
// This closure's environment would capture a `doc_id`,
// `view_id`, and the `suggestion`.
move |editor| {
// Create and apply a Transaction to replace `word`'s
// range in the source document with the suggestion
// string. Clear any diagnostics for that spelling
// mistake.
todo!()
}
))
}
let word = word.to_string();
actions.push(Action::new(
format!("Add '{word}' to the dictionary"),
PRIORITY,
// This closure's environment would capture the word. If
// multiple languages/dictionaries are supported then it would
// also capture the language.
move |editor| {
editor.dictionary.add(&word);
// Re-check any documents using this dictionary.
todo!()
}
))
Since all LSP details are now in `actions`, the `code_action` command
has been moved to the general `commands` module - the function is now
only in charge of UI interactions.
This change also makes a patch to the LSP code actions code: all actions
are now combined into a Vec and sorted together. Beforehand, each
language server's code actions were sorted separately and then collected
into one list based on the order in which the language server was
defined.
Even though there is a check for `is_like_msvc`, when setting `CXX` to
`clang++` this will miss that check and try to use `-fPIC`, which is an
invlaid flag for the target.
Previously the `call` helper (and its related functions) returned a
`serde_json::Value` which was then decoded either later in the client
(see signature help and hover) or by the client's caller. This led to
some unnecessary boilerplate in the client:
let resp = self.call::<MyRequest>(params);
Some(async move { Ok(serde_json::from_value(resp.await?)?) })
and in the caller. It also allowed for mistakes with the types. The
workspace symbol request's calling code for example mistakenly decoded a
`lsp::WorkspaceSymbolResponse` as `Vec<lsp::SymbolInformation>` - one of
the untagged enum members (so it parsed successfully) but not the
correct type.
With this change, the `call` helper eagerly decodes the response to a
request as the `lsp::request::Request::Result` trait item. This is
similar to the old helper `request` (which has become redundant and has
been eliminated) but all work is done within the same async block which
avoids some awkward lifetimes. The return types of functions like
`Client::text_document_range_inlay_hints` are now more verbose but it is
no longer possible to accidentally decode as an incorrect type.
Additionally `Client::resolve_code_action` now uses the `call_with_ref`
helper to avoid an unnecessary clone.
This adds events for:
* a document being opened
* a document being closed
* a language server sending the initialized notification
* a language server exiting
and also moves some handling done for these scenarios into hooks,
generally moving more into helix-view. A hook is also added on
`DocumentDidChange` which sends the `text_document_did_change`
notification - this resolves a TODO in `document`.
This resolves a TODO in the core diagnostic module to refactor this
type. It was originally an alias of `LanguageServerId` for simplicity.
Refactoring as an enum is a necessary step towards introducing
"internal" diagnostics - diagnostics emitted by core features such as
a spell checker. Fully supporting this use-case will require further
larger changes to the diagnostic type, but the change to the provider
can be made first.
Note that `Copy` is not derived for `DiagnosticProvider` (as it was
previously because `LanguageServerId` is `Copy`). In the child commits
we will add the `identifier` used in LSP pull diagnostics which is a
string - not `Copy`.
This is the same change as 1c9a5bd366 but for:
* document symbols
* workspace symbols
* goto definition/declaration/.../references
* hover
Instead of bailing when one server fails, we log an error and continue
gathering items from the other responses.
When requesting code actions from multiple LSP servers,
rather than bailing as soon as an error is encountered,
instead log the error and then keep going so that successful
requests can be presented to the user.
This is purely for ergonomics: we should be able to pass strings for
example
crate::runtime_file(format!("namespace/{foo}/{bar}.txt"))
(Note that this works on Windows, see the `Path` documentation.)
This is a minor move that will make future refactors of code actions
simpler. We should be able to move nearly all code action functionality
into `helix-view`, save UI stuff like the `menu::Item` implementation
and dealings with the compositor.
Note that this injection doesn't work currently because precedence is
not handled by the current syntax highlighter. The switch to tree-house
will properly handle the precedence of this pattern.
if `config.toml` either does not have `editor.true-color` or sets
it to false, many (most?) themes stop being usable. when loading such a
theme, Helix falls back to the default theme, but didn't mention this
anywhere - even in `~/.cache/helix/helix.log` when run with `-v`.
if this occurs when reloading a theme at runtime with `:theme`, there's
a fairly helpful error about
> `theme requires true color support`
seems worth logging about this if it happens during startup too.
