[Python] Decorator refactoring #743
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also called "qualified names", just like the context is called now. Additionally, add meta scopes for qualified names and generic names for more precise testing.
... and use it to match function calls less explicitly and more magically. Usually you would think this is bad, but it's not because it's not really less explicit. It just avoids repetition when scoping the last (non-special) part of a qualified name with a name other than "meta.generic-name". This change triggers the bug described in sublimehq/sublime_text#1190, where the most-inner context in a multi-push operation *must* match at least one character before it can be popped. Due this, the tests are currently *not* passing.
Adheres to conventions discussed in sublimehq#709 and sublimehq#737.
wbond
reviewed
Dec 19, 2016
| - match: (?=\.?\s*{{path}}\s*\() # now we do | ||
| set: [decorator-function-call-wrapper, qualified-name-until-leaf] | ||
| - match: (?=\.?\s*{{path}}) | ||
| push: [decorator-wrapper, qualified-name-until-leaf] |
There was a problem hiding this comment.
So this (and two lines above) is where you are pushing two contexts, the later of which does not consume anything?
There was a problem hiding this comment.
Exactly. The latter may or may not consume something, but due to the bug it always consumes the first character.
|
Note that after this is merged, https://github.com/FichteForks/Packages/tree/python_unpacking-operators can probably be merged too. It's been a while since I looked into this and I obviously can't test it myself. |
deathaxe
pushed a commit
to deathaxe/sublime-packages
that referenced
this pull request
Jun 9, 2019
…opes [Python] Decorator refactoring
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requires sublimehq/sublime_text#1190 to be fixed before merging!
When trying to implement proper scoping according to #709 and #737, I noticed that I would have to rebuild almost the entire
function-callsscope for decorators only with different scope names. Since I religiously hate duplicated code (especially considering I would have to duplicate it twice), I worked on refactoring how qualified names are matched instead andqualified-name-until-leafwas born.Unfortunately, it is affected by the bug described in sublimehq/sublime_text#1190 and thus is not ready to be merged yet. The following 17 failing tests should be fixed when this bug is fixed:
Note: I am unsure if I split the commits correctly (and tests are passing after each of them).