Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to reload a file #18395

Merged
merged 10 commits into from
Oct 15, 2024
Merged

Conversation

schpet
Copy link
Contributor

@schpet schpet commented Sep 26, 2024

Closes #13212

Release Notes:

  • Added reload command
  • vim: Added :e[dit], :e[dit]! which calls reload

Copy link

cla-bot bot commented Sep 26, 2024

We require contributors to sign our Contributor License Agreement, and we don't have @schpet on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@maxdeviant maxdeviant changed the title feat: Reload file Add ability to reload a file Sep 26, 2024
@ConradIrwin
Copy link
Member

@schpet Thanks for this!

We have the code to reload already - for when you click "Discard" in the case of conflict on save (

fn reload(&mut self, project: Model<Project>, cx: &mut ViewContext<Self>) -> Task<Result<()>> {
).

This will ensure it also works for remote projects and should resolve the file status (though I'm less sure how well it deals with cursor position, as it's currently only called when an alert is open :D).

To make it work for vim, you can add it to the commands here:

("w", "rite"),

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Sep 27, 2024
@@ -604,6 +604,8 @@ fn generate_commands(_: &AppContext) -> Vec<VimCommand> {
VimCommand::new(("$", ""), EndOfDocument),
VimCommand::new(("%", ""), EndOfDocument),
VimCommand::new(("0", ""), StartOfDocument),
// TODO: vim prints "E37: No write since last change (add ! to override)" if you have unsaved changes and you run :edit without the bang
VimCommand::new(("e", "dit"), editor::actions::ReloadFile).bang(editor::actions::ReloadFile),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

looking for suggestions on how to handle this. in vim, :edit will not throw away your changes, but :edit! will. to quote https://vimhelp.org/editing.txt.html#:edit

:e[dit] [++opt] [+cmd]	Edit the current file.  This is useful to re-edit the
			current file, when it has been changed outside of Vim.
			This fails when changes have been made to the current
			buffer and 'autowriteall' isn't set or the file can't
			be written.
			Also see ++opt and +cmd.

							:edit! discard
:e[dit]! [++opt] [+cmd]
			Edit the current file always.  Discard any changes to
			the current buffer.  This is useful if you want to
			start all over again.
			Also see ++opt and +cmd.

							:edit_f

i don't think i ever run :e without any arguments in vim, always :e!, is there a way to only map :e! :edit! in zed? or will that require changes to VimCommand ?

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 either keep them the same (as you have done), and improve it later; or we could add another action that is just a no-op. I think it'd be better to keep them the same (until we add argument support and we get another try).

.ok();
});
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm going to attempt to figure this out but if someone happens to look at this and knows what i need to do to call the existing reload function i'm interested in learning (i don't know rust very well at all)

Copy link
Member

Choose a reason for hiding this comment

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

You should just be able to call self.reload() in your action callback. Does that not work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh! thanks. c3a72e1 attempts to do that. i am curious if

  • is Item as WorkspaceItem legit?
  • is let _ = self.reload(project, cx); legit or should i try to do something with the result

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh: also wondering if project.clone() makes sense

Copy link
Member

Choose a reason for hiding this comment

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

You probably need the clone to keep the borrow checker happy (= may not do what you might expect in rust https://cirw.in/blog/rust-assignment).

With the result you should be able to call .notify_err() on the result to have it show up:

 pub fn reload_file(&mut self, _: &ReloadFile, cx: &mut ViewContext<Self>) {
        let Some(workspace) = self.workspace() else {
           return;
        }
        let Some(project) = self.project.clone() else {
            return;
        }
        self.reload(project, cx).notify_err(workspace, cx)
    }

(though I haven't tried running it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

huh! really appreciate the link and the post.

landed on 1b1a690 - self.reload(project, cx).detach_and_notify_err(cx);

…reee

* origin/main:
  git blame gutter: Use smallest possible space (zed-industries#18145)
  Fix minimum gutter line number spacing (zed-industries#18021)
  terraform: Bump to v0.1.1 (zed-industries#18382)
  lsp: Do not notify all language servers on file save (zed-industries#17756)
  Remove leftover println statements (zed-industries#18389)
  Fix `use_on_type_format` setting being unused per language (zed-industries#18387)
  Avoid panic by only restoring workspace if UI has launched (zed-industries#18386)
  Fix Typo in rust language guide (zed-industries#18383)
  editor: Fix cursor shape not restoring when setting removed (zed-industries#18379)
  Avoid unwrap in file finder (zed-industries#18374)
@schpet schpet marked this pull request as ready for review September 27, 2024 21:12
@schpet
Copy link
Contributor Author

schpet commented Sep 27, 2024

@cla-bot check

Copy link

cla-bot bot commented Sep 27, 2024

The cla-bot has been summoned, and re-checked this pull request!

* upstream/main: (33 commits)
  Fine-tune hunk control spacing (zed-industries#18463)
  Add tooltip for code actions icon button (zed-industries#18461)
  More git hunk highlighting fixes (zed-industries#18459)
  Move git hunk controls to the left side (zed-industries#18460)
  Capitalize tooltip labels on buffer search (zed-industries#18458)
  Install cargo-edito without extra features (zed-industries#18457)
  Fix bugs in diff hunk highlighting (zed-industries#18454)
  Remove Qwen2 model (zed-industries#18444)
  vim: Command selection fixes (zed-industries#18424)
  Add a `get-release-notes-since` script (zed-industries#18445)
  Fix GoToDefinition changing the viewport unnecessarily (zed-industries#18441)
  docs: Ollama api_url improvements (zed-industries#18440)
  Fix missing tooltips for selected buttons (zed-industries#18435)
  Add missing shortcuts in tooltips (zed-industries#18282)
  assistant: Fix copy/cut not working when selection is empty (zed-industries#18403)
  Fix the numeration in line wrap docs (zed-industries#18428)
  ssh remoting: Show error if opening connection timed out (zed-industries#18401)
  project: Fix worktree store event missing in remote projects (zed-industries#18376)
  Fix read timeout for ollama (zed-industries#18417)
  vim: Support za (zed-industries#18421)
  ...
* upstream/main: (76 commits)
  Remove a debug dev config line (zed-industries#18689)
  Update Rust crate thiserror to v1.0.64 (zed-industries#18677)
  Add command palette action name to outline panel docs (zed-industries#18678)
  Add basic outline panel docs (zed-industries#18674)
  ssh: Add session state indicator to title bar (zed-industries#18645)
  Update Rust crate clap to v4.5.19 (zed-industries#18660)
  Sort dependencies in `Cargo.toml` files (zed-industries#18657)
  collab: Revert changes to Clickhouse event rows (zed-industries#18654)
  Replace isahc with async ureq (zed-industries#18414)
  Update cloudflare/wrangler-action digest to 168bc28 (zed-industries#18651)
  Prepare to sync other kinds of settings (zed-industries#18616)
  editor: Ensure proposed changes editor is syntax-highlighted when opened (zed-industries#18648)
  language: Update buffer doc comments (zed-industries#18646)
  Revert "Fix blurry cursor on Wayland at a scale other than 100%" (zed-industries#18642)
  Tweak warning diagnostic toggle (zed-industries#18637)
  Adjust spacing and sizing of buffer search bar icon buttons (zed-industries#18638)
  docs: Add missing UI font settings to "Configuring Zed" (zed-industries#18267)
  Use `const` over `static` for string literals (zed-industries#18635)
  docs: Add FIPS mode error to Linux troubleshooting (zed-industries#18407)
  v0.157.x dev
  ...
@alexanderjeurissen
Copy link

@schpet Great work on the PR, what is blocking us from shipping this ? how can we move this along ?

@schpet
Copy link
Contributor Author

schpet commented Oct 8, 2024

@alexanderjeurissen thanks! i'm waiting for the original reviewer to get back from a trip before following up with them. otherwise this just needs a set of discerning eyes and access to the merge button.

@ConradIrwin
Copy link
Member

Thanks for this, and sorry for not hitting merge before disappearing for two weeks!

@ConradIrwin ConradIrwin merged commit 56163b1 into zed-industries:main Oct 15, 2024
13 checks passed
noaccOS pushed a commit to noaccOS/zed that referenced this pull request Oct 19, 2024
Closes zed-industries#13212

Release Notes:

- Added reload command
- vim: Added `:e[dit]`, `:e[dit]!` which calls reload
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.

Reset or refresh opened file
3 participants