The Wayback Machine - https://web.archive.org/web/20241214213007/https://github.com/python/cpython/pull/17578
Skip to content
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

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ZackerySpytz
Copy link
Contributor

@ZackerySpytz ZackerySpytz commented Dec 12, 2019

@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
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

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
Copy link
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
Copy link
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
Copy link
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)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting merge needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants