Skip to content

workspace: Improve error handling when dropping a file that cannot be opened into the workspace pane#15613

Merged
mrnugget merged 4 commits intozed-industries:mainfrom
ssut:feature/workspace-file-open-error-handling
Aug 20, 2024
Merged

workspace: Improve error handling when dropping a file that cannot be opened into the workspace pane#15613
mrnugget merged 4 commits intozed-industries:mainfrom
ssut:feature/workspace-file-open-error-handling

Conversation

@ssut
Copy link
Copy Markdown
Contributor

@ssut ssut commented Aug 1, 2024

This PR can improve the UX when dropping a file that cannot be opened into the workspace pane. Previously, nothing happened without any messages when such error occurred, which could be awkward for users. Additionally the pane was being split even though the file failed to open.

Here's a screen recording demonstrating the previous/updated behavior:

zed-1.mp4

Changes:

  • It now displays an error message if a file cannot be opened.
  • Updated the logic to first try to open the file. The pane splits only if the file opening process is successful.

Release Notes:

  • Improved error handling when opening files in the workspace pane. An error message will now be displayed if the file cannot be opened.
  • Fixed an issue where unnecessary pane splitting occurred when a file fails to open.

@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Aug 1, 2024
@mrnugget mrnugget self-assigned this Aug 6, 2024
@mrnugget
Copy link
Copy Markdown
Contributor

mrnugget commented Aug 6, 2024

Hey! I'm pretty sure you can simplify your code by using this method:

fn notify_err(self, workspace: &mut Workspace, cx: &mut ViewContext<Workspace>) -> Option<T> {
match self {
Ok(value) => Some(value),
Err(err) => {
log::error!("TODO {err:?}");
workspace.show_error(&err, cx);
None
}
}
}

You can see how it's used here:

let terminal = workspace
.project()
.update(cx, |project, cx| {
project.create_terminal(TerminalKind::Shell(working_directory), window, cx)
})
.notify_err(workspace, cx);

@ssut
Copy link
Copy Markdown
Contributor Author

ssut commented Aug 18, 2024

Hey! I'm pretty sure you can simplify your code by using this method:

fn notify_err(self, workspace: &mut Workspace, cx: &mut ViewContext<Workspace>) -> Option<T> {
match self {
Ok(value) => Some(value),
Err(err) => {
log::error!("TODO {err:?}");
workspace.show_error(&err, cx);
None
}
}
}

You can see how it's used here:

let terminal = workspace
.project()
.update(cx, |project, cx| {
project.create_terminal(TerminalKind::Shell(working_directory), window, cx)
})
.notify_err(workspace, cx);

Thanks for letting me know, I refactored the code accordingly calling notify_async_err.

Comment thread crates/workspace/src/pane.rs Outdated
@ssut ssut requested a review from mrnugget August 20, 2024 07:20
@mrnugget
Copy link
Copy Markdown
Contributor

Works for me! Thanks!
https://github.com/user-attachments/assets/dcd959df-c49b-48f7-a149-5e0f9db97c4a

@mrnugget mrnugget merged commit b674043 into zed-industries:main Aug 20, 2024
@ssut ssut deleted the feature/workspace-file-open-error-handling branch August 20, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants