Skip to content

gh-50948: IDLE: Warn if saving a file will overwrite a newer version - #17578

Merged
serhiy-storchaka merged 7 commits into
python:mainfrom
ZackerySpytz:bpo-6699-idle-overwrite-file
Jun 6, 2026
Merged

gh-50948: IDLE: Warn if saving a file will overwrite a newer version#17578
serhiy-storchaka merged 7 commits into
python:mainfrom
ZackerySpytz:bpo-6699-idle-overwrite-file

Conversation

@ZackerySpytz

@ZackerySpytz ZackerySpytz commented Dec 12, 2019 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

@hauntsaninja hauntsaninja changed the title bpo-6699: IDLE: Warn the user if a file will be overwritten when saving gh-50948: IDLE: Warn the user if a file will be overwritten when saving Jan 7, 2023
serhiy-storchaka
serhiy-storchaka reviewed Dec 27, 2023
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

./python -m idlelib newfile.py fails, because io.set_filename() is called with non-existing file name.

serhiy-storchaka
serhiy-storchaka reviewed Dec 27, 2023
View reviewed changes
Comment thread Lib/idlelib/iomenu.py Outdated
@serhiy-storchaka serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Dec 27, 2023
@serhiy-storchaka

serhiy-storchaka commented Dec 27, 2023

Copy link
Copy Markdown
Member

@terryjreedy, please take a look. I tested this change and fixed it. It LGTM now.

@terryjreedy terryjreedy changed the title gh-50948: IDLE: Warn the user if a file will be overwritten when saving gh-50948: IDLE: Warn if saving a file will overwrite a newer version Dec 27, 2023
@terryjreedy

terryjreedy commented Dec 28, 2023

Copy link
Copy Markdown
Member

  1. loadfile twice calls stat, saving to temp local and thence to self.xxx. Is there an advantage to doing the stat call while the file is open? (Faster?)

  2. One of four stat calls is wrapped in try-except. Should all 4 be so wrapped? Or at least the other save call?

  3. In save, the comparison != might just as well be <. See 4.

  4. The default return for SaveAs and SaveCopyAs is the current file. So I think the stat call and check should either be moved to writefile or put in a new method or function. Not sure of the best option yet.

@serhiy-storchaka

serhiy-storchaka commented Dec 28, 2023

Copy link
Copy Markdown
Member
  1. I I initially was going to use os.fstat() (it is safer), but it cannot be used in other two places, so I leave it for now. Only one of them is called, the second way is executed if the first one fails. It is also more reliable to call here, in try/except blocks.
  2. This one is only can easily fail. All three others are very unlikely to fail, because they are called immediately after reading or writing a file. The two in loadfile are also in try/except blocks.
  3. I also thought about this, but it is better to keep !=. Imagine that you restored the earlier version of the file from archive or backup.
  4. SaveAs updates the current file name and the timestamp. SaveCopyAs keeps the current file name and the timestamp. Both call writefile. Initially updating the timestamp was in set_filename, but I separated them, because it failed if set_filename was called with the name of non-existing file (see gh-50948: IDLE: Warn if saving a file will overwrite a newer version #17578 (review)).

@hugovk hugovk removed the needs backport to 3.11 only security fixes label Apr 22, 2024
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.13 bugs and security fixes label May 9, 2024
@gpolo gpolo mannequin mentioned this pull request Aug 6, 2024
Closed
@hugovk hugovk removed the needs backport to 3.12 only security fixes label Apr 10, 2025
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.14 bugs and security fixes label May 8, 2025
@github-actions

github-actions Bot commented Apr 7, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 7, 2026
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label May 30, 2026
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Jun 4, 2026
@serhiy-storchaka
serhiy-storchaka enabled auto-merge (squash) June 6, 2026 16:48
@serhiy-storchaka
serhiy-storchaka disabled auto-merge June 6, 2026 16:49
@serhiy-storchaka
serhiy-storchaka enabled auto-merge (squash) June 6, 2026 16:49
@serhiy-storchaka
serhiy-storchaka merged commit 69851a6 into python:main Jun 6, 2026
52 checks passed
@miss-islington-app

miss-islington-app Bot commented Jun 6, 2026

Copy link
Copy Markdown

Thanks @ZackerySpytz for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Jun 6, 2026

Copy link
Copy Markdown

GH-151026 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jun 6, 2026
@bedevere-app

bedevere-app Bot commented Jun 6, 2026

Copy link
Copy Markdown

GH-151027 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Jun 6, 2026
@bedevere-app

bedevere-app Bot commented Jun 6, 2026

Copy link
Copy Markdown

GH-151028 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Jun 6, 2026
serhiy-storchaka added a commit that referenced this pull request Jun 6, 2026
@miss-islington @ZackerySpytz
@gpolo @priyapappachan @serhiy-storchaka
[3.15] gh-50948: IDLE: Warn if saving a file will overwrite a newer v…
9607373 
…ersion (GH-17578) (GH-151026)

(cherry picked from commit 69851a6)

Co-authored-by: Zackery Spytz <[email protected]>
Co-authored-by: Guilherme Polo <[email protected]>
Co-authored-by: Priya Pappachan <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit that referenced this pull request Jun 6, 2026
@miss-islington @ZackerySpytz
@gpolo @priyapappachan @serhiy-storchaka
[3.14] gh-50948: IDLE: Warn if saving a file will overwrite a newer v…
3d45ab2 
…ersion (GH-17578) (GH-151027)

(cherry picked from commit 69851a6)

Co-authored-by: Zackery Spytz <[email protected]>
Co-authored-by: Guilherme Polo <[email protected]>
Co-authored-by: Priya Pappachan <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit that referenced this pull request Jun 6, 2026
@miss-islington @ZackerySpytz
@gpolo @priyapappachan @serhiy-storchaka
[3.13] gh-50948: IDLE: Warn if saving a file will overwrite a newer v…
2fa0d4b 
…ersion (GH-17578) (GH-151028)

(cherry picked from commit 69851a6)

Co-authored-by: Zackery Spytz <[email protected]>
Co-authored-by: Guilherme Polo <[email protected]>
Co-authored-by: Priya Pappachan <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
philthompson10 pushed a commit to philthompson10/cpython that referenced this pull request Jun 17, 2026
@ZackerySpytz @gpolo
@priyapappachan @serhiy-storchaka @philthompson10
pythongh-50948: IDLE: Warn if saving a file will overwrite a newer ve…
944bc52 
…rsion (pythonGH-17578)

Co-authored-by: Guilherme Polo <[email protected]>
Co-authored-by: Priya Pappachan <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@terryjreedy terryjreedy Awaiting requested review from terryjreedy terryjreedy is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ZackerySpytz @serhiy-storchaka @terryjreedy @hugovk @the-knights-who-say-ni @ezio-melotti @bedevere-bot