Skip to content

workspace: Fix autosave on buffer change in multibuffer#50686

Merged
MrSubidubi merged 4 commits intozed-industries:mainfrom
OmChillure:fix-autosave-on-buffer-change-in-multibuffer
Mar 5, 2026
Merged

workspace: Fix autosave on buffer change in multibuffer#50686
MrSubidubi merged 4 commits intozed-industries:mainfrom
OmChillure:fix-autosave-on-buffer-change-in-multibuffer

Conversation

@OmChillure
Copy link
Copy Markdown
Contributor

@OmChillure OmChillure commented Mar 4, 2026 •
edited by MrSubidubi
Loading

Fixes #50526
Fixes #42841
Fixes #49875

Summary

Fix autosave: on_focus_change not firing reliably when leaving editors with nested focus targets (e.g. multibuffer/search flows).

Root cause

Autosave on focus change was wired to on_blur of the item focus handle.
on_blur only fires when that exact handle is the focused leaf, which misses common descendant-to-outside focus transitions.

Fix

In crates/workspace/src/item.rs, switch autosave subscription from:
- cx.on_blur(&self.read(cx).focus_handle(cx), ...)

to:
- cx.on_focus_out(&self.read(cx).focus_handle(cx), ...)

Autosave behavior and guards remain unchanged:

  • only for AutosaveSetting::OnFocusChange
  • only when focus truly left the item (!contains_focused)
  • skipped when modal is active (!has_active_modal)

Impact

  • Fixes missed autosaves when moving focus from item descendants to other UI (terminal, file tree, search inputs, etc.).
  • No behavior change for other autosave modes (off, after_delay, on_window_change).

Video

Screencast.from.2026-03-04.15-23-07.webm

Release Notes:

  • Fixed an issue where "autosave": "on_focus_change" would not reliably work in multibuffers.

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Mar 4, 2026
@MrSubidubi MrSubidubi changed the title fix autosave on buffer cahgne in multibuffer workspace: Fix autosave on buffer change in multibuffer Mar 4, 2026
@jrmajor
Copy link
Copy Markdown

jrmajor commented Mar 4, 2026

Thank you so much for taking this on! It's been a significant headache for me.

Tested this locally, seems it also fixes #42841. #49875 sounds similar too, although I didn't test that one.

@OmChillure
Copy link
Copy Markdown
Contributor Author

OmChillure commented Mar 4, 2026

@jrmajor glad my PR fixed 3 issues, thanks for tagging

MrSubidubi
MrSubidubi reviewed Mar 4, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@MrSubidubi MrSubidubi left a comment

Choose a reason for hiding this comment

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

Thanks for confirming @jrmajor !

This also looks good to me, however, I think we would benefit if we could add a test for this in test_autosave. To be honest, this might be a bit tedious, but I'd really like for us to ensure that we do not ever regress on this again. Could you look into that?

@MrSubidubi MrSubidubi self-assigned this Mar 4, 2026
@OmChillure
Copy link
Copy Markdown
Contributor Author

OmChillure commented Mar 4, 2026 •
edited
Loading

Hey @MrSubidubi I was looking into the test for above but landed on a test failure in worksapce that stopping me form testing, was writing test in worskpace > test_autosave

can you check it here : #50701

@MrSubidubi
Copy link
Copy Markdown
Member

MrSubidubi commented Mar 4, 2026

Thanks for filing this, we are taking a look.

@MrSubidubi
Copy link
Copy Markdown
Member

MrSubidubi commented Mar 4, 2026

Thanks to @osiewicz , the issue is now fixed on the latest main

@OmChillure
Copy link
Copy Markdown
Contributor Author

OmChillure commented Mar 5, 2026

Hey @MrSubidubi

Added : test_autosave_on_focus_change_in_multibuffer

which:

  • Verifies that switching focus between child handles of the same item does not trigger autosave
  • Verifies that blurring the window does trigger autosave when a child handle owns focus (the regression case)
  • Verifies that deactivating the window does trigger autosave when a child handle owns focus

To support this test,
TestItem was extended with an optional set of child focus handles via with_child_focus_handles(count, cx), simulating a multibuffer-like item with nested focusable sub-editors.

Can you check if the test is fine and covers everything

MrSubidubi
MrSubidubi approved these changes Mar 5, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@MrSubidubi MrSubidubi left a comment

Choose a reason for hiding this comment

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

Nice, very good find, fix and test. Thank you!

@MrSubidubi MrSubidubi enabled auto-merge (squash) March 5, 2026 09:42
@MrSubidubi MrSubidubi merged commit b5881c9 into zed-industries:main Mar 5, 2026
28 checks passed
@sarmadgulzar sarmadgulzar mentioned this pull request Mar 13, 2026
Merged
@zelenenka zelenenka added the guild Pull requests by someone in Zed Guild. NOTE: the label application is automated via github actions label Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MrSubidubi MrSubidubi MrSubidubi approved these changes

Assignees

@MrSubidubi MrSubidubi

Labels

cla-signed The user has signed the Contributor License Agreement guild Pull requests by someone in Zed Guild. NOTE: the label application is automated via github actions

Projects

None yet

Milestone

No milestone

4 participants

@OmChillure @jrmajor @MrSubidubi @zelenenka