gh-80744: do not read .pdbrc twice when cwd == $home#136816
gh-80744: do not read .pdbrc twice when cwd == $home#136816gaogaotiantian merged 10 commits intopython:mainfrom
Conversation
Lib/pdb.py
Outdated
| except OSError: | ||
| pass | ||
| if os.path.join( | ||
| os.path.abspath(os.curdir), ".pdbrc" |
There was a problem hiding this comment.
I think the equivalent check should be os.path.abspath(".pdbrc") right?
There was a problem hiding this comment.
@gaogaotiantian yeah i don't know what i was thinking there, updated
btw would it be ok to send a separate pr updating the pdb module & its tests to start using pathlib or preferring to keep os.path ?
Lib/test/test_pdb.py
Outdated
| rc_path = os.path.join(cwd, ".pdbrc") | ||
| with open(rc_path, "w") as f: | ||
| f.write("invalid") | ||
| self.assertEqual(pdb.Pdb().rcLines[0], "invalid") |
There was a problem hiding this comment.
I think you should combine this line and the line below. Just check pdb.Pdb().rcLines directly.
There was a problem hiding this comment.
I've changed it self.assertEqual(pdb.Pdb().rcLines[-1], "invalid"), this test is not patching a .pdbrc at $HOME so potentially there can be other lines first in rcLines, so now it only checks that the last one comes from the file written during the test
incidentally, i just found there are 5 other tests that fail if you do happen to have a non-empty .pdbrc at $home, it's ok in CI because there is none, but locally a few unrelated things fail when the file is present
Lib/test/test_pdb.py
Outdated
| self.assertEqual(pdb.Pdb().rcLines[0], "invalid") | ||
| self.assertEqual(len(pdb.Pdb().rcLines), 1) | ||
|
|
||
| def test_readrc_home_twice(self): |
There was a problem hiding this comment.
Let's rename the function name to be something more descriptive. test_readrc_cwd_is_home maybe?
|
@gaogaotiantian could you take a look at the latest version? thanks! |
|
This looks good to me. @gaogaotiantian, do you want to take another look? I can see possible ways to improve it, but nothing major:
|
|
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 |
|
@gaogaotiantian added the news item & applied petr's suggestion to reuse the expanded result |
|
I have a final minor suggestion. We don't normally do cosmetic change but since we are working on this piece already, we might as well change |
saucoide
force-pushed
the
pdbrc-repeat
branch
from
cdf5b2d to
d3d79f8
Compare
|
@gaogaotiantian done |
3 participants
This is for #80744
There is a previous PR fixing this issue #12731 but it's been abandoned for years as the author is not active in github anymore