Fixing scopes region by offset #429
Conversation
rope/base/pyscopes.py
Outdated
| @@ -135,6 +136,14 @@ def get_name(self, name): | |||
| return self.builtin_names[name] | |||
| raise exceptions.NameNotFoundError("name %s not found" % name) | |||
|
|
|||
| @utils.saveit | |||
| def calculate_scope_regions(self): | |||
There was a problem hiding this comment.
Should we prefix this method with an underscore for now? I don't think that calculate_scope_regions() is a function that we want to expose as a public interface for this class.
There was a problem hiding this comment.
I was thinking about that. It is used outside class hierarchy: https://github.com/python-rope/rope/pull/429/files#diff-17ca262a23eefd8aedad6b0b8a032ef11a5a67123f647c3edb02ad932af9fa27R111
I'll change name.
I'm thinking if i can move regions calculations to scope creation phase. But i need to think about performance. How often scope is created without needs for regions.
ropetest/pyscopestest.py
Outdated
| def test_get_scope_for_offset_for_function_scope_and_async_with_statement(self): | ||
| scope = libutils.get_string_scope( | ||
| self.project, | ||
| "async def func():\n async with a_func() as var:\n print(var)\n", |
|
fixed all |
| % node.__class__.__name__, | ||
| RuntimeWarning, | ||
| ) | ||
| return |
There was a problem hiding this comment.
Hi @climbus, is there any special reason why you removed this warning?
I'm a bit hesitant of removing this warning without understanding the context of why it was added in the first place, and especially since this does not just remove the warning but by removing the return statement, it would've also allowed re-patching an already patched ast.
I've merged the rest of the PR which I think is good, but restored this warning for now. If you strongly believe that this warning is no longer relevant in current rope and should be removed, please re-open a new ticket so we can discuss it properly.
There was a problem hiding this comment.
Ok, I'll check if it should or shouldn't be raised.
Description
I've found some bugs with my last feature: #426.
I think I've fixed it.
Checklist (delete if not relevant):