-
Notifications
You must be signed in to change notification settings - Fork 26
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
Refactor resolver into a tree of callable objects, or partially evaluated #95
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
061c3ca
Convert resolver to a pre-evaluated tree
Pike 577f38f
refactor lookup into bundle, change dirty, and make named arguments work
Pike 0e89199
Fix VariableReference
Pike 849061c
Fix numbers
Pike e2f8395
use isolation
Pike 9a75356
fix external variables to be fluent types
Pike ef8972f
fix attributes and value-less messages
Pike b3423f3
Add protection against abuse.
Pike 4eeeba7
remove unused code
Pike b490f5b
fix datetime
Pike 4cd29ff
remove unused code, flake8
Pike 67a2b36
We expand attributes at compile time
Pike ec7b732
We don't use bundle in the compiler
Pike 6d6c21a
simplify VariantList and VariantExpression
Pike 5172851
self-review nits on VariableReference.
Pike 3ebafce
comment
Pike 584723c
update dependencies
Pike 32f1cb7
use dict comprehension
Pike ea239e1
Clean up module namespace in resolver, add docstring to clarify what …
Pike File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
from __future__ import absolute_import, unicode_literals | ||
from fluent.syntax import ast as FTL | ||
from . import resolver | ||
|
||
|
||
class Compiler(object): | ||
def __init__(self, use_isolating=False): | ||
self.use_isolating = use_isolating | ||
|
||
def __call__(self, item): | ||
if isinstance(item, FTL.BaseNode): | ||
return self.compile(item) | ||
if isinstance(item, (tuple, list)): | ||
return [self(elem) for elem in item] | ||
return item | ||
|
||
def compile(self, node): | ||
nodename = type(node).__name__ | ||
if not hasattr(resolver, nodename): | ||
return node | ||
kwargs = vars(node).copy() | ||
for propname, propvalue in kwargs.items(): | ||
kwargs[propname] = self(propvalue) | ||
handler = getattr(self, 'compile_' + nodename, self.compile_generic) | ||
return handler(nodename, **kwargs) | ||
|
||
def compile_generic(self, nodename, **kwargs): | ||
return getattr(resolver, nodename)(**kwargs) | ||
|
||
def compile_Placeable(self, _, expression, **kwargs): | ||
if self.use_isolating: | ||
return resolver.IsolatingPlaceable(expression=expression, **kwargs) | ||
if isinstance(expression, resolver.Literal): | ||
return expression | ||
return resolver.Placeable(expression=expression, **kwargs) | ||
|
||
def compile_Pattern(self, _, elements, **kwargs): | ||
if ( | ||
len(elements) == 1 and | ||
isinstance(elements[0], resolver.IsolatingPlaceable) | ||
): | ||
# Don't isolate isolated placeables | ||
return elements[0].expression | ||
if any( | ||
not isinstance(child, resolver.Literal) | ||
for child in elements | ||
): | ||
return resolver.Pattern(elements=elements, **kwargs) | ||
if len(elements) == 1: | ||
return elements[0] | ||
return resolver.TextElement( | ||
''.join(child(None) for child in elements) | ||
) |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this kind of dynamic lookup of attributes on the
resolver
module, I think it would help comprehension if:__all__
attribute for show what it is exporting clearly, including all theBaseResolver
subclasses.Otherwise someone new to the code base sees the
resolver.Message
class, for instance, but can't find any other references to it or work out how it is actually used.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a doc string to the module and cleaned up the global namespace a bit more.
I didn't go for adding an
__all__
after all. I started doing it, and then looked at the impact it had on our code paths, and there weren't that I could see. In particular, the pre-evaluator imports the module, and the complete namespace is on the module object. The only thing that would differ isfrom fluent.runtime.resolver import *
, which we're not doing. Thus the__all__
would mostly be a source of typos and overlooked changes.