gh-44968: Add "Reload from Disk" feature to IDLE #141574

ashm-dev · 2025-11-14T22:02:46Z

Issue: Add File - Reload #44968

Lib/idlelib/iomenu.py

sharktide

I also don’t think we should restore the cursor position since a file’s line numbers could also change when it is edited and it could cause confusion.

Co-authored-by: R.C.M <sharktidedev@gmail.com>

Removed cursor position restoration logic during file reload.

ashm-dev · 2025-11-15T07:23:48Z

I also don’t think we should restore the cursor position since a file’s line numbers could also change when it is edited and it could cause confusion.

Rewrote it, now it won't save cursor position.

picnixz

Please rely more on unittest.mock.patch() as well as mock features directly. In addition add a What's New entry in addition to the NEWS entry.

Lib/idlelib/idle_test/test_iomenu.py

bedevere-app · 2025-11-15T13:15:01Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

ashm-dev · 2025-11-15T22:55:16Z

I have made the requested changes; please review again

Doc/whatsnew/3.15.rst

Co-authored-by: R.C.M <sharktidedev@gmail.com>

Doc/whatsnew/3.15.rst

Lib/idlelib/idle_test/test_iomenu.py

Lib/idlelib/mainmenu.py

Misc/NEWS.d/next/IDLE/2025-11-14-20-58-55.gh-issue-44968.rL9dK3.rst

…3.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Doc/whatsnew/3.15.rst

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Changes were addressed.

sharktide · 2025-11-18T21:15:35Z

You don't need to keep updating the branch because we all get emails and it uses up ci time unless its been really long or unrelated tests fail. When merging, git usually can handle it automatically unless you see that there are conflicts

Lib/idlelib/idle_test/test_iomenu.py

Co-authored-by: R.C.M <sharktidedev@gmail.com>

picnixz

Please, your PRs always take me long to review because there are unrelated changes, or un-necessary things that are introduced. It's really not the first time. Carefully read the devguide and honestly, avoid using an LLM. If you are worried about NEWS writing, leave it blank and ask someone else to write it for you. But for the code, there are many places where I feel it's been LLM-generated. In general:

Never remove code that is not related to the issue you are fixing.
Never add type annotations.
Never make unrelated changes.
Read the devguide.
Don't add trivial comments. Usually this is a sign that an LLM has been used.
Follow PEP-8. In particular, wrap your code under the 80-char mark.

picnixz · 2025-11-22T09:42:14Z

Lib/idlelib/idle_test/test_iomenu.py

        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.


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.

picnixz · 2025-11-22T09:43:09Z

Doc/whatsnew/3.15.rst

  (Contributed by Nick Burns and Senthil Kumaran in :gh:`92936`.)


+idlelib


@terryjreedy For IDLE changes, do we add it here or somewhere else in general?

picnixz · 2025-11-22T09:43:25Z

Lib/idlelib/idle_test/test_iomenu.py

+        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])


Suggested change

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])

with patch('idlelib.iomenu.messagebox.showinfo') as mock_showinfo:

result = io.reload(None)

self.assertEqual(result, "break")

mock_showinfo.assert_called_once()

args, _ = mock_showinfo.call_args

self.assertIn("File Not Found", args[0])

picnixz · 2025-11-22T09:43:58Z

Lib/idlelib/idle_test/test_iomenu.py

+            io.loadfile(io.filename)
+            self.assertEqual(text.get('1.0', 'end-1c'), original_content)
+
+            with builtins.open(filename, 'w', encoding='utf-8') as f:


Why using builtins.open now that you have tokenize_open?

I'm pretty sure tokenize_open can only open the file in read mode here

It's not about tokenize_open. It's about just using the plain open. There is no need to use builtins.open now that open is not shadowed.

picnixz · 2025-11-22T09:46:44Z

Lib/idlelib/idle_test/test_iomenu.py

+        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


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

filename = tempfile.mktemp()

self.addCleanup(os_helper.unlink, filename)

with open(filename, "w") as f:

f.write(content)

return filename

We don't care about secure temporary file for a test.

picnixz · 2025-11-22T09:47:03Z

Lib/idlelib/idle_test/test_iomenu.py

        cls.root.destroy()
        del cls.root

+    def _create_tempfile(self, content: str) -> str:


Don't use type annotations.

picnixz · 2025-11-22T09:47:14Z

Lib/idlelib/idle_test/test_iomenu.py

+        text.insert('end', "\n# Unsaved change")
+        io.set_saved(False)
+
+        with patch('idlelib.iomenu.messagebox.askokcancel', return_value=False) as mock_ask:


Please wrap lines under 80 chars.

bedevere-app · 2025-11-22T09:50:38Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

sharktide

Added commit suggestions based on picnixz's comments, also added one suggestion for the news entry

sharktide · 2025-11-22T15:40:21Z

Lib/idlelib/idle_test/test_iomenu.py

        cls.root.destroy()
        del cls.root

+    def _create_tempfile(self, content: str) -> str:


Suggested change

def _create_tempfile(self, content: str) -> str:

def _create_tempfile(self, content):

sharktide · 2025-11-22T15:41:03Z

Lib/idlelib/idle_test/test_iomenu.py

        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.


Suggested change

eq(fix(), shelltext) # Get... call and '\n' not added.

eq(fix(), shelltext) # Get... call and '\n' not added.

sharktide · 2025-11-22T15:42:58Z

Misc/NEWS.d/next/IDLE/2025-11-14-20-58-55.gh-issue-44968.rL9dK3.rst

@@ -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.


Suggested change

easily reload a file from disk, discarding any unsaved changes in the editor.

reload a file from disk, discarding unsaved changes in the editor.

Remove unnecessary words

sharktide · 2025-11-22T15:44:04Z

Lib/idlelib/iomenu.py

+                self.text.focus_set()
+                return "break"
+
+        # Reload the file


Suggested change

# Reload the file

Trivial comment

sharktide · 2025-11-22T15:45:28Z

Lib/idlelib/idle_test/test_iomenu.py

+        text.insert('end', "\n# Unsaved change")
+        io.set_saved(False)
+
+        with patch('idlelib.iomenu.messagebox.askokcancel', return_value=False) as mock_ask:


Suggested change

with patch('idlelib.iomenu.messagebox.askokcancel', return_value=False) as mock_ask:

with patch('idlelib.iomenu.messagebox.askokcancel',

return_value=False) as mock_ask:

sharktide · 2025-11-22T15:52:52Z

Lib/idlelib/idle_test/test_iomenu.py

+            io.loadfile(io.filename)
+            self.assertEqual(text.get('1.0', 'end-1c'), original_content)
+
+            with builtins.open(filename, 'w', encoding='utf-8') as f:


I'm pretty sure tokenize_open can only open the file in read mode here

pythongh-44968: Add "Reload from Disk" feature to IDLE

39e6187

ashm-dev requested a review from terryjreedy as a code owner November 14, 2025 22:02

bedevere-app bot added the awaiting review label Nov 14, 2025

bedevere-app bot mentioned this pull request Nov 14, 2025

Add File - Reload #44968

Open

Use builtins.open for file operations in tests

9dacd3d

sharktide reviewed Nov 15, 2025

View reviewed changes

Lib/idlelib/iomenu.py Outdated Show resolved Hide resolved

sharktide reviewed Nov 15, 2025

View reviewed changes

ashm-dev and others added 2 commits November 15, 2025 09:47

Update Lib/idlelib/iomenu.py

884758b

Co-authored-by: R.C.M <sharktidedev@gmail.com>

Simplify file reload by removing cursor restoration

d6f54a2

Removed cursor position restoration logic during file reload.

Merge branch 'main' into idle

66ab8ce

ashm-dev requested a review from sharktide November 15, 2025 07:24

ashm-dev added 2 commits November 15, 2025 10:38

Fix typo in 'File Not Found' message

c10c5b9

Update error message in test for file reload

7a88c34

picnixz previously requested changes Nov 15, 2025

View reviewed changes

bedevere-app bot removed the awaiting review label Nov 15, 2025

bedevere-app bot added the awaiting changes label Nov 15, 2025

ashm-dev added 8 commits November 15, 2025 23:30

fix import

5614c28

Merge remote-tracking branch 'origin/idle' into idle

3ce95d3

fix: move local imports to global

545ba86

test_iomenu: use unittest.mock.patch and call_args

5f5e769

