gh-125600: Only show stale code warning on source code display commands #125601
Conversation
Lib/pdb.py
Outdated
| try: | ||
| filename = self.curframe.f_code.co_filename | ||
| mtime = os.path.getmtime(filename) | ||
| self._file_mtime_table.setdefault(filename, mtime) |
There was a problem hiding this comment.
We don't need to overwrite a previous value if the file changed on disk?
There was a problem hiding this comment.
If not, then let's avoid going to the disk when the dict already has a value.
There was a problem hiding this comment.
Because we separated the update and check. We do not generate warnings on every update. We want to avoid file change -> update(updated the new info) -> check(fail to find the change).
But yes, we should avoid updating it if not necessary.
Lib/pdb.py
Outdated
| @@ -493,6 +493,16 @@ def _cmdloop(self): | |||
| except KeyboardInterrupt: | |||
| self.message('--KeyboardInterrupt--') | |||
|
|
|||
| def _update_file_mtime(self): | |||
There was a problem hiding this comment.
The function name seems wrong - it's not "updating" the mtime if it's already there. It only sets it if it's not there.
Also, the docstring of _validate_file_mtime is no longer correct - it's not "since last time we saw it".
There was a problem hiding this comment.
I changed it to init, do you think it's better? Also changed the docstring a bit.
There was a problem hiding this comment.
Is "save" better than "init"? Because I think save is similar to update. In this specific case, we only care for the entries that are not intialized.
There was a problem hiding this comment.
init sounds to me like you are initialising the actual file_mtime, rather than saving a copy of it. How about save_initial_file_mtime?
There was a problem hiding this comment.
Okay that makes sense. I thought about it and I don't think we need to put it in the command path now. We used to do that because we want the error message on every command. Now we can do it once in setup for all the frames and leave onecmd alone.
Co-authored-by: Irit Katriel <[email protected]>
Co-authored-by: Irit Katriel <[email protected]>
Now we only show this warning on
l,lland any command that triggers a stack print.