GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-153480: Stop IDLE crashing when a file open in the editor is deleted by tonghuaroot · Pull Request #153481 · python/cpython Conversation
|
|
Done: I have made the requested changes; please review again |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for making the requested changes! @serhiy-storchaka: please review the changes made to this pull request. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this works for me now: starting with a not yet existing file, Ignore, reload after the file is recreated, Close. One remaining problem with a cancelled Save As, see the comment.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
A cancelled Save As kept the old mtime and reprompted on every focus. Forget the mtime for both Save As and Ignore; a successful Save As restores it via set_saved(True). Reword the NEWS per review.
|
Fixed: the dialog now forgets the mtime for both Save As and Ignore, so a cancelled Save As no longer reprompts (a successful one restores it via I have made the requested changes; please review again |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for making the requested changes! @serhiy-storchaka: please review the changes made to this pull request. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Found one more case: modify the file, delete it, get the focus back, choose Close. See the comment.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@tonghuaroot Serhiy requested another change 5 days ago. I would like to get this into early October releases. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…hoices With a modified buffer, Close asks whether to save and the queued FocusIn was delivered inside that message box, reopening the dialog. Forget the mtime before showing the dialog (not only for Save As/Ignore); a successful Save As restores it via set_saved(True).
|
Done: I have made the requested changes; please review again |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for making the requested changes! @serhiy-storchaka: please review the changes made to this pull request. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
For the future: It looks like you are force-pushing or something, so that each commit shows the cumulative change rather that just the little change made. For the last one, the diff should be a simple merge commit adding the one. We far prefer the latter for reviewing changes. Serhiy, ready to merge? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your update.
test_deleted_file_returns_none and test_not_yet_created_filename are the same test.
Optional nits: the new tests could use os_helper.temp_dir() and os_helper.create_empty_file() instead of tempfile and pathlib.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| def test_deleted_file_returns_none(self): | ||
| with tempfile.TemporaryDirectory() as d: | ||
| p = os.path.join(d, 'gone.py') | ||
| Path(p).touch() | ||
| os.remove(p) | ||
| stub = types.SimpleNamespace(io=types.SimpleNamespace(filename=p)) | ||
| self.assertIsNone(Editor.last_mtime(stub)) | ||
|
|
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Serhiy, is simply deleting one of the two specified method what you requested?
| def test_deleted_file_returns_none(self): | |
| with tempfile.TemporaryDirectory() as d: | |
| p = os.path.join(d, 'gone.py') | |
| Path(p).touch() | |
| os.remove(p) | |
| stub = types.SimpleNamespace(io=types.SimpleNamespace(filename=p)) | |
| self.assertIsNone(Editor.last_mtime(stub)) |
I am fine with not replacing the working code.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I'm merging as is. Tests can be edited later.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Not clear which changes this refers to. Essential changes have been done.
There was an error while loading. Please reload this page.
|
Thanks @tonghuaroot for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks @tonghuaroot for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks @tonghuaroot for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
GH-158262 is a backport of this pull request to the 3.14 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
GH-158263 is a backport of this pull request to the 3.13 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
GH-158264 is a backport of this pull request to the 3.15 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…s deleted (GH-153481) (#158263) gh-153480: Stop IDLE crashing when a file open in the editor is deleted (GH-153481) EditorWindow.focus_in_event calls last_mtime on every <FocusIn> event, which passed the open file path to os.path.getmtime with no guard. When another program deleted or renamed the file, refocusing the IDLE window let a FileNotFoundError escape the Tk event handler. last_mtime now returns the last known mtime when the path is inaccessible, so refocusing no longer raises and does not prompt to reload a file that is gone. See PR discussion for more. --------- (cherry picked from commit 6af40a6) Co-authored-by: tonghuaroot (童话) <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>
…s deleted (GH-153481) (#158262) gh-153480: Stop IDLE crashing when a file open in the editor is deleted (GH-153481) EditorWindow.focus_in_event calls last_mtime on every <FocusIn> event, which passed the open file path to os.path.getmtime with no guard. When another program deleted or renamed the file, refocusing the IDLE window let a FileNotFoundError escape the Tk event handler. last_mtime now returns the last known mtime when the path is inaccessible, so refocusing no longer raises and does not prompt to reload a file that is gone. See PR discussion for more. --------- (cherry picked from commit 6af40a6) Co-authored-by: tonghuaroot (童话) <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>
…s deleted (GH-153481) (#158264) gh-153480: Stop IDLE crashing when a file open in the editor is deleted (GH-153481) EditorWindow.focus_in_event calls last_mtime on every <FocusIn> event, which passed the open file path to os.path.getmtime with no guard. When another program deleted or renamed the file, refocusing the IDLE window let a FileNotFoundError escape the Tk event handler. last_mtime now returns the last known mtime when the path is inaccessible, so refocusing no longer raises and does not prompt to reload a file that is gone. See PR discussion for more. --------- (cherry picked from commit 6af40a6) Co-authored-by: tonghuaroot (童话) <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Terry Jan Reedy <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.