Skip to content

Fix variable scope issue when renaming list comprehension variables (#293)#430

Merged
lieryan merged 23 commits into
python-rope:masterfrom
climbus:issue_293
Oct 9, 2021
Merged

Fix variable scope issue when renaming list comprehension variables (#293)#430
lieryan merged 23 commits into
python-rope:masterfrom
climbus:issue_293

Conversation

@climbus

@climbus climbus commented Oct 6, 2021

Copy link
Copy Markdown
Contributor

Description

Finally, fixed issue #293

Checklist (delete if not relevant):

  • I have added tests that prove my fix is effective or that my feature works
  • I have updated CHANGELOG.md

Comment thread ropetest/refactor/renametest.py Outdated
refactored = self._local_rename(
dedent("""\
dedent(
"""\

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.

Hi @climbus, I think these reformattings should not be made here. There is a setting in pyproject.toml that tells black that it should not be reformatting files in ropetest folder.

Can you modify your editor/black settings to make sure that it honors the pyproject.toml setting and revert these reformatting changes.

Thanks.

@climbus

climbus commented Oct 8, 2021

Copy link
Copy Markdown
Contributor Author

I was thinking why it happens to me ;)

Thanks. I fixed it.

@lieryan lieryan merged commit 030d474 into python-rope:master Oct 9, 2021
@lieryan lieryan changed the title Issue 293 Fix variable scope issue when renaming list comprehension variables (#293) Oct 9, 2021
@lieryan lieryan added this to the 0.21.x milestone Oct 9, 2021
@lieryan

lieryan commented Oct 9, 2021

Copy link
Copy Markdown
Member

Hi @climbus massive thank you and congratulations for fixing this issue.

It had been quite a complex issue to fix this.

@climbus

climbus commented Oct 9, 2021

Copy link
Copy Markdown
Contributor Author

Hi @climbus massive thank you and congratulations for fixing this issue.

It had been quite a complex issue to fix this.

No problem. I was interesting challenge ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants