Add Code actions on save by junglerobba · Pull Request #14481 · helix-editor/helix

junglerobba · 2025-09-22T17:54:40Z

For now this is just #6486 redone on top of latest master. Because there was no more discussion on that PR I thought I’d resubmit a working recent version and see if there’s anything left to do.
The only thing changed is making it work akin to autoformat so that code actions don’t trigger on auto save.

allow textDocument/didChange notification to be synchronous Co-authored-by: junglerobba <junglerobba@jngl.one> Add code actions on save * Add code-actions-on-save config * Match VS Code config to allow future flexibility * Refactor lsp commands to allow for code reuse * Attempt code actions for all language servers for the document * Add lsp specific integration tests * Update documentation in book * Canonicalize path argument when retrieving documents by path * Resolves issue when running lsp integration tests in windows commands: add no-code-actions flag to write and write-all Same as with auto-format, auto save will not trigger code actions

allow textDocument/didChange notification to be synchronous Add code actions on save * Add code-actions-on-save config * Match VS Code config to allow future flexibility * Refactor lsp commands to allow for code reuse * Attempt code actions for all language servers for the document * Add lsp specific integration tests * Update documentation in book * Canonicalize path argument when retrieving documents by path * Resolves issue when running lsp integration tests in windows commands: add no-code-actions flag to write and write-all Same as with auto-format, auto save will not trigger code actions Co-authored-by: Jonatan Pettersson <jonatan.pettersson@proton.me>

the-mikedavis · 2025-11-11T15:00:31Z

I'm not a fan of integration tests using an external language server, I'd prefer to see that part dropped

allow textDocument/didChange notification to be synchronous Add code actions on save * Add code-actions-on-save config * Match VS Code config to allow future flexibility * Refactor lsp commands to allow for code reuse * Attempt code actions for all language servers for the document * Add lsp specific integration tests * Update documentation in book * Canonicalize path argument when retrieving documents by path * Resolves issue when running lsp integration tests in windows commands: add no-code-actions flag to write and write-all Same as with auto-format, auto save will not trigger code actions Co-authored-by: Jonatan Pettersson <jonatan.pettersson@proton.me>

Allow textDocument/didChange notification to be synchronous Add code actions on save * Add code-actions-on-save config * Match VS Code config to allow future flexibility * Refactor lsp commands to allow for code reuse * Attempt code actions for all language servers for the document * Add lsp specific integration tests * Update documentation in book * Canonicalize path argument when retrieving documents by path * Resolves issue when running lsp integration tests in windows commands: Add no-code-actions flag to write and write-all Same as with auto-format, auto save will not trigger code actions Co-authored-by: Jonatan Pettersson <jonatan.pettersson@proton.me>

archseer

Sorry this has been stuck in the queue for a while but it looks good to me apart from a small nit!

archseer · 2026-01-21T06:38:54Z

