-
-
Notifications
You must be signed in to change notification settings - Fork 33.6k
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
base: main
Are you sure you want to change the base?
Changes from all 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
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 | ||||||||||||||||||||||||||
| 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?