IDLE tests: use addCleanup for tempfile removal in test_iomenu

da37468

remove comments

66e0495

refactor(tests): Use mock.patch for messagebox dialogs

a154c9f

add news

bd28bb4

ashm-dev requested a review from AA-Turner as a code owner November 15, 2025 22:28

rewrite tests

fe997ec

bedevere-app bot removed the awaiting changes label Nov 15, 2025

Merge branch 'main' into idle

0f1795b

sharktide reviewed Nov 15, 2025

View reviewed changes

Doc/whatsnew/3.15.rst Outdated Show resolved Hide resolved

Update Doc/whatsnew/3.15.rst

7421699

Co-authored-by: R.C.M <sharktidedev@gmail.com>

ashm-dev requested a review from sharktide November 15, 2025 23:03

picnixz reviewed Nov 15, 2025

View reviewed changes

ashm-dev and others added 2 commits November 16, 2025 02:29

Update Misc/NEWS.d/next/IDLE/2025-11-14-20-58-55.gh-issue-44968.rL9dK…

21c5a9c

…3.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Update Doc/whatsnew/3.15.rst

ad17754

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

picnixz reviewed Nov 15, 2025

View reviewed changes

Doc/whatsnew/3.15.rst Outdated Show resolved Hide resolved

Update Doc/whatsnew/3.15.rst

8672683

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

bedevere-app bot added awaiting review and removed awaiting change review labels Nov 15, 2025

ashm-dev added 2 commits November 16, 2025 02:52

fix

a1e94ca

Merge remote-tracking branch 'origin/idle' into idle

9485351

ashm-dev requested a review from picnixz November 15, 2025 23:53

ashm-dev added 5 commits November 16, 2025 03:29

fix tsan

1788b80

fix tests

953dc8d

Merge branch 'main' into idle

a689fd1

Merge branch 'main' into idle

d27c71b

Merge branch 'main' into idle

a77a5c0

sharktide reviewed Nov 18, 2025

View reviewed changes

Lib/idlelib/idle_test/test_iomenu.py Outdated Show resolved Hide resolved

Lib/idlelib/idle_test/test_iomenu.py Outdated Show resolved Hide resolved

ashm-dev and others added 2 commits November 19, 2025 09:51

Update Lib/idlelib/idle_test/test_iomenu.py

18a30d2

Co-authored-by: R.C.M <sharktidedev@gmail.com>

Update Lib/idlelib/idle_test/test_iomenu.py

885f2cf

Co-authored-by: R.C.M <sharktidedev@gmail.com>

ashm-dev requested a review from sharktide November 19, 2025 06:51

picnixz requested changes Nov 22, 2025

View reviewed changes

bedevere-app bot added awaiting changes and removed awaiting review labels Nov 22, 2025

sharktide reviewed Nov 22, 2025

View reviewed changes

		(Contributed by Nick Burns and Senthil Kumaran in :gh:`92936`.)


		idlelib

	def _create_tempfile(self, content: str) -> str:
	def _create_tempfile(self, content):

	eq(fix(), shelltext) # Get... call and '\n' not added.
	eq(fix(), shelltext) # Get... call and '\n' not added.

		@@ -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.

	easily reload a file from disk, discarding any unsaved changes in the editor.
	reload a file from disk, discarding unsaved changes in the editor.

	with patch('idlelib.iomenu.messagebox.askokcancel', return_value=False) as mock_ask:
	with patch('idlelib.iomenu.messagebox.askokcancel',
	return_value=False) as mock_ask:

Uh oh!

gh-44968: Add "Reload from Disk" feature to IDLE #141574

Are you sure you want to change the base?

gh-44968: Add "Reload from Disk" feature to IDLE #141574

Uh oh!

Conversation

ashm-dev commented Nov 14, 2025 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sharktide left a comment

Choose a reason for hiding this comment

Uh oh!

ashm-dev commented Nov 15, 2025

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-app bot commented Nov 15, 2025

Uh oh!

ashm-dev commented Nov 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sharktide commented Nov 18, 2025

Uh oh!

Uh oh!

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-app bot commented Nov 22, 2025

Uh oh!

sharktide left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

ashm-dev commented Nov 14, 2025 •

edited by bedevere-app bot

Loading