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

Nested with, issue 104 #398

Merged
merged 14 commits into from Sep 26, 2021
Merged

Nested with, issue 104 #398

merged 14 commits into from Sep 26, 2021

Conversation

climbus
Copy link
Contributor

@climbus climbus commented Sep 19, 2021

#104

It was tricky issue and need an ugly hack.

I've added support for list of tokens. I think it is more obvious solution compared to adding pseudo object and next if condition: https://github.com/python-rope/rope/blob/master/rope/refactor/patchedast.py#L126

The tricky one was in python2, where nested with and comma separated has identical ast.

If you have more elegant solution, feel free to send me.

@lieryan
Copy link
Member

lieryan commented Sep 19, 2021

Hi @climbus, thank you for working on this.

If it will significantly simplify this ticket, then I think we should just drop support for Python 2.7.

I've been considering for a while that 0.20.0 might just be the last version of Rope to explicitly support Python 2.7.

Python 2.7 is already EOL for quite some time and the 1% of people who still uses with Python 2.7 can just download an older version of Rope. AFAICT, 0.20.0 has covered pretty much all of Python 2.7 features, and 2.7 isn't going to grow new features any time soon 😅.

We'll need a Python 2.7 EOL planning to block 2.7 users from accidentally upgrading to incompatible version.

@lieryan
Copy link
Member

lieryan commented Sep 19, 2021

I've opened #400 to start the ball rolling for dropping support for Python 2.7.

@climbus
Copy link
Contributor Author

climbus commented Sep 19, 2021

If it will significantly simplify this ticket, then I think we should just drop support for Python 2.7.

By now it isn't a big problem. Few lines of code.

I'll add my thoughts in the issue you added.

@climbus
Copy link
Contributor Author

climbus commented Sep 26, 2021

@lieryan could you merge this to master? I have to resolve new conflicts.

@lieryan lieryan added this to the 0.21.x milestone Sep 26, 2021
@lieryan lieryan merged commit 3e81fe6 into python-rope:master Sep 26, 2021
@lieryan
Copy link
Member

lieryan commented Sep 26, 2021

Hi @climbus thanks for working on this issue. I've merged this PR with a couple additional changes.

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.

None yet

2 participants