-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
gh-44968: Add "Reload from Disk" feature to IDLE #141574
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
Changes from 30 commits
39e6187
9dacd3d
884758b
d6f54a2
66ab8ce
c10c5b9
7a88c34
5614c28
3ce95d3
545ba86
5f5e769
da37468
66e0495
a154c9f
bd28bb4
fe997ec
0f1795b
7421699
21c5a9c
ad17754
8672683
a1e94ca
9485351
1788b80
953dc8d
a689fd1
d27c71b
a77a5c0
18a30d2
885f2cf
6397cb7
a3956e0
e55c1fd
c0c4438
cea60ca
a9cd136
6ca016d
a019bca
b20ebb9
16b0e3f
37aec79
2aee349
8016888
12c647f
f40ac09
abda153
ea0dde7
5555133
e767089
8a786d8
e592112
01108b7
e5a79b4
28f3cc0
a2d9aa0
1b0326a
047e5ec
730511d
ab8c014
7dceff5
703c81b
20a9f19
c37a25e
9e2f9e1
23e14ca
1b50172
d601bc3
4e68a7a
e0b0416
41ba1a0
678f161
5b77088
6387a5e
3f21052
4dc0467
947df21
bfb7dd6
6abf303
83ca688
bd14ec2
29de0fe
293a740
5d76785
9739b12
4edec83
86fd240
7369349
1b6b257
f7dcd4e
2987e47
4e840e5
e778021
4fcd09c
1919a0f
32aa46a
1e4815f
1fdf8af
6595639
5ff1393
6014660
8b489af
ca9b915
a3e41e5
576f272
00322c7
1fd61d5
5a8eb3e
e34c226
6cd3ad6
a21d953
2317884
0dd6a73
6d24e09
2331a94
ead61f0
81cb419
0fb7ccb
35666a8
e2c4803
d48522f
2a1c076
38b7a1e
a4c2d0b
da2348e
f2285c1
59d452a
a091f46
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,17 +1,20 @@ | ||||||||||||||||||||||||||
| "Test , coverage 17%." | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from idlelib import iomenu | ||||||||||||||||||||||||||
| import builtins | ||||||||||||||||||||||||||
|
ashm-dev marked this conversation as resolved.
|
||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||
| import tempfile | ||||||||||||||||||||||||||
| import unittest | ||||||||||||||||||||||||||
| from unittest.mock import patch | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from test.support import requires | ||||||||||||||||||||||||||
| from tkinter import Tk | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| from idlelib import iomenu, util | ||||||||||||||||||||||||||
| from idlelib.editor import EditorWindow | ||||||||||||||||||||||||||
| from idlelib import util | ||||||||||||||||||||||||||
| from idlelib.idle_test.mock_idle import Func | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Fail if either tokenize.open and t.detect_encoding does not exist. | ||||||||||||||||||||||||||
| # These are used in loadfile and encode. | ||||||||||||||||||||||||||
| # Also used in pyshell.MI.execfile and runscript.tabnanny. | ||||||||||||||||||||||||||
| from tokenize import open, detect_encoding | ||||||||||||||||||||||||||
| from tokenize import open as tokenize_open, detect_encoding | ||||||||||||||||||||||||||
| # Remove when we have proper tests that use both. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -36,6 +39,14 @@ def tearDownClass(cls): | |||||||||||||||||||||||||
| cls.root.destroy() | ||||||||||||||||||||||||||
| del cls.root | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def _create_tempfile(self, content: str) -> str: | ||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use type annotations.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||
| fd, filename = tempfile.mkstemp(suffix='.py') | ||||||||||||||||||||||||||
| os.close(fd) | ||||||||||||||||||||||||||
| self.addCleanup(os.unlink, filename) | ||||||||||||||||||||||||||
| with builtins.open(filename, 'w', encoding='utf-8') as f: | ||||||||||||||||||||||||||
| f.write(content) | ||||||||||||||||||||||||||
| return filename | ||||||||||||||||||||||||||
|
Comment on lines
+43
to
+48
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
We don't care about secure temporary file for a test. |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def test_init(self): | ||||||||||||||||||||||||||
| self.assertIs(self.io.editwin, self.editwin) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -45,17 +56,88 @@ def test_fixnewlines_end(self): | |||||||||||||||||||||||||
| fix = io.fixnewlines | ||||||||||||||||||||||||||
| text = io.editwin.text | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Make the editor temporarily look like Shell. | ||||||||||||||||||||||||||
| self.editwin.interp = None | ||||||||||||||||||||||||||
| shelltext = '>>> if 1' | ||||||||||||||||||||||||||
| self.editwin.get_prompt_text = Func(result=shelltext) | ||||||||||||||||||||||||||
| eq(fix(), shelltext) # Get... call and '\n' not added. | ||||||||||||||||||||||||||
| eq(fix(), shelltext) # Get... call and '\n' not added. | ||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, this will be the last time I'm going to say this but please never change unrelated code. Do not use an autoformatter, or configure your IDE so that it doesn't autoformat on save, or ask your LLM not to do any change.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||
| del self.editwin.interp, self.editwin.get_prompt_text | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| text.insert(1.0, 'a') | ||||||||||||||||||||||||||
| eq(fix(), 'a'+io.eol_convention) | ||||||||||||||||||||||||||
| eq(fix(), 'a' + io.eol_convention) | ||||||||||||||||||||||||||
| eq(text.get('1.0', 'end-1c'), 'a\n') | ||||||||||||||||||||||||||
| eq(fix(), 'a'+io.eol_convention) | ||||||||||||||||||||||||||
| eq(fix(), 'a' + io.eol_convention) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def test_reload_no_file(self): | ||||||||||||||||||||||||||
| io = self.io | ||||||||||||||||||||||||||
| io.filename = None | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| with patch('idlelib.iomenu.messagebox.showinfo') as mock_showinfo: | ||||||||||||||||||||||||||
| result = io.reload(None) | ||||||||||||||||||||||||||
| self.assertEqual(result, "break") | ||||||||||||||||||||||||||
| mock_showinfo.assert_called_once() | ||||||||||||||||||||||||||
| args, kwargs = mock_showinfo.call_args | ||||||||||||||||||||||||||
| self.assertIn("File Not Found", args[0]) | ||||||||||||||||||||||||||
|
Comment on lines
+74
to
+79
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def test_reload_with_file(self): | ||||||||||||||||||||||||||
| io = self.io | ||||||||||||||||||||||||||
| text = io.editwin.text | ||||||||||||||||||||||||||
| original_content = "# Original content\n" | ||||||||||||||||||||||||||
| modified_content = "# Modified content\n" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| filename = self._create_tempfile(original_content) | ||||||||||||||||||||||||||
| io.filename = filename | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| with patch('idlelib.iomenu.messagebox.showerror') as mock_showerror: | ||||||||||||||||||||||||||
| io.loadfile(io.filename) | ||||||||||||||||||||||||||
| self.assertEqual(text.get('1.0', 'end-1c'), original_content) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| with builtins.open(filename, 'w', encoding='utf-8') as f: | ||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why using builtins.open now that you have
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm pretty sure tokenize_open can only open the file in read mode here
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not about tokenize_open. It's about just using the plain |
||||||||||||||||||||||||||
| f.write(modified_content) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| result = io.reload(None) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| mock_showerror.assert_not_called() | ||||||||||||||||||||||||||
| self.assertEqual(result, "break") | ||||||||||||||||||||||||||
| self.assertEqual(text.get('1.0', 'end-1c'), modified_content) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def test_reload_with_unsaved_changes_cancel(self): | ||||||||||||||||||||||||||
| io = self.io | ||||||||||||||||||||||||||
| text = io.editwin.text | ||||||||||||||||||||||||||
| original_content = "# Original content\n" | ||||||||||||||||||||||||||
| unsaved_content = original_content + "\n# Unsaved change" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| filename = self._create_tempfile(original_content) | ||||||||||||||||||||||||||
| io.filename = filename | ||||||||||||||||||||||||||
| io.loadfile(io.filename) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| text.insert('end', "\n# Unsaved change") | ||||||||||||||||||||||||||
| io.set_saved(False) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| with patch('idlelib.iomenu.messagebox.askokcancel', return_value=False) as mock_ask: | ||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please wrap lines under 80 chars.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||
| result = io.reload(None) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| self.assertEqual(result, "break") | ||||||||||||||||||||||||||
| self.assertEqual(text.get('1.0', 'end-1c'), unsaved_content) | ||||||||||||||||||||||||||
| mock_ask.assert_called_once() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def test_reload_with_unsaved_changes_confirm(self): | ||||||||||||||||||||||||||
| io = self.io | ||||||||||||||||||||||||||
| text = io.editwin.text | ||||||||||||||||||||||||||
| original_content = "# Original content\n" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| filename = self._create_tempfile(original_content) | ||||||||||||||||||||||||||
| io.filename = filename | ||||||||||||||||||||||||||
| io.loadfile(io.filename) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| text.insert('end', "\n# Unsaved change") | ||||||||||||||||||||||||||
| io.set_saved(False) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| with patch('idlelib.iomenu.messagebox.askokcancel', return_value=True) as mock_ask: | ||||||||||||||||||||||||||
| result = io.reload(None) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| self.assertEqual(result, "break") | ||||||||||||||||||||||||||
| self.assertEqual(text.get('1.0', 'end-1c'), original_content) | ||||||||||||||||||||||||||
| mock_ask.assert_called_once() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def _extension_in_filetypes(extension): | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -31,6 +31,7 @@ def __init__(self, editwin): | |||
| self.save_as) | ||||
| self.__id_savecopy = self.text.bind("<<save-copy-of-window-as-file>>", | ||||
| self.save_a_copy) | ||||
| self.__id_reload = self.text.bind("<<reload-window>>", self.reload) | ||||
| self.fileencoding = 'utf-8' | ||||
| self.__id_print = self.text.bind("<<print-window>>", self.print_window) | ||||
|
|
||||
|
|
@@ -40,6 +41,7 @@ def close(self): | |||
| self.text.unbind("<<save-window>>", self.__id_save) | ||||
| self.text.unbind("<<save-window-as-file>>",self.__id_saveas) | ||||
| self.text.unbind("<<save-copy-of-window-as-file>>", self.__id_savecopy) | ||||
| self.text.unbind("<<reload-window>>", self.__id_reload) | ||||
| self.text.unbind("<<print-window>>", self.__id_print) | ||||
| # Break cycles | ||||
| self.editwin = None | ||||
|
|
@@ -237,6 +239,35 @@ def save_a_copy(self, event): | |||
| self.updaterecentfileslist(filename) | ||||
| return "break" | ||||
|
|
||||
| def reload(self, event): | ||||
| """Reload the file from disk, discarding any unsaved changes. | ||||
|
|
||||
| If the file has unsaved changes, ask the user to confirm. | ||||
| """ | ||||
| if not self.filename: | ||||
| messagebox.showinfo( | ||||
| "File Not Found", | ||||
| "This window has no associated file to reload.", | ||||
| parent=self.text) | ||||
| self.text.focus_set() | ||||
| return "break" | ||||
|
|
||||
| if not self.get_saved(): | ||||
| confirm = messagebox.askokcancel( | ||||
| title="Reload File", | ||||
| message=f"Discard changes to {self.filename}?", | ||||
| default=messagebox.CANCEL, | ||||
| parent=self.text) | ||||
| if not confirm: | ||||
| self.text.focus_set() | ||||
| return "break" | ||||
|
|
||||
| # Reload the file | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Trivial comment |
||||
| self.loadfile(self.filename) | ||||
|
|
||||
| self.text.focus_set() | ||||
| return "break" | ||||
|
|
||||
| def writefile(self, filename): | ||||
| text = self.fixnewlines() | ||||
| chars = self.encode(text) | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,3 @@ | ||||||
| Add "Reload from Disk" menu item to IDLE's File menu. This allows users to | ||||||
| easily reload a file from disk, discarding any unsaved changes in the editor. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Remove unnecessary words |
||||||
| Patch by Shamil Abdulaev. | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@terryjreedy For IDLE changes, do we add it here or somewhere else in general?