book/src/languages.md

 | `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.
 | `rainbow-brackets` | Overrides the `editor.rainbow-brackets` config key for the language |
+| `code-actions-on-save`    | List of LSP code actions to be run in order on save, for example `[{ code-action = "source.organizeImports", enabled = true }]` |


How about just making this an array? ["source.organizeImports", ...]

archseer · 2026-01-21T06:40:08Z

helix-term/src/commands/typed.rs

    ..Flag::DEFAULT
 };

+const WRITE_NO_CODE_ACTIONS_FLAG: Flag = Flag {


nit: We could just use the --no-format flag and treat code actions as part of formatting? I don't mind a separate flag though.

makes sense, I cannot really think of a usecase where you'd want to skip one but not the other, but that was probably what I had in mind for this

archseer

Looks good to me now, I'll test drive this for a couple of days :)

junglerobba · 2026-01-22T07:38:44Z

Since the largest part of this is still just the original PR rebased, I hope I split the attributions to the original author correctly between commits :)

the-mikedavis · 2026-01-25T14:40:48Z

helix-term/src/commands/lsp.rs

+        if let Some(future) = language_server.resolve_code_action(code_action) {
+            if let Ok(code_action) = helix_lsp::block_on(future) {


Whenever possible we should avoid blocking the main thread on an LSP request like this. Resolving completion items is a good example of something that does this currently which ideally should not. By blocking the main thread, a slow request can make the editor pause or it can lead to a deadlock since the main thread also handles requests from the server. Instead this should spawn, await the result of the future, and then use a job to handle a successful result in the main thread.

Yes, currently in the process of reworking this to be non-blocking. The difficulty with this is that it moves back between needing to be async (for querying LSP actions and resolving the action), but in between these needing access to Editor (to get access to current document version, language server handle and compute the new document range).
To keep this non-blocking, would it be reasonable to introduce a way for a callback to return another Job to be queued?

format_selections has a good example of how this is usually done

helix/helix-term/src/commands.rs

Lines 5074 to 5094 in b7fbae5

tokio::spawn(async move {

match future.await {

Ok(Some(res)) => {

let transaction =

helix_lsp::util::generate_transaction_from_edits(&text, res, offset_encoding);

job::dispatch(move |editor, _compositor| {

let Some(doc) = editor.document_mut(doc_id) else {

return;

};

// Updating a desynced document causes problems with applying the transaction

if doc.version() != doc_version {

return;

}

doc.apply(&transaction, view_id);

})

.await

}

Err(err) => log::error!("format sections failed: {err}"),

Ok(None) => (),

}

});

You can spawn a tokio task to await the future off of the main thread and then use job::dispatch to modify the editor with the result on the main thread.

Started on it before this comment, so for now I have a non-blocking version by introducing a new type of callback that can optionally return another job to be queued.
Main reason for doing it that way is that I wanted to make sure that the order of code actions is as configured, each runs after each other and operates on the latest document state, and formatting will only run after all code actions (or immediately if no code actions are configured)
If that's not wanted I can take a look at how this could be converted to what you suggested. But this way I could also keep wait_before_exiting as a possibility on these jobs (currently only on formatting)
Converted to draft for now, have to use this for a couple of days myself to see how it works out

the-mikedavis · 2026-01-25T14:42:38Z

helix-lsp/src/client.rs

        }
    }

+    pub fn notify_sync<R: lsp::notification::Notification>(&self, params: R::Params) -> Result<()>


Notifications can't really be made synchronous since we don't have any idea when/if the server receives the notification and handles it. It looks like instead this is just ensuring that any errors to send are returned to the caller rather than logging / dropping the errors. Why is that necessary?

Dropped this part 👍

This introduces `Callback::Followup` that can optionally return another job to be queued after the callback finishes. This allows creation of async operation chains where the editor state needs to be queried in between async operations, without needing blocking operations. Needed for features like running multiple LSP code actions in order, where each action must operate on the latest document state after the previous action completes.

Adds a new `code-actions-on-save` config option for LSP code actions to run on save, before auto formatting will be applied. These actions will be spawned as a series of alternating async jobs and callbacks, using the callback followups from the previous commit, which starts the next async task after each callback executes. This is needed to ensure these actions each operate on the latest document version and will run in the configured order, so access to non-thread-safe Editor is required for querying document state in between async tasks. Because this is run automatically on save, and is user-configurable, the reasonable default is using each action from the first LSP that advertises it and, in case it resolves to multiple actions, use the first one. This way no user interaction is required, and potentially conflicting applications of actions are avoided. Co-authored-by: Jonatan Pettersson <jonatan.pettersson@proton.me>

Enables the `--no-format` flag for the write and write-all commands to also disable configured on save code actions.

junglerobba force-pushed the code-actions-on-save branch 2 times, most recently from ef43f0d to 340154a Compare September 29, 2025 10:01

junglerobba force-pushed the code-actions-on-save branch from 340154a to 2fa960c Compare September 30, 2025 09:10

junglerobba force-pushed the code-actions-on-save branch from 2fa960c to 25ee721 Compare October 23, 2025 20:46

junglerobba force-pushed the code-actions-on-save branch from 25ee721 to fed3f51 Compare November 12, 2025 09:39

archseer requested changes Jan 21, 2026

View reviewed changes

archseer requested a review from the-mikedavis January 21, 2026 06:42

junglerobba force-pushed the code-actions-on-save branch from fed3f51 to f835a1a Compare January 21, 2026 08:55

archseer previously approved these changes Jan 21, 2026

View reviewed changes

the-mikedavis reviewed Jan 25, 2026

View reviewed changes

junglerobba dismissed archseer’s stale review via 0727ba4 February 1, 2026 23:19

junglerobba force-pushed the code-actions-on-save branch from f835a1a to 0727ba4 Compare February 1, 2026 23:19

junglerobba marked this pull request as draft February 1, 2026 23:20

junglerobba force-pushed the code-actions-on-save branch 2 times, most recently from 5fddc08 to d35dea6 Compare February 3, 2026 00:24

junglerobba marked this pull request as ready for review February 3, 2026 00:38

junglerobba force-pushed the code-actions-on-save branch from d35dea6 to 80e5131 Compare February 5, 2026 17:16

junglerobba and others added 2 commits February 12, 2026 21:51

commands: skip code actions on auto save

cd339a7

Enables the `--no-format` flag for the write and write-all commands to also disable configured on save code actions.

junglerobba force-pushed the code-actions-on-save branch from 80e5131 to cd339a7 Compare February 12, 2026 20:58

		if let Some(future) = language_server.resolve_code_action(code_action) {
		if let Ok(code_action) = helix_lsp::block_on(future) {

	tokio::spawn(async move {
	match future.await {
	Ok(Some(res)) => {
	let transaction =
	helix_lsp::util::generate_transaction_from_edits(&text, res, offset_encoding);
	job::dispatch(move \|editor, _compositor\| {
	let Some(doc) = editor.document_mut(doc_id) else {
	return;
	};
	// Updating a desynced document causes problems with applying the transaction
	if doc.version() != doc_version {
	return;
	}
	doc.apply(&transaction, view_id);
	})
	.await
	}
	Err(err) => log::error!("format sections failed: {err}"),
	Ok(None) => (),
	}
	});

Uh oh!

Conversation

junglerobba commented Sep 22, 2025

Uh oh!

the-mikedavis commented Nov 11, 2025

Uh oh!

archseer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

archseer left a comment

Choose a reason for hiding this comment

Uh oh!

junglerobba commented Jan 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants