Skip to content
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
54 changes: 53 additions & 1 deletion helix-lsp/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,59 @@ impl Client {
})
}

// will_save / will_save_wait_until
pub fn text_document_will_save(
&self,
text_document: lsp::TextDocumentIdentifier,
reason: lsp::TextDocumentSaveReason,
) -> Option<()> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To match other notification-style functions in the client let's not return anything here,

Suggested change
) -> Option<()> {
) {

Similarly see text_document_did_open

let capabilities = self.capabilities.get().unwrap();

match &capabilities.text_document_sync.as_ref()? {
lsp::TextDocumentSyncCapability::Options(lsp::TextDocumentSyncOptions {
will_save: enabled,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can match explicitly on will_save: Some(true) here for the same effect as the if below.

..
}) => {
if !*enabled.as_ref()? {
return None;
}
}
_ => return None,
};

self.notify::<lsp::notification::WillSaveTextDocument>(lsp::WillSaveTextDocumentParams {
text_document,
reason,
});
Some(())
}

pub fn text_document_will_save_wait_until(
&self,
text_document: lsp::TextDocumentIdentifier,
reason: lsp::TextDocumentSaveReason,
) -> Option<impl Future<Output = Result<Option<Vec<lsp::TextEdit>>>>> {
let capabilities = self.capabilities.get().unwrap();

match &capabilities.text_document_sync.as_ref()? {
lsp::TextDocumentSyncCapability::Options(lsp::TextDocumentSyncOptions {
will_save_wait_until: enabled,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here about pattern matching

..
}) => {
if !*enabled.as_ref()? {
return None;
}
}
_ => return None,
};

Some(self.call_with_timeout::<lsp::request::WillSaveWaitUntil>(
&lsp::WillSaveTextDocumentParams {
text_document,
reason,
},
5,
))
}

pub fn text_document_did_save(
&self,
Expand Down
4 changes: 3 additions & 1 deletion helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub(crate) mod typed;
pub use dap::*;
use futures_util::FutureExt;
use helix_event::status;
use helix_lsp::lsp::TextDocumentSaveReason;
use helix_stdx::{
path::{self, find_paths},
rope::{self, RopeSliceExt},
Expand Down Expand Up @@ -3590,6 +3591,7 @@ async fn make_format_callback(
view_id: ViewId,
format: impl Future<Output = Result<Transaction, FormatterError>> + Send + 'static,
write: Option<(Option<PathBuf>, bool)>,
reason: TextDocumentSaveReason,
) -> anyhow::Result<job::Callback> {
let format = format.await;

Expand Down Expand Up @@ -3624,7 +3626,7 @@ async fn make_format_callback(

if let Some((path, force)) = write {
let id = doc.id();
if let Err(err) = editor.save(id, path, force) {
if let Err(err) = editor.save(id, path, force, reason) {
editor.set_error(format!("Error saving: {}", err));
}
}
Expand Down
30 changes: 27 additions & 3 deletions helix-term/src/commands/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use helix_core::command_line::{Args, Flag, Signature, Token, TokenKind};
use helix_core::fuzzy::fuzzy_match;
use helix_core::indent::MAX_INDENT;
use helix_core::line_ending;
use helix_lsp::lsp::TextDocumentSaveReason;
use helix_stdx::path::home_dir;
use helix_view::document::{read_to_string, DEFAULT_LANGUAGE_NAME};
use helix_view::editor::{CloseError, ConfigEvent};
Expand Down Expand Up @@ -368,6 +369,7 @@ fn write_impl(
view.id,
fmt,
Some((path.map(Into::into), options.force)),
options.reason,
);

jobs.add(Job::with_callback(callback).wait_before_exiting());
Expand All @@ -378,7 +380,7 @@ fn write_impl(

if fmt.is_none() {
let id = doc.id();
cx.editor.save(id, path, options.force)?;
cx.editor.save(id, path, options.force, options.reason)?;
}

Ok(())
Expand Down Expand Up @@ -447,6 +449,7 @@ fn insert_final_newline(doc: &mut Document, view_id: ViewId) {
pub struct WriteOptions {
pub force: bool,
pub auto_format: bool,
pub reason: TextDocumentSaveReason,
}

fn write(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> anyhow::Result<()> {
Expand All @@ -460,6 +463,7 @@ fn write(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> anyhow
WriteOptions {
force: false,
auto_format: !args.has_flag(WRITE_NO_FORMAT_FLAG.name),
reason: TextDocumentSaveReason::MANUAL,
},
)
}
Expand All @@ -475,6 +479,7 @@ fn force_write(cx: &mut compositor::Context, args: Args, event: PromptEvent) ->
WriteOptions {
force: true,
auto_format: !args.has_flag(WRITE_NO_FORMAT_FLAG.name),
reason: TextDocumentSaveReason::MANUAL,
},
)
}
Expand All @@ -494,6 +499,7 @@ fn write_buffer_close(
WriteOptions {
force: false,
auto_format: !args.has_flag(WRITE_NO_FORMAT_FLAG.name),
reason: TextDocumentSaveReason::MANUAL,
},
)?;

Expand All @@ -516,6 +522,7 @@ fn force_write_buffer_close(
WriteOptions {
force: true,
auto_format: !args.has_flag(WRITE_NO_FORMAT_FLAG.name),
reason: TextDocumentSaveReason::MANUAL,
},
)?;

Expand All @@ -542,7 +549,14 @@ fn format(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> anyh
let format = doc.format(cx.editor).context(
"A formatter isn't available, and no language server provides formatting capabilities",
)?;
let callback = make_format_callback(doc.id(), doc.version(), view.id, format, None);
let callback = make_format_callback(
doc.id(),
doc.version(),
view.id,
format,
None,
TextDocumentSaveReason::MANUAL,
);
cx.jobs.callback(callback);

Ok(())
Expand Down Expand Up @@ -704,6 +718,7 @@ fn write_quit(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> a
WriteOptions {
force: false,
auto_format: !args.has_flag(WRITE_NO_FORMAT_FLAG.name),
reason: TextDocumentSaveReason::MANUAL,
},
)?;
cx.block_try_flush_writes()?;
Expand All @@ -725,6 +740,7 @@ fn force_write_quit(
WriteOptions {
force: true,
auto_format: !args.has_flag(WRITE_NO_FORMAT_FLAG.name),
reason: TextDocumentSaveReason::MANUAL,
},
)?;
cx.block_try_flush_writes()?;
Expand Down Expand Up @@ -769,6 +785,7 @@ pub struct WriteAllOptions {
pub force: bool,
pub write_scratch: bool,
pub auto_format: bool,
pub reason: TextDocumentSaveReason,
}

pub fn write_all_impl(
Expand Down Expand Up @@ -829,6 +846,7 @@ pub fn write_all_impl(
target_view,
fmt,
Some((None, options.force)),
options.reason,
);
jobs.add(Job::with_callback(callback).wait_before_exiting());
})
Expand All @@ -837,7 +855,8 @@ pub fn write_all_impl(
};

if fmt.is_none() {
cx.editor.save::<PathBuf>(doc_id, None, options.force)?;
cx.editor
.save::<PathBuf>(doc_id, None, options.force, options.reason)?;
}
}

Expand All @@ -859,6 +878,7 @@ fn write_all(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> an
force: false,
write_scratch: true,
auto_format: !args.has_flag(WRITE_NO_FORMAT_FLAG.name),
reason: TextDocumentSaveReason::MANUAL,
},
)
}
Expand All @@ -878,6 +898,7 @@ fn force_write_all(
force: true,
write_scratch: true,
auto_format: !args.has_flag(WRITE_NO_FORMAT_FLAG.name),
reason: TextDocumentSaveReason::MANUAL,
},
)
}
Expand All @@ -896,6 +917,7 @@ fn write_all_quit(
force: false,
write_scratch: true,
auto_format: !args.has_flag(WRITE_NO_FORMAT_FLAG.name),
reason: TextDocumentSaveReason::MANUAL,
},
)?;
quit_all_impl(cx, false)
Expand All @@ -915,6 +937,7 @@ fn force_write_all_quit(
force: true,
write_scratch: true,
auto_format: !args.has_flag(WRITE_NO_FORMAT_FLAG.name),
reason: TextDocumentSaveReason::MANUAL,
},
);
quit_all_impl(cx, true)
Expand Down Expand Up @@ -1478,6 +1501,7 @@ fn update(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> anyho
WriteOptions {
force: false,
auto_format: !args.has_flag(WRITE_NO_FORMAT_FLAG.name),
reason: TextDocumentSaveReason::MANUAL,
},
)
} else {
Expand Down
2 changes: 2 additions & 0 deletions helix-term/src/handlers/auto_save.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use anyhow::Ok;
use arc_swap::access::Access;

use helix_event::{register_hook, send_blocking};
use helix_lsp::lsp::TextDocumentSaveReason;
use helix_view::{
document::Mode,
events::DocumentDidChange,
Expand Down Expand Up @@ -91,6 +92,7 @@ fn request_auto_save(editor: &mut Editor) {
force: false,
write_scratch: false,
auto_format: false,
reason: TextDocumentSaveReason::AFTER_DELAY,
};

if let Err(e) = commands::typed::write_all_impl(context, options) {
Expand Down
2 changes: 2 additions & 0 deletions helix-term/src/ui/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use helix_core::{
unicode::width::UnicodeWidthStr,
visual_offset_from_block, Change, Position, Range, Selection, Transaction,
};
use helix_lsp::lsp::TextDocumentSaveReason;
use helix_view::{
annotations::diagnostics::DiagnosticFilter,
document::{Mode, SCRATCH_BUFFER_NAME},
Expand Down Expand Up @@ -1511,6 +1512,7 @@ impl Component for EditorView {
force: false,
write_scratch: false,
auto_format: false,
reason: TextDocumentSaveReason::FOCUS_OUT,
};
if let Err(e) = commands::typed::write_all_impl(context, options) {
context.editor.set_error(format!("{}", e));
Expand Down
44 changes: 43 additions & 1 deletion helix-view/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use helix_vcs::DiffProviderRegistry;

use futures_util::stream::select_all::SelectAll;
use futures_util::{future, StreamExt};
use helix_lsp::{Call, LanguageServerId};
use helix_lsp::{lsp::TextDocumentSaveReason, Call, LanguageServerId};
use tokio_stream::wrappers::UnboundedReceiverStream;

use std::{
Expand Down Expand Up @@ -1948,12 +1948,54 @@ impl Editor {
doc_id: DocumentId,
path: Option<P>,
force: bool,
reason: TextDocumentSaveReason,
) -> anyhow::Result<()> {
// convert a channel of futures to pipe into main queue one by one
// via stream.then() ? then push into main future

let path = path.map(|path| path.into());
let view_id = self.get_synced_view_id(doc_id);
let doc = doc_mut!(self, &doc_id);
let view = view_mut!(self, view_id);
let url = match &path {
Some(path) => url::Url::from_file_path(path).ok(),
None => doc.url(),
};
if let Some(url) = url {
let identifier = lsp::TextDocumentIdentifier::new(url.clone());
let language_servers: Vec<_> = self
.language_servers
.iter_clients()
Comment on lines +1966 to +1968
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will send the will-saves to all language servers even if the document doesn't belong to a language server. Instead this can iterate over doc.language_servers.values()

.filter(|client| client.is_initialized())
.cloned()
.collect();
for language_server in language_servers {
language_server.text_document_will_save(identifier.clone(), reason);

let Some(request) =
language_server.text_document_will_save_wait_until(identifier.clone(), reason)
else {
continue;
};
let edits = match helix_lsp::block_on(request) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will block the main thread and it could take a while if a language server sends edits when saving many documents with :write-all. Instead these will-save requests should be done asynchronously like formatting requests

Ok(Some(edits)) => edits,
Ok(None) => continue,
Err(err) => {
log::error!("invalid willSaveWaitUntil response: {err:?}");
continue;
}
};
let transaction = helix_lsp::util::generate_transaction_from_edits(
doc.text(),
edits,
language_server.offset_encoding(),
);

doc.apply(&transaction, view.id);
doc.append_changes_to_history(view);
}
}

let doc_save_future = doc.save(path, force)?;

// When a file is written to, notify the file event handler.
Expand Down
Loading