From 02f82958da93c8d27d3b6c5616591aafc4da80bf Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Wed, 23 Jun 2021 13:35:39 -0500 Subject: [PATCH 1/5] Make formatting happen asynchronously. --- helix-lsp/src/lib.rs | 16 ++++++++ helix-term/src/commands.rs | 66 +++++++++++++++++++++++------- helix-view/src/document.rs | 82 ++++++++++++++++++++++++++------------ helix-view/src/editor.rs | 4 ++ 4 files changed, 128 insertions(+), 40 deletions(-) diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index e4ab153c3336..96a45bb9000f 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -182,6 +182,22 @@ pub mod util { }), ) } + + /// The result of asking the language server to format the document. This can be turned into a + /// `Transaction`, but the advantage of not doing that straight away is that this one is + /// `Send` and `Sync`. + #[derive(Clone, Debug)] + pub struct LspFormatting { + pub doc: Rope, + pub edits: Vec, + pub offset_encoding: OffsetEncoding, + } + + impl From for Transaction { + fn from(fmt: LspFormatting) -> Transaction { + generate_transaction_from_edits(&fmt.doc, fmt.edits, fmt.offset_encoding) + } + } } #[derive(Debug, PartialEq, Clone)] diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 688e653a2c9e..0512af03a41b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -35,7 +35,7 @@ use crate::{ ui::{self, Completion, Picker, Popup, Prompt, PromptEvent}, }; -use crate::application::{LspCallbackWrapper, LspCallbacks}; +use crate::application::{LspCallback, LspCallbackWrapper, LspCallbacks}; use futures_util::FutureExt; use std::{fmt, future::Future, path::Display, str::FromStr}; @@ -1106,11 +1106,12 @@ mod cmd { } fn write_impl>( - view: &View, - doc: &mut Document, + cx: &mut compositor::Context, path: Option

, ) -> Result>, anyhow::Error> { use anyhow::anyhow; + let callbacks = &mut cx.callbacks; + let (view, doc) = current!(cx.editor); if let Some(path) = path { if let Err(err) = doc.set_path(path.as_ref()) { @@ -1124,15 +1125,21 @@ mod cmd { .language_config() .map(|config| config.auto_format) .unwrap_or_default(); - if autofmt { - doc.format(view.id); // TODO: merge into save - } - Ok(tokio::spawn(doc.save())) + let fmt = if autofmt { + doc.format().map(|fmt| { + let shared = fmt.shared(); + let callback = make_format_callback(doc.id(), doc.version(), true, shared.clone()); + callbacks.push(callback); + shared + }) + } else { + None + }; + Ok(tokio::spawn(doc.format_and_save(fmt))) } fn write(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { - let (view, doc) = current!(cx.editor); - if let Err(e) = write_impl(view, doc, args.first()) { + if let Err(e) = write_impl(cx, args.first()) { cx.editor.set_error(e.to_string()); }; } @@ -1142,9 +1149,12 @@ mod cmd { } fn format(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { - let (view, doc) = current!(cx.editor); + let (_, doc) = current!(cx.editor); - doc.format(view.id) + if let Some(format) = doc.format() { + let callback = make_format_callback(doc.id(), doc.version(), false, format); + cx.callbacks.push(callback); + } } fn set_indent_style(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { @@ -1249,8 +1259,7 @@ mod cmd { } fn write_quit(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { - let (view, doc) = current!(cx.editor); - match write_impl(view, doc, args.first()) { + match write_impl(cx, args.first()) { Ok(handle) => { if let Err(e) = helix_lsp::block_on(handle) { cx.editor.set_error(e.to_string()); @@ -1266,7 +1275,7 @@ mod cmd { fn force_write_quit(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { let (view, doc) = current!(cx.editor); - match write_impl(view, doc, args.first()) { + match write_impl(cx, args.first()) { Ok(handle) => { if let Err(e) = helix_lsp::block_on(handle) { cx.editor.set_error(e.to_string()); @@ -1863,6 +1872,35 @@ fn append_to_line(cx: &mut Context) { doc.set_selection(view.id, selection); } +// Creates an LspCallback that waits for formatting changes to be computed. When they're done, +// it applies them, but only if the doc hasn't changed. +fn make_format_callback( + doc_id: DocumentId, + doc_version: i32, + set_unmodified: bool, + format: impl Future + Send + 'static, +) -> LspCallback { + Box::pin(async move { + let format = format.await; + let call: LspCallbackWrapper = + Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { + let view_id = view!(editor).id; + if let Some(doc) = editor.document_mut(doc_id) { + if doc.version() == doc_version { + doc.apply(&Transaction::from(format), view_id); + doc.append_changes_to_history(view_id); + if set_unmodified { + doc.reset_modified(); + } + } else { + log::info!("discarded formatting changes because the document changed"); + } + } + }); + Ok(call) + }) +} + enum Open { Below, Above, diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index dcacdb5ef284..80ef54d518a6 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -16,6 +16,7 @@ use helix_core::{ ChangeSet, Diagnostic, LineEnding, Rope, RopeBuilder, Selection, State, Syntax, Transaction, DEFAULT_LINE_ENDING, }; +use helix_lsp::util::LspFormatting; use crate::{DocumentId, Theme, ViewId}; @@ -472,39 +473,55 @@ impl Document { Ok(doc) } - // TODO: remove view_id dependency here - pub fn format(&mut self, view_id: ViewId) { - if let Some(language_server) = self.language_server() { - // TODO: await, no blocking - let transaction = helix_lsp::block_on(language_server.text_document_formatting( - self.identifier(), - lsp::FormattingOptions::default(), - None, - )) - .map(|edits| { - helix_lsp::util::generate_transaction_from_edits( - self.text(), + pub fn format(&self) -> Option + 'static> { + if let Some(language_server) = self.language_server.clone() { + let text = self.text.clone(); + let id = self.identifier(); + let fut = async move { + let edits = language_server + .text_document_formatting(id, lsp::FormattingOptions::default(), None) + .await + .unwrap_or_else(|e| { + log::warn!("LSP formatting failed: {}", e); + Default::default() + }); + LspFormatting { + doc: text, edits, - language_server.offset_encoding(), - ) - }); - - if let Ok(transaction) = transaction { - self.apply(&transaction, view_id); - self.append_changes_to_history(view_id); - } + offset_encoding: language_server.offset_encoding(), + } + }; + Some(fut) + } else { + None } } + pub fn save(&mut self) -> impl Future> { + self.save_impl::>(None) + } + + pub fn format_and_save( + &mut self, + formatting: Option>, + ) -> impl Future> { + self.save_impl(formatting) + } + // TODO: do we need some way of ensuring two save operations on the same doc can't run at once? // or is that handled by the OS/async layer /// The `Document`'s text is encoded according to its encoding and written to the file located /// at its `path()`. - pub fn save(&mut self) -> impl Future> { + /// + /// If `formatting` is present, it supplies some changes that we apply to the text before saving. + fn save_impl>( + &mut self, + formatting: Option, + ) -> impl Future> { // we clone and move text + path into the future so that we asynchronously save the current // state without blocking any further edits. - let text = self.text().clone(); + let mut text = self.text().clone(); let path = self.path.clone().expect("Can't save with no path set!"); // TODO: handle no path let identifier = self.identifier(); @@ -512,10 +529,7 @@ impl Document { let language_server = self.language_server.clone(); - // reset the modified flag - let history = self.history.take(); - self.last_saved_revision = history.current_revision(); - self.history.set(history); + self.reset_modified(); let encoding = self.encoding; @@ -531,6 +545,15 @@ impl Document { } } + if let Some(fmt) = formatting { + let success = Transaction::from(fmt.await).changes().apply(&mut text); + if !success { + // This shouldn't happen, because the transaction changes were generated + // from the same text we're saving. + log::error!("failed to apply format changes before saving"); + } + } + let mut file = File::create(path).await?; to_writer(&mut file, encoding, &text).await?; @@ -877,6 +900,13 @@ impl Document { current_revision != self.last_saved_revision || !self.changes.is_empty() } + pub fn reset_modified(&mut self) { + let history = self.history.take(); + let current_revision = history.current_revision(); + self.history.set(history); + self.last_saved_revision = current_revision; + } + pub fn mode(&self) -> Mode { self.mode } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 0a2dc54de50f..a16cc50f8b7b 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -270,6 +270,10 @@ impl Editor { self.documents.get(id) } + pub fn document_mut(&mut self, id: DocumentId) -> Option<&mut Document> { + self.documents.get_mut(id) + } + pub fn documents(&self) -> impl Iterator { self.documents.iter().map(|(_id, doc)| doc) } From 6c215d298ad63971c807e6577d2a6b1b948c3f49 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Mon, 28 Jun 2021 07:48:38 -0500 Subject: [PATCH 2/5] Add some async job infrastructure. --- helix-term/src/application.rs | 34 ++++-------- helix-term/src/commands.rs | 32 ++++++----- helix-term/src/compositor.rs | 4 +- helix-term/src/job.rs | 100 ++++++++++++++++++++++++++++++++++ helix-term/src/lib.rs | 1 + helix-term/src/ui/editor.rs | 4 +- 6 files changed, 133 insertions(+), 42 deletions(-) create mode 100644 helix-term/src/job.rs diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index aea0d89fe792..c2ae46516ff9 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -6,6 +6,7 @@ use crate::{ args::Args, compositor::Compositor, config::Config, + job::Jobs, keymap::Keymaps, ui::{self, Spinner}, }; @@ -31,13 +32,6 @@ use crossterm::{ use futures_util::{future, stream::FuturesUnordered}; -type BoxFuture = Pin + Send>>; -pub type LspCallback = - BoxFuture, anyhow::Error>>; - -pub type LspCallbacks = FuturesUnordered; -pub type LspCallbackWrapper = Box; - pub struct Application { compositor: Compositor, editor: Editor, @@ -48,7 +42,7 @@ pub struct Application { theme_loader: Arc, syn_loader: Arc, - callbacks: LspCallbacks, + jobs: Jobs, lsp_progress: LspProgressMap, } @@ -120,7 +114,7 @@ impl Application { theme_loader, syn_loader, - callbacks: FuturesUnordered::new(), + jobs: Jobs::new(), lsp_progress: LspProgressMap::new(), }; @@ -130,11 +124,11 @@ impl Application { fn render(&mut self) { let editor = &mut self.editor; let compositor = &mut self.compositor; - let callbacks = &mut self.callbacks; + let jobs = &mut self.jobs; let mut cx = crate::compositor::Context { editor, - callbacks, + jobs, scroll: None, }; @@ -148,6 +142,7 @@ impl Application { loop { if self.editor.should_close() { + self.jobs.finish(); break; } @@ -172,27 +167,18 @@ impl Application { } self.render(); } - Some(callback) = &mut self.callbacks.next() => { - self.handle_language_server_callback(callback) + Some(callback) = self.jobs.next() => { + self.jobs.handle_callback(&mut self.editor, &mut self.compositor, callback); + self.render(); } } } } - pub fn handle_language_server_callback( - &mut self, - callback: Result, - ) { - if let Ok(callback) = callback { - // TODO: handle Err() - callback(&mut self.editor, &mut self.compositor); - self.render(); - } - } pub fn handle_terminal_events(&mut self, event: Option>) { let mut cx = crate::compositor::Context { editor: &mut self.editor, - callbacks: &mut self.callbacks, + jobs: &mut self.jobs, scroll: None, }; // Handle key events diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 0512af03a41b..a03f72226f1f 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -35,8 +35,8 @@ use crate::{ ui::{self, Completion, Picker, Popup, Prompt, PromptEvent}, }; -use crate::application::{LspCallback, LspCallbackWrapper, LspCallbacks}; -use futures_util::FutureExt; +use crate::job::{self, Job, JobFuture, Jobs}; +use futures_util::{FutureExt, TryFutureExt}; use std::{fmt, future::Future, path::Display, str::FromStr}; use std::{ @@ -54,7 +54,7 @@ pub struct Context<'a> { pub callback: Option, pub on_next_key_callback: Option>, - pub callbacks: &'a mut LspCallbacks, + pub jobs: &'a mut Jobs, } impl<'a> Context<'a> { @@ -85,13 +85,13 @@ impl<'a> Context<'a> { let callback = Box::pin(async move { let json = call.await?; let response = serde_json::from_value(json)?; - let call: LspCallbackWrapper = + let call: job::Callback = Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { callback(editor, compositor, response) }); Ok(call) }); - self.callbacks.push(callback); + self.jobs.callback(callback); } /// Returns 1 if no explicit count was provided @@ -1110,7 +1110,7 @@ mod cmd { path: Option

, ) -> Result>, anyhow::Error> { use anyhow::anyhow; - let callbacks = &mut cx.callbacks; + let jobs = &mut cx.jobs; let (view, doc) = current!(cx.editor); if let Some(path) = path { @@ -1129,7 +1129,7 @@ mod cmd { doc.format().map(|fmt| { let shared = fmt.shared(); let callback = make_format_callback(doc.id(), doc.version(), true, shared.clone()); - callbacks.push(callback); + jobs.callback(callback); shared }) } else { @@ -1139,8 +1139,12 @@ mod cmd { } fn write(cx: &mut compositor::Context, args: &[&str], event: PromptEvent) { - if let Err(e) = write_impl(cx, args.first()) { - cx.editor.set_error(e.to_string()); + match write_impl(cx, args.first()) { + Err(e) => cx.editor.set_error(e.to_string()), + Ok(handle) => { + cx.jobs + .add(Job::new(handle.unwrap_or_else(|e| Err(e.into()))).wait_before_exiting()); + } }; } @@ -1153,7 +1157,7 @@ mod cmd { if let Some(format) = doc.format() { let callback = make_format_callback(doc.id(), doc.version(), false, format); - cx.callbacks.push(callback); + cx.jobs.callback(callback); } } @@ -1879,10 +1883,10 @@ fn make_format_callback( doc_version: i32, set_unmodified: bool, format: impl Future + Send + 'static, -) -> LspCallback { - Box::pin(async move { +) -> impl Future> { + async move { let format = format.await; - let call: LspCallbackWrapper = + let call: job::Callback = Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { let view_id = view!(editor).id; if let Some(doc) = editor.document_mut(doc_id) { @@ -1898,7 +1902,7 @@ fn make_format_callback( } }); Ok(call) - }) + } } enum Open { diff --git a/helix-term/src/compositor.rs b/helix-term/src/compositor.rs index c00b95e95be0..ba8c4bc74b62 100644 --- a/helix-term/src/compositor.rs +++ b/helix-term/src/compositor.rs @@ -26,12 +26,12 @@ pub enum EventResult { use helix_view::Editor; -use crate::application::LspCallbacks; +use crate::job::Jobs; pub struct Context<'a> { pub editor: &'a mut Editor, pub scroll: Option, - pub callbacks: &'a mut LspCallbacks, + pub jobs: &'a mut Jobs, } pub trait Component: Any + AnyComponent { diff --git a/helix-term/src/job.rs b/helix-term/src/job.rs new file mode 100644 index 000000000000..fcecfbcec201 --- /dev/null +++ b/helix-term/src/job.rs @@ -0,0 +1,100 @@ +use helix_view::Editor; + +use crate::compositor::Compositor; + +use futures_util::future::{self, BoxFuture, Future, FutureExt}; +use futures_util::stream::{self, FuturesUnordered, Select, StreamExt}; + +pub type Callback = Box; +pub type JobFuture = BoxFuture<'static, anyhow::Result>>; + +pub struct Job { + pub future: BoxFuture<'static, anyhow::Result>>, + /// Do we need to wait for this job to finish before exiting? + pub wait: bool, +} + +#[derive(Default)] +pub struct Jobs { + futures: FuturesUnordered, + /// These are the ones that need to complete before we exit. + wait_futures: FuturesUnordered, +} + +impl Job { + pub fn new> + Send + 'static>(f: F) -> Job { + Job { + future: f.map(|r| r.map(|()| None)).boxed(), + wait: false, + } + } + + pub fn with_callback> + Send + 'static>( + f: F, + ) -> Job { + Job { + future: f.map(|r| r.map(|x| Some(x))).boxed(), + wait: false, + } + } + + pub fn wait_before_exiting(mut self) -> Job { + self.wait = true; + self + } +} + +impl Jobs { + pub fn new() -> Jobs { + Jobs::default() + } + + pub fn spawn> + Send + 'static>(&mut self, f: F) { + self.add(Job::new(f)); + } + + pub fn callback> + Send + 'static>( + &mut self, + f: F, + ) { + self.add(Job::with_callback(f)); + } + + pub fn handle_callback( + &mut self, + editor: &mut Editor, + compositor: &mut Compositor, + call: anyhow::Result>, + ) { + match call { + Ok(None) => {} + Ok(Some(call)) => { + call(editor, compositor); + } + Err(e) => { + editor.set_error(format!("Async job failed: {}", e)); + } + } + } + + pub fn next<'a>( + &'a mut self, + ) -> impl Future>>> + 'a { + future::select(self.futures.next(), self.wait_futures.next()) + .map(|either| either.factor_first().0) + } + + pub fn add(&mut self, j: Job) { + if j.wait { + self.wait_futures.push(j.future); + } else { + self.futures.push(j.future); + } + } + + /// Blocks until all the jobs that need to be waited on are done. + pub fn finish(&mut self) { + let wait_futures = std::mem::take(&mut self.wait_futures); + helix_lsp::block_on(wait_futures.for_each(|_| future::ready(()))); + } +} diff --git a/helix-term/src/lib.rs b/helix-term/src/lib.rs index dc8ec38e6ffc..3f28818849cb 100644 --- a/helix-term/src/lib.rs +++ b/helix-term/src/lib.rs @@ -8,5 +8,6 @@ pub mod args; pub mod commands; pub mod compositor; pub mod config; +pub mod job; pub mod keymap; pub mod ui; diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index d86b0793ceaa..da6d5f6b12b2 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -621,7 +621,7 @@ impl Component for EditorView { count: None, callback: None, on_next_key_callback: None, - callbacks: cx.callbacks, + jobs: cx.jobs, }; if let Some(on_next_key) = self.on_next_key.take() { @@ -639,7 +639,7 @@ impl Component for EditorView { // use a fake context here let mut cx = Context { editor: cxt.editor, - callbacks: cxt.callbacks, + jobs: cxt.jobs, scroll: None, }; let res = completion.handle_event(event, &mut cx); From bdbec8792c5d00672f72cacabd2fe17033646e3b Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Mon, 28 Jun 2021 08:00:44 -0500 Subject: [PATCH 3/5] Satisfy clippy. --- helix-term/src/application.rs | 2 +- helix-term/src/commands.rs | 37 ++++++++++++++++------------------- helix-term/src/job.rs | 8 ++++---- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index c2ae46516ff9..3d20e1b3ef04 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -167,7 +167,7 @@ impl Application { } self.render(); } - Some(callback) = self.jobs.next() => { + Some(callback) = self.jobs.next_job() => { self.jobs.handle_callback(&mut self.editor, &mut self.compositor, callback); self.render(); } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index a03f72226f1f..146846afc116 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1878,31 +1878,28 @@ fn append_to_line(cx: &mut Context) { // Creates an LspCallback that waits for formatting changes to be computed. When they're done, // it applies them, but only if the doc hasn't changed. -fn make_format_callback( +async fn make_format_callback( doc_id: DocumentId, doc_version: i32, set_unmodified: bool, format: impl Future + Send + 'static, -) -> impl Future> { - async move { - let format = format.await; - let call: job::Callback = - Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { - let view_id = view!(editor).id; - if let Some(doc) = editor.document_mut(doc_id) { - if doc.version() == doc_version { - doc.apply(&Transaction::from(format), view_id); - doc.append_changes_to_history(view_id); - if set_unmodified { - doc.reset_modified(); - } - } else { - log::info!("discarded formatting changes because the document changed"); - } +) -> anyhow::Result { + let format = format.await; + let call: job::Callback = Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { + let view_id = view!(editor).id; + if let Some(doc) = editor.document_mut(doc_id) { + if doc.version() == doc_version { + doc.apply(&Transaction::from(format), view_id); + doc.append_changes_to_history(view_id); + if set_unmodified { + doc.reset_modified(); } - }); - Ok(call) - } + } else { + log::info!("discarded formatting changes because the document changed"); + } + } + }); + Ok(call) } enum Open { diff --git a/helix-term/src/job.rs b/helix-term/src/job.rs index fcecfbcec201..4b59c81c58d9 100644 --- a/helix-term/src/job.rs +++ b/helix-term/src/job.rs @@ -33,7 +33,7 @@ impl Job { f: F, ) -> Job { Job { - future: f.map(|r| r.map(|x| Some(x))).boxed(), + future: f.map(|r| r.map(Some)).boxed(), wait: false, } } @@ -77,9 +77,9 @@ impl Jobs { } } - pub fn next<'a>( - &'a mut self, - ) -> impl Future>>> + 'a { + pub fn next_job( + &mut self, + ) -> impl Future>>> + '_ { future::select(self.futures.next(), self.wait_futures.next()) .map(|either| either.factor_first().0) } From caecba4a34dfe8e201ed5f603092d07219106443 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Tue, 29 Jun 2021 22:36:33 -0500 Subject: [PATCH 4/5] Make Document's format API a little nicer. --- helix-term/src/commands.rs | 23 +++++++++-------------- helix-view/src/document.rs | 12 ++++++++++++ 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 146846afc116..1e3ccdfff6c3 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1121,20 +1121,12 @@ mod cmd { if doc.path().is_none() { return Err(anyhow!("cannot write a buffer without a filename")); } - let autofmt = doc - .language_config() - .map(|config| config.auto_format) - .unwrap_or_default(); - let fmt = if autofmt { - doc.format().map(|fmt| { - let shared = fmt.shared(); - let callback = make_format_callback(doc.id(), doc.version(), true, shared.clone()); - jobs.callback(callback); - shared - }) - } else { - None - }; + let fmt = doc.auto_format().map(|fmt| { + let shared = fmt.shared(); + let callback = make_format_callback(doc.id(), doc.version(), true, shared.clone()); + jobs.callback(callback); + shared + }); Ok(tokio::spawn(doc.format_and_save(fmt))) } @@ -1878,6 +1870,9 @@ fn append_to_line(cx: &mut Context) { // Creates an LspCallback that waits for formatting changes to be computed. When they're done, // it applies them, but only if the doc hasn't changed. +// +// TODO: provide some way to cancel this, probably as part of a more general job cancellation +// scheme async fn make_format_callback( doc_id: DocumentId, doc_version: i32, diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 80ef54d518a6..0f1f3a8f70a9 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -473,6 +473,18 @@ impl Document { Ok(doc) } + /// The same as [`format`], but only returns formatting changes if auto-formatting + /// is configured. + pub fn auto_format(&self) -> Option + 'static> { + if self.language_config().map(|c| c.auto_format) == Some(true) { + self.format() + } else { + None + } + } + + /// If supported, returns the changes that should be applied to this document in order + /// to format it nicely. pub fn format(&self) -> Option + 'static> { if let Some(language_server) = self.language_server.clone() { let text = self.text.clone(); From 975ca566e5dccfbeb10c5e342e1a95fcf324d697 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Tue, 29 Jun 2021 22:52:09 -0500 Subject: [PATCH 5/5] Make set_unmodified an enum. --- helix-term/src/commands.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 1e3ccdfff6c3..beae3c0cd24a 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1123,7 +1123,12 @@ mod cmd { } let fmt = doc.auto_format().map(|fmt| { let shared = fmt.shared(); - let callback = make_format_callback(doc.id(), doc.version(), true, shared.clone()); + let callback = make_format_callback( + doc.id(), + doc.version(), + Modified::SetUnmodified, + shared.clone(), + ); jobs.callback(callback); shared }); @@ -1148,7 +1153,8 @@ mod cmd { let (_, doc) = current!(cx.editor); if let Some(format) = doc.format() { - let callback = make_format_callback(doc.id(), doc.version(), false, format); + let callback = + make_format_callback(doc.id(), doc.version(), Modified::LeaveModified, format); cx.jobs.callback(callback); } } @@ -1868,6 +1874,13 @@ fn append_to_line(cx: &mut Context) { doc.set_selection(view.id, selection); } +/// Sometimes when applying formatting changes we want to mark the buffer as unmodified, for +/// example because we just applied the same changes while saving. +enum Modified { + SetUnmodified, + LeaveModified, +} + // Creates an LspCallback that waits for formatting changes to be computed. When they're done, // it applies them, but only if the doc hasn't changed. // @@ -1876,7 +1889,7 @@ fn append_to_line(cx: &mut Context) { async fn make_format_callback( doc_id: DocumentId, doc_version: i32, - set_unmodified: bool, + modified: Modified, format: impl Future + Send + 'static, ) -> anyhow::Result { let format = format.await; @@ -1886,7 +1899,7 @@ async fn make_format_callback( if doc.version() == doc_version { doc.apply(&Transaction::from(format), view_id); doc.append_changes_to_history(view_id); - if set_unmodified { + if let Modified::SetUnmodified = modified { doc.reset_modified(); } } else {