Skip to content

Fix save on tab close using wrong parent reference#5

Open
srfwb wants to merge 3 commits into
AlexHettle:mainfrom
srfwb:fix/save-on-tab-close
Open

Fix save on tab close using wrong parent reference#5
srfwb wants to merge 3 commits into
AlexHettle:mainfrom
srfwb:fix/save-on-tab-close

Conversation

@srfwb
Copy link
Copy Markdown

@srfwb srfwb commented May 11, 2026

Summary

Fixes #4 -- TabManager could not save files when closing a tab or quitting
the app because it relied on finding action_save() on its parent widget.
That method does not exist on the parent (a QFrame container), so the save
was silently skipped and data was lost.

What changed

  • tab_manager.py: TabManager now receives file_manager in its constructor.
    close_tab() and prompt_save_all() save the editor content directly via
    file_manager.save_file() instead of looking for action_save() on a parent.
  • main_window.py: passes file_manager to TabManager on creation.

Additional fix: untitled tabs

While fixing the above, I discovered a pre-existing bug in the same code
path: closing an untitled tab (no file path) and clicking Save did nothing
-- the code was silently lost. This was never working because the original
action_save() lookup already failed on the QFrame parent.

close_tab() and prompt_save_all() now call file_manager.save_file_as()
for untitled tabs, which opens a Save As dialog. If the user cancels the
dialog, the tab stays open.

After fix

  • Open two tabs, edit Tab 1, switch to Tab 2, close Tab 1 via X, click Save -- file is saved
  • Quit the app with unsaved tabs, click Save -- files are saved

  • Close an untitled tab with code, click Save -- Save As dialog appears
  • Cancel the Save As dialog -- tab stays open, code is not lost

@srfwb srfwb marked this pull request as ready for review May 11, 2026 21:43
@AlexHettle
Copy link
Copy Markdown
Owner

I appreciate you jumping in to find and work on a fix for this! My initial review for it has me thinking that this is good, however, this is leading to some test failures in the test suite:

image

I always appreciate the help, just make sure that when implementing changes that you are either making sure you aren't breaking any tests, or adding/updating them when fitting. See if you can update these tests to take your new changes into account, and add any if you think it makes sense to. I'll check back in when you do, thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Save" on tab close silently discards changes instead of saving

2 participants