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

Vim mode :wq doesn't save a buffer if it's open in another pane #21059

Closed
1 task done
Daste745 opened this issue Nov 22, 2024 · 6 comments · Fixed by #24603
Closed
1 task done

Vim mode :wq doesn't save a buffer if it's open in another pane #21059

Daste745 opened this issue Nov 22, 2024 · 6 comments · Fixed by #24603
Labels
bug [core label] vim

Comments

@Daste745
Copy link
Contributor

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

When a buffer is open in multiple panes using the :wq vim mode command doesn't perform a save. :wq only saves when a buffer is the last one to be quit.

This is an issue for me mainly when renaming symbols accross the project. It only surfaces when all affected files are already open as panes. When I perform a rename, "accept" it by running :wq, I'd expect all files in the "rename x -> y" multibuffer to be saved regardless if they are already open elsewhere. I'm guessing it's the same scenario where all files open in the multibuffer are open in other buffers causing the save action to be skipped.

Reproduction steps:

  1. Open a file
  2. Open the same file in another pane (e.g. a split)
  3. Use :wq to save and quit the file in one of the splits
  4. Observe the file not being saved

Reproduction (video):

Screencast_20241122_150111.webm

Environment

Zed: v0.161.2 (Zed)
OS: Linux Wayland endeavouros unknown
Memory: 22.3 GiB
Architecture: x86_64
GPU: AMD Radeon Graphics (RADV RENOIR) || radv || Mesa 24.2.6-arch1.1

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your Zed.log file to this issue.

Zed.log

@Daste745 Daste745 added admin read bug [core label] labels Nov 22, 2024
@nirs
Copy link

nirs commented Nov 23, 2024

I could not reproduce with Zed 0.162.4 on macOS 15.1.

What I did:

  1. open new file
  2. write one line
  3. save (:wq)
  4. split right
  5. change the line on the right pane
  6. save (:wq)

Both panes looks saved.

Maybe this is linux specific bug.

@Daste745
Copy link
Contributor Author

You saved the file before opening it in another pane @nirs, but the issue here is when the file is open in two panes at once and you do :wq in one of them. Expected behavior for me would be to save and quit the file in the right pane, but currently it only gets closed without any saving, leaving the file unsaved in the first pane.

I'm able to reproduce on MacOS, latest Zed version.

System specs
Zed: v0.162.5 (Zed)
OS: macOS 15.0.0
Memory: 16 GiB
Architecture: aarch64

@nirs
Copy link

nirs commented Nov 23, 2024

You saved the file before opening it in another pane @nirs, but the issue here is when the file is open in two panes at once and you do :wq in one of them.

The file was opened in both panes, and modified in the second pane. The difference is that I did save (:w) not save and quit (:wq). I can reproduce it now with (:wq).

@JosephTLyons JosephTLyons added vim and removed triage labels Dec 3, 2024
@dinocosta
Copy link
Contributor

@ConradIrwin Was taking a look at this one after seeing it in the vim channel notes. Although I believe I understand why it's happening, I'm not sure what the correct approach to fix it would be, as it seems I'm missing context on some of the decisions in the code's implementation.

I've nailed it down to this section of the code 🔽

// Check if this view has any project items that are not open anywhere else
// in the workspace, AND that the user has not already been prompted to save.
// If there are any such project entries, prompt the user to save this item.
let project = workspace.update(&mut cx, |workspace, cx| {
    for open_item in workspace.items(cx) {
        let open_item_id = open_item.item_id();
        if !item_ids_to_close.contains(&open_item_id) {
            let other_project_item_ids = open_item.project_item_model_ids(cx);
            dirty_project_item_ids
                .retain(|id| !other_project_item_ids.contains(id));
        }
    }
    workspace.project().clone()
})?;

If the same buffer is open in two different splits, it'll end up being removed from the dirty_project_item_ids vector, seeing as the same Project Item ID that is dirty will be present in open_item.project_item_model_ids(cx). Since the ID is removed from the vector, the should_save variable that is used to determine whether to save the file, will be set to false, preventing the file from being saved.

Is there any tips/hints you can share? It looks like the code doesn't actually check the "the user has not already been prompted to save" bit, is there another condition the code should be checking for before removing the Project Item ID from dirty_project_item_ids? 🤔

@ConradIrwin
Copy link
Member

@dinocosta I think we could make that code conditional on the SaveIntent. :wq provides a SaveIntent::Save, which should save all files even if they are open in other places.

@dinocosta
Copy link
Contributor

@dinocosta I think we could make that code conditional on the SaveIntent. :wq provides a SaveIntent::Save, which should save all files even if they are open in other places.

Awesome, thank you for the suggestion! I've opened a Pull Request with the changes – #24561 – I'll move the implementation discussion there, if you're ok with that ✌️

ConradIrwin added a commit that referenced this issue Feb 13, 2025
Supercedes #24561
Closes #21059

Before this change we would skip saving multibuffers regardless of the
save intent. Now we correctly save them.

Along the way:
* Prompt to save when closing the last singleton copy of an item (even
if it's still open in a multibuffer).
* Update our file name prompt to pull out dirty project items from
multibuffers instead of counting multibuffers as untitled files.
* Fix our prompt test helpers to require passing the button name instead
of the index. A few tests were passing invalid responses to save
prompts.
* Refactor the code a bit to hopefully clarify it for the next bug.

Release Notes:

- Fixed edge-cases when closing multiple items including multibuffers.
Previously no prompt was generated when closing an item that was open in
a multibuffer, now you will be prompted.
- vim: Fix :wq in a multibuffer
Shidfar pushed a commit to Shidfar/zed that referenced this issue Feb 14, 2025
Supercedes zed-industries#24561
Closes zed-industries#21059

Before this change we would skip saving multibuffers regardless of the
save intent. Now we correctly save them.

Along the way:
* Prompt to save when closing the last singleton copy of an item (even
if it's still open in a multibuffer).
* Update our file name prompt to pull out dirty project items from
multibuffers instead of counting multibuffers as untitled files.
* Fix our prompt test helpers to require passing the button name instead
of the index. A few tests were passing invalid responses to save
prompts.
* Refactor the code a bit to hopefully clarify it for the next bug.

Release Notes:

- Fixed edge-cases when closing multiple items including multibuffers.
Previously no prompt was generated when closing an item that was open in
a multibuffer, now you will be prompted.
- vim: Fix :wq in a multibuffer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [core label] vim
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants