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

Fix one line regexp for functions #11

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix one line regexp for functions #11

wants to merge 3 commits into from

Conversation

snoack
Copy link
Contributor

@snoack snoack commented Nov 23, 2015

I looked into the issue you told me about the other day, that one_line_regex.match_magic() doesn't work when called from within a function, but only when called from the top-level scope.

That is because accessing a variable that hasn't been defined results into a LOAD_GLOBAL instruction. But you assign to the caller's local scope. However, if the function is called from the top-level scope f_globals == f_locals. That is the reason it still worked in that scenario. So the fix is as simple as assigning to the callers global scope instead, and voila magic_match() works within function calls as well.

Of course this bears side effects like polluting the global scope, but I'm confident this is the only way to make it work (without a decorator rewriting the byte code). Also there is still a corner case left that seems to be impossible to tackle (without rewriting the byte code):

def foo():
    match = None
    magic_match(r'.*', 'foo')
    print match

In that case FAST_LOAD is used to lookup the variable match and globals are ignored. However, assigning to locals wouldn't help either, because the value of the variable is looked up from the stack.

The other changes in this PR are just minor code cleanups (isolated in separate commits) to allow importing this file actually as module and passing additional parameters such as flags.

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.

1 participant