Show formatting failure (#9229)

This fixes #8072 and #9061 by surfacing formatting errors in the
activity indicator.

It shows a message in the activity indicator if the last attempt
to format a buffer failed.

It only keeps track of the last attempt, so any further formatting
that succeeds will reset or update the error message.

I chose to only keep track of that, because everything else (keeping
track of formatting state per buffer, per project, per worktree) seems
complicated with little benefit, since we'd have to keep track of that
state, update it, clean it, etc.

We can still do that should we decide that we need to keep track
of the state on a per-buffer basis, but I think for now this is a
good, simple solution.

This also changes the `OpenLog` action to scroll to the end of the
buffer
and to not mark the buffer as dirty.


Release Notes:

- Added message to activity indicator if last attempt to format a buffer
failed. Message will get reset when next formatting succeeds. Clicking
on message opens log with more information.
([#8072](https://github.com/zed-industries/zed/issues/8072) and
[#9061](https://github.com/zed-industries/zed/issues/9061)).
- Changed `zed: Open Log` action to not mark the opened log file as
dirty and to always scroll to the bottom of the log.


https://github.com/zed-industries/zed/assets/1185253/951fb9ac-8b8b-483a-a46d-712e52878a4d
This commit is contained in:
Thorsten Ball 2024-03-12 16:30:08 +01:00 committed by GitHub
parent 39a0841ea8
commit 8c87b349dc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 334 additions and 290 deletions

View File

@ -241,6 +241,17 @@ impl ActivityIndicator {
};
}
// Show any formatting failure
if let Some(failure) = self.project.read(cx).last_formatting_failure() {
return Content {
icon: Some(WARNING_ICON),
message: format!("Formatting failed: {}. Click to see logs.", failure),
on_click: Some(Arc::new(|_, cx| {
cx.dispatch_action(Box::new(workspace::OpenLog));
})),
};
}
// Show any application auto-update info.
if let Some(updater) = &self.auto_updater {
return match &updater.read(cx).status() {

View File

@ -135,6 +135,7 @@ pub struct Project {
language_servers: HashMap<LanguageServerId, LanguageServerState>,
language_server_ids: HashMap<(WorktreeId, LanguageServerName), LanguageServerId>,
language_server_statuses: BTreeMap<LanguageServerId, LanguageServerStatus>,
last_formatting_failure: Option<String>,
last_workspace_edits_by_language_server: HashMap<LanguageServerId, ProjectTransaction>,
language_server_watched_paths: HashMap<LanguageServerId, HashMap<WorktreeId, GlobSet>>,
client: Arc<client::Client>,
@ -606,6 +607,7 @@ impl Project {
language_servers: Default::default(),
language_server_ids: HashMap::default(),
language_server_statuses: Default::default(),
last_formatting_failure: None,
last_workspace_edits_by_language_server: Default::default(),
language_server_watched_paths: HashMap::default(),
buffers_being_formatted: Default::default(),
@ -738,6 +740,7 @@ impl Project {
)
})
.collect(),
last_formatting_failure: None,
last_workspace_edits_by_language_server: Default::default(),
language_server_watched_paths: HashMap::default(),
opened_buffers: Default::default(),
@ -3992,6 +3995,10 @@ impl Project {
self.language_server_statuses.values()
}
pub fn last_formatting_failure(&self) -> Option<&str> {
self.last_formatting_failure.as_deref()
}
pub fn update_diagnostics(
&mut self,
language_server_id: LanguageServerId,
@ -4297,7 +4304,7 @@ impl Project {
cx: &mut ModelContext<Project>,
) -> Task<anyhow::Result<ProjectTransaction>> {
if self.is_local() {
let mut buffers_with_paths = buffers
let buffers_with_paths = buffers
.into_iter()
.filter_map(|buffer_handle| {
let buffer = buffer_handle.read(cx);
@ -4308,284 +4315,23 @@ impl Project {
.collect::<Vec<_>>();
cx.spawn(move |project, mut cx| async move {
// Do not allow multiple concurrent formatting requests for the
// same buffer.
project.update(&mut cx, |this, cx| {
buffers_with_paths.retain(|(buffer, _)| {
this.buffers_being_formatted
.insert(buffer.read(cx).remote_id())
});
let result = Self::format_locally(
project.clone(),
buffers_with_paths,
push_to_history,
trigger,
cx.clone(),
)
.await;
project.update(&mut cx, |project, _| match &result {
Ok(_) => project.last_formatting_failure = None,
Err(error) => {
project.last_formatting_failure.replace(error.to_string());
}
})?;
let _cleanup = defer({
let this = project.clone();
let mut cx = cx.clone();
let buffers = &buffers_with_paths;
move || {
this.update(&mut cx, |this, cx| {
for (buffer, _) in buffers {
this.buffers_being_formatted
.remove(&buffer.read(cx).remote_id());
}
})
.ok();
}
});
let mut project_transaction = ProjectTransaction::default();
for (buffer, buffer_abs_path) in &buffers_with_paths {
let adapters_and_servers: Vec<_> = project.update(&mut cx, |project, cx| {
project
.language_servers_for_buffer(&buffer.read(cx), cx)
.map(|(adapter, lsp)| (adapter.clone(), lsp.clone()))
.collect()
})?;
let settings = buffer.update(&mut cx, |buffer, cx| {
language_settings(buffer.language(), buffer.file(), cx).clone()
})?;
let remove_trailing_whitespace = settings.remove_trailing_whitespace_on_save;
let ensure_final_newline = settings.ensure_final_newline_on_save;
let tab_size = settings.tab_size;
// First, format buffer's whitespace according to the settings.
let trailing_whitespace_diff = if remove_trailing_whitespace {
Some(
buffer
.update(&mut cx, |b, cx| b.remove_trailing_whitespace(cx))?
.await,
)
} else {
None
};
let whitespace_transaction_id = buffer.update(&mut cx, |buffer, cx| {
buffer.finalize_last_transaction();
buffer.start_transaction();
if let Some(diff) = trailing_whitespace_diff {
buffer.apply_diff(diff, cx);
}
if ensure_final_newline {
buffer.ensure_final_newline(cx);
}
buffer.end_transaction(cx)
})?;
for (lsp_adapter, language_server) in adapters_and_servers.iter() {
// Apply the code actions on
let code_actions: Vec<lsp::CodeActionKind> = settings
.code_actions_on_format
.iter()
.flat_map(|(kind, enabled)| {
if *enabled {
Some(kind.clone().into())
} else {
None
}
})
.collect();
#[allow(clippy::nonminimal_bool)]
if !code_actions.is_empty()
&& !(trigger == FormatTrigger::Save
&& settings.format_on_save == FormatOnSave::Off)
{
let actions = project
.update(&mut cx, |this, cx| {
this.request_lsp(
buffer.clone(),
LanguageServerToQuery::Other(language_server.server_id()),
GetCodeActions {
range: text::Anchor::MIN..text::Anchor::MAX,
kinds: Some(code_actions),
},
cx,
)
})?
.await?;
for mut action in actions {
Self::try_resolve_code_action(&language_server, &mut action)
.await
.context("resolving a formatting code action")?;
if let Some(edit) = action.lsp_action.edit {
if edit.changes.is_none() && edit.document_changes.is_none() {
continue;
}
let new = Self::deserialize_workspace_edit(
project
.upgrade()
.ok_or_else(|| anyhow!("project dropped"))?,
edit,
push_to_history,
lsp_adapter.clone(),
language_server.clone(),
&mut cx,
)
.await?;
project_transaction.0.extend(new.0);
}
if let Some(command) = action.lsp_action.command {
project.update(&mut cx, |this, _| {
this.last_workspace_edits_by_language_server
.remove(&language_server.server_id());
})?;
language_server
.request::<lsp::request::ExecuteCommand>(
lsp::ExecuteCommandParams {
command: command.command,
arguments: command.arguments.unwrap_or_default(),
..Default::default()
},
)
.await?;
project.update(&mut cx, |this, _| {
project_transaction.0.extend(
this.last_workspace_edits_by_language_server
.remove(&language_server.server_id())
.unwrap_or_default()
.0,
)
})?;
}
}
}
}
// Apply language-specific formatting using either the primary language server
// or external command.
let primary_language_server = adapters_and_servers
.first()
.cloned()
.map(|(_, lsp)| lsp.clone());
let server_and_buffer = primary_language_server
.as_ref()
.zip(buffer_abs_path.as_ref());
let mut format_operation = None;
match (&settings.formatter, &settings.format_on_save) {
(_, FormatOnSave::Off) if trigger == FormatTrigger::Save => {}
(Formatter::LanguageServer, FormatOnSave::On | FormatOnSave::Off)
| (_, FormatOnSave::LanguageServer) => {
if let Some((language_server, buffer_abs_path)) = server_and_buffer {
format_operation = Some(FormatOperation::Lsp(
Self::format_via_lsp(
&project,
buffer,
buffer_abs_path,
language_server,
tab_size,
&mut cx,
)
.await
.context("failed to format via language server")?,
));
}
}
(
Formatter::External { command, arguments },
FormatOnSave::On | FormatOnSave::Off,
)
| (_, FormatOnSave::External { command, arguments }) => {
if let Some(buffer_abs_path) = buffer_abs_path {
format_operation = Self::format_via_external_command(
buffer,
buffer_abs_path,
command,
arguments,
&mut cx,
)
.await
.context(format!(
"failed to format via external command {:?}",
command
))?
.map(FormatOperation::External);
}
}
(Formatter::Auto, FormatOnSave::On | FormatOnSave::Off) => {
if let Some(new_operation) =
prettier_support::format_with_prettier(&project, buffer, &mut cx)
.await
{
format_operation = Some(new_operation);
} else if let Some((language_server, buffer_abs_path)) =
server_and_buffer
{
format_operation = Some(FormatOperation::Lsp(
Self::format_via_lsp(
&project,
buffer,
buffer_abs_path,
language_server,
tab_size,
&mut cx,
)
.await
.context("failed to format via language server")?,
));
}
}
(Formatter::Prettier, FormatOnSave::On | FormatOnSave::Off) => {
if let Some(new_operation) =
prettier_support::format_with_prettier(&project, buffer, &mut cx)
.await
{
format_operation = Some(new_operation);
}
}
};
buffer.update(&mut cx, |b, cx| {
// If the buffer had its whitespace formatted and was edited while the language-specific
// formatting was being computed, avoid applying the language-specific formatting, because
// it can't be grouped with the whitespace formatting in the undo history.
if let Some(transaction_id) = whitespace_transaction_id {
if b.peek_undo_stack()
.map_or(true, |e| e.transaction_id() != transaction_id)
{
format_operation.take();
}
}
// Apply any language-specific formatting, and group the two formatting operations
// in the buffer's undo history.
if let Some(operation) = format_operation {
match operation {
FormatOperation::Lsp(edits) => {
b.edit(edits, None, cx);
}
FormatOperation::External(diff) => {
b.apply_diff(diff, cx);
}
FormatOperation::Prettier(diff) => {
b.apply_diff(diff, cx);
}
}
if let Some(transaction_id) = whitespace_transaction_id {
b.group_until_transaction(transaction_id);
} else if let Some(transaction) = project_transaction.0.get(buffer) {
b.group_until_transaction(transaction.id)
}
}
if let Some(transaction) = b.finalize_last_transaction().cloned() {
if !push_to_history {
b.forget_transaction(transaction.id);
}
project_transaction.0.insert(buffer.clone(), transaction);
}
})?;
}
Ok(project_transaction)
result
})
} else {
let remote_id = self.remote_id();
@ -4618,6 +4364,289 @@ impl Project {
}
}
async fn format_locally(
project: WeakModel<Project>,
mut buffers_with_paths: Vec<(Model<Buffer>, Option<PathBuf>)>,
push_to_history: bool,
trigger: FormatTrigger,
mut cx: AsyncAppContext,
) -> anyhow::Result<ProjectTransaction> {
// Do not allow multiple concurrent formatting requests for the
// same buffer.
project.update(&mut cx, |this, cx| {
buffers_with_paths.retain(|(buffer, _)| {
this.buffers_being_formatted
.insert(buffer.read(cx).remote_id())
});
})?;
let _cleanup = defer({
let this = project.clone();
let mut cx = cx.clone();
let buffers = &buffers_with_paths;
move || {
this.update(&mut cx, |this, cx| {
for (buffer, _) in buffers {
this.buffers_being_formatted
.remove(&buffer.read(cx).remote_id());
}
})
.ok();
}
});
let mut project_transaction = ProjectTransaction::default();
for (buffer, buffer_abs_path) in &buffers_with_paths {
let adapters_and_servers: Vec<_> = project.update(&mut cx, |project, cx| {
project
.language_servers_for_buffer(&buffer.read(cx), cx)
.map(|(adapter, lsp)| (adapter.clone(), lsp.clone()))
.collect()
})?;
let settings = buffer.update(&mut cx, |buffer, cx| {
language_settings(buffer.language(), buffer.file(), cx).clone()
})?;
let remove_trailing_whitespace = settings.remove_trailing_whitespace_on_save;
let ensure_final_newline = settings.ensure_final_newline_on_save;
let tab_size = settings.tab_size;
// First, format buffer's whitespace according to the settings.
let trailing_whitespace_diff = if remove_trailing_whitespace {
Some(
buffer
.update(&mut cx, |b, cx| b.remove_trailing_whitespace(cx))?
.await,
)
} else {
None
};
let whitespace_transaction_id = buffer.update(&mut cx, |buffer, cx| {
buffer.finalize_last_transaction();
buffer.start_transaction();
if let Some(diff) = trailing_whitespace_diff {
buffer.apply_diff(diff, cx);
}
if ensure_final_newline {
buffer.ensure_final_newline(cx);
}
buffer.end_transaction(cx)
})?;
for (lsp_adapter, language_server) in adapters_and_servers.iter() {
// Apply the code actions on
let code_actions: Vec<lsp::CodeActionKind> = settings
.code_actions_on_format
.iter()
.flat_map(|(kind, enabled)| {
if *enabled {
Some(kind.clone().into())
} else {
None
}
})
.collect();
#[allow(clippy::nonminimal_bool)]
if !code_actions.is_empty()
&& !(trigger == FormatTrigger::Save
&& settings.format_on_save == FormatOnSave::Off)
{
let actions = project
.update(&mut cx, |this, cx| {
this.request_lsp(
buffer.clone(),
LanguageServerToQuery::Other(language_server.server_id()),
GetCodeActions {
range: text::Anchor::MIN..text::Anchor::MAX,
kinds: Some(code_actions),
},
cx,
)
})?
.await?;
for mut action in actions {
Self::try_resolve_code_action(&language_server, &mut action)
.await
.context("resolving a formatting code action")?;
if let Some(edit) = action.lsp_action.edit {
if edit.changes.is_none() && edit.document_changes.is_none() {
continue;
}
let new = Self::deserialize_workspace_edit(
project
.upgrade()
.ok_or_else(|| anyhow!("project dropped"))?,
edit,
push_to_history,
lsp_adapter.clone(),
language_server.clone(),
&mut cx,
)
.await?;
project_transaction.0.extend(new.0);
}
if let Some(command) = action.lsp_action.command {
project.update(&mut cx, |this, _| {
this.last_workspace_edits_by_language_server
.remove(&language_server.server_id());
})?;
language_server
.request::<lsp::request::ExecuteCommand>(
lsp::ExecuteCommandParams {
command: command.command,
arguments: command.arguments.unwrap_or_default(),
..Default::default()
},
)
.await?;
project.update(&mut cx, |this, _| {
project_transaction.0.extend(
this.last_workspace_edits_by_language_server
.remove(&language_server.server_id())
.unwrap_or_default()
.0,
)
})?;
}
}
}
}
// Apply language-specific formatting using either the primary language server
// or external command.
let primary_language_server = adapters_and_servers
.first()
.cloned()
.map(|(_, lsp)| lsp.clone());
let server_and_buffer = primary_language_server
.as_ref()
.zip(buffer_abs_path.as_ref());
let mut format_operation = None;
match (&settings.formatter, &settings.format_on_save) {
(_, FormatOnSave::Off) if trigger == FormatTrigger::Save => {}
(Formatter::LanguageServer, FormatOnSave::On | FormatOnSave::Off)
| (_, FormatOnSave::LanguageServer) => {
if let Some((language_server, buffer_abs_path)) = server_and_buffer {
format_operation = Some(FormatOperation::Lsp(
Self::format_via_lsp(
&project,
buffer,
buffer_abs_path,
language_server,
tab_size,
&mut cx,
)
.await
.context("failed to format via language server")?,
));
}
}
(
Formatter::External { command, arguments },
FormatOnSave::On | FormatOnSave::Off,
)
| (_, FormatOnSave::External { command, arguments }) => {
if let Some(buffer_abs_path) = buffer_abs_path {
format_operation = Self::format_via_external_command(
buffer,
buffer_abs_path,
command,
arguments,
&mut cx,
)
.await
.context(format!(
"failed to format via external command {:?}",
command
))?
.map(FormatOperation::External);
}
}
(Formatter::Auto, FormatOnSave::On | FormatOnSave::Off) => {
if let Some(new_operation) =
prettier_support::format_with_prettier(&project, buffer, &mut cx).await
{
format_operation = Some(new_operation);
} else if let Some((language_server, buffer_abs_path)) = server_and_buffer {
format_operation = Some(FormatOperation::Lsp(
Self::format_via_lsp(
&project,
buffer,
buffer_abs_path,
language_server,
tab_size,
&mut cx,
)
.await
.context("failed to format via language server")?,
));
}
}
(Formatter::Prettier, FormatOnSave::On | FormatOnSave::Off) => {
if let Some(new_operation) =
prettier_support::format_with_prettier(&project, buffer, &mut cx).await
{
format_operation = Some(new_operation);
}
}
};
buffer.update(&mut cx, |b, cx| {
// If the buffer had its whitespace formatted and was edited while the language-specific
// formatting was being computed, avoid applying the language-specific formatting, because
// it can't be grouped with the whitespace formatting in the undo history.
if let Some(transaction_id) = whitespace_transaction_id {
if b.peek_undo_stack()
.map_or(true, |e| e.transaction_id() != transaction_id)
{
format_operation.take();
}
}
// Apply any language-specific formatting, and group the two formatting operations
// in the buffer's undo history.
if let Some(operation) = format_operation {
match operation {
FormatOperation::Lsp(edits) => {
b.edit(edits, None, cx);
}
FormatOperation::External(diff) => {
b.apply_diff(diff, cx);
}
FormatOperation::Prettier(diff) => {
b.apply_diff(diff, cx);
}
}
if let Some(transaction_id) = whitespace_transaction_id {
b.group_until_transaction(transaction_id);
} else if let Some(transaction) = project_transaction.0.get(buffer) {
b.group_until_transaction(transaction.id)
}
}
if let Some(transaction) = b.finalize_last_transaction().cloned() {
if !push_to_history {
b.forget_transaction(transaction.id);
}
project_transaction.0.insert(buffer.clone(), transaction);
}
})?;
}
Ok(project_transaction)
}
async fn format_via_lsp(
this: &WeakModel<Self>,
buffer: &Model<Buffer>,

View File

@ -4124,6 +4124,7 @@ pub async fn last_opened_workspace_paths() -> Option<WorkspaceLocation> {
}
actions!(collab, [OpenChannelNotes]);
actions!(zed, [OpenLog]);
async fn join_channel_internal(
channel_id: ChannelId,

View File

@ -7,7 +7,7 @@ use assistant::AssistantPanel;
use breadcrumbs::Breadcrumbs;
use client::ZED_URL_SCHEME;
use collections::VecDeque;
use editor::{Editor, MultiBuffer};
use editor::{scroll::Autoscroll, Editor, MultiBuffer};
use gpui::{
actions, point, px, AppContext, AsyncAppContext, Context, FocusableView, PromptLevel,
TitlebarOptions, View, ViewContext, VisualContext, WindowBounds, WindowKind, WindowOptions,
@ -41,7 +41,7 @@ use vim::VimModeSetting;
use welcome::BaseKeymap;
use workspace::{
create_and_open_local_file, notifications::simple_message_notification::MessageNotification,
open_new, AppState, NewFile, NewWindow, Toast, Workspace, WorkspaceSettings,
open_new, AppState, NewFile, NewWindow, OpenLog, Toast, Workspace, WorkspaceSettings,
};
use workspace::{notifications::DetachAndPromptErr, Pane};
use zed_actions::{OpenBrowser, OpenSettings, OpenZedUrl, Quit};
@ -62,7 +62,6 @@ actions!(
OpenLicenses,
OpenLocalSettings,
OpenLocalTasks,
OpenLog,
OpenTasks,
OpenTelemetryLog,
ResetBufferFontSize,
@ -561,21 +560,25 @@ fn open_log_file(workspace: &mut Workspace, cx: &mut ViewContext<Workspace>) {
};
let project = workspace.project().clone();
let buffer = project
.update(cx, |project, cx| project.create_buffer("", None, cx))
.update(cx, |project, cx| project.create_buffer(&log, None, cx))
.expect("creating buffers on a local workspace always succeeds");
buffer.update(cx, |buffer, cx| buffer.edit([(0..0, log)], None, cx));
let buffer = cx.new_model(|cx| {
MultiBuffer::singleton(buffer, cx).with_title("Log".into())
});
workspace.add_item_to_active_pane(
Box::new(
cx.new_view(|cx| {
Editor::for_multibuffer(buffer, Some(project), cx)
}),
),
cx,
);
let editor =
cx.new_view(|cx| Editor::for_multibuffer(buffer, Some(project), cx));
editor.update(cx, |editor, cx| {
let last_multi_buffer_offset = editor.buffer().read(cx).len(cx);
editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
s.select_ranges(Some(
last_multi_buffer_offset..last_multi_buffer_offset,
));
})
});
workspace.add_item_to_active_pane(Box::new(editor), cx);
})
.log_err();
})