Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add code actions on save #6486

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ rustflags = ["--cfg", "tokio_unstable", "-C", "target-feature=-crt-static"]
[alias]
xtask = "run --package xtask --"
integration-test = "test --features integration --profile integration --workspace --test integration"
integration-test-lsp = "test --features integration-lsp --profile integration --workspace --test integration-lsp -- --test-threads=1"

11 changes: 11 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,23 @@ jobs:
key: ${{ runner.os }}-stable-v${{ env.CACHE_VERSION }}-tree-sitter-grammars-${{ hashFiles('languages.toml') }}
restore-keys: ${{ runner.os }}-stable-v${{ env.CACHE_VERSION }}-tree-sitter-grammars-

- name: Install go lsp integration tests
uses: actions/setup-go@v5
with:
go-version: '^1.22.0'

- name: Install gopls for lsp integration tests
run: go install golang.org/x/tools/gopls@latest

- name: Run cargo test
run: cargo test --workspace

- name: Run cargo integration-test
run: cargo integration-test

- name: Run cargo integration-test-lsp
run: cargo integration-test-lsp

strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
Expand Down
1 change: 1 addition & 0 deletions book/src/languages.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ These configuration keys are available:
| `text-width` | Maximum line length. Used for the `:reflow` command and soft-wrapping if `soft-wrap.wrap-at-text-width` is set, defaults to `editor.text-width` |
| `workspace-lsp-roots` | Directories relative to the workspace root that are treated as LSP roots. Should only be set in `.helix/config.toml`. Overwrites the setting of the same name in `config.toml` if set. |
| `persistent-diagnostic-sources` | An array of LSP diagnostic sources assumed unchanged when the language server resends the same set of diagnostics. Helix can track the position for these diagnostics internally instead. Useful for diagnostics that are recomputed on save.
| `code-actions-on-save` | List of LSP code actions to be run in order on save, for example `[{ code-action = "source.organizeImports", enabled = true }]` |

### File-type detection and the `file-types` key

Expand Down
8 changes: 8 additions & 0 deletions docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ Contributors using MacOS might encounter `Too many open files (os error 24)`
failures while running integration tests. This can be resolved by increasing
the default value (e.g. to `10240` from `256`) by running `ulimit -n 10240`.

### Language Server tests

There are integration tests specific for language server integration that can be
run with `cargo integration-test-lsp` and have additional dependencies.

* [go](https://go.dev)
* [gopls](https://pkg.go.dev/golang.org/x/tools/gopls)

## Minimum Stable Rust Version (MSRV) Policy

Helix follows the MSRV of Firefox.
Expand Down
9 changes: 9 additions & 0 deletions helix-core/src/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ pub struct LanguageConfiguration {
pub block_comment_tokens: Option<Vec<BlockCommentToken>>,
pub text_width: Option<usize>,
pub soft_wrap: Option<SoftWrap>,
#[serde(default)]
pub code_actions_on_save: Option<Vec<CodeActionsOnSave>>, // List of LSP code actions to be run in order upon saving

#[serde(default)]
pub auto_format: bool,
Expand Down Expand Up @@ -490,6 +492,13 @@ pub struct AdvancedCompletion {
pub default: Option<String>,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub struct CodeActionsOnSave {
pub code_action: String,
pub enabled: bool,
}

#[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case", untagged)]
pub enum DebugConfigCompletion {
Expand Down
68 changes: 62 additions & 6 deletions helix-lsp/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::lsp::{
};
use helix_core::{find_workspace, syntax::LanguageServerFeature, ChangeSet, Rope};
use helix_loader::VERSION_AND_GIT_HASH;
use helix_lsp_types::TextDocumentContentChangeEvent;
use helix_stdx::path;
use parking_lot::Mutex;
use serde::Deserialize;
Expand Down Expand Up @@ -482,6 +483,28 @@ impl Client {
}
}

/// Send a RPC notification to the language server synchronously.
pub fn notify_sync<R: lsp::notification::Notification>(&self, params: R::Params) -> Result<()>
where
R::Params: serde::Serialize,
{
let server_tx = self.server_tx.clone();

let params = serde_json::to_value(params)?;

let notification = jsonrpc::Notification {
jsonrpc: Some(jsonrpc::Version::V2),
method: R::METHOD.to_string(),
params: Self::value_into_params(params),
};

server_tx
.send(Payload::Notification(notification))
.map_err(|e| Error::Other(e.into()))?;

Ok(())
}

/// Reply to a language server RPC call.
pub fn reply(
&self,
Expand Down Expand Up @@ -930,6 +953,44 @@ impl Client {
new_text: &Rope,
changes: &ChangeSet,
) -> Option<impl Future<Output = Result<()>>> {
if let Some(changes) = self.text_document_did_change_impl(old_text, new_text, changes) {
return Some(self.notify::<lsp::notification::DidChangeTextDocument>(
lsp::DidChangeTextDocumentParams {
text_document,
content_changes: changes,
},
));
};
None
}

/// Will send textDocument/didChange notificaiton synchronously
pub fn text_document_did_change_sync(
&self,
text_document: lsp::VersionedTextDocumentIdentifier,
old_text: &Rope,
new_text: &Rope,
changes: &ChangeSet,
) -> Option<Result<()>> {
if let Some(changes) = self.text_document_did_change_impl(old_text, new_text, changes) {
return Some(
self.notify_sync::<lsp::notification::DidChangeTextDocument>(
lsp::DidChangeTextDocumentParams {
text_document,
content_changes: changes,
},
),
);
};
None
}

pub fn text_document_did_change_impl(
&self,
old_text: &Rope,
new_text: &Rope,
changes: &ChangeSet,
) -> Option<Vec<TextDocumentContentChangeEvent>> {
let capabilities = self.capabilities.get().unwrap();

// Return early if the server does not support document sync.
Expand Down Expand Up @@ -961,12 +1022,7 @@ impl Client {
kind => unimplemented!("{:?}", kind),
};

Some(self.notify::<lsp::notification::DidChangeTextDocument>(
lsp::DidChangeTextDocumentParams {
text_document,
content_changes: changes,
},
))
Some(changes)
}

pub fn text_document_did_close(
Expand Down
1 change: 1 addition & 0 deletions helix-term/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ homepage.workspace = true
default = ["git"]
unicode-lines = ["helix-core/unicode-lines", "helix-view/unicode-lines"]
integration = ["helix-event/integration_test"]
integration-lsp = ["integration"]
git = ["helix-vcs/git"]

[[bin]]
Expand Down
Loading
Loading