-
Notifications
You must be signed in to change notification settings - Fork 1
Reworked errors in template syntax and overall more validation, caching by default #4
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
Reworked errors in template syntax and overall more validation, caching by default #4
Conversation
… for underlining token in template
9676f80 to
5ce56d1
Compare
8259b61 to
8870ff6
Compare
b67d871 to
2c79edb
Compare
|
All template syntax error that I was able to think of should be catched, if I missed something please point to that and I will update. Thanks in advance for testing/review. |
FoamyGuy
left a comment
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.
This looks good to me.
I tested the modified examples successfully on a Feather S3 TFT. I also tested the template example from the httpserver repo examles/ dir.
Updating https://github.com/adafruit/Adafruit_CircuitPython_HTTPServer to 4.5.8 from 4.5.7: > Merge pull request adafruit/Adafruit_CircuitPython_HTTPServer#94 from michalpokusa/connection-manager-and-ap-examples Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 3.2.5 from 3.2.4: > Merge pull request adafruit/Adafruit_CircuitPython_Requests#172 from DJDevon3/DJDevon3-openskynetwork_public Updating https://github.com/adafruit/Adafruit_CircuitPython_TemplateEngine to 2.0.0 from 1.2.0: > Merge pull request adafruit/Adafruit_CircuitPython_TemplateEngine#4 from michalpokusa/template-syntax-error-rework Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Updated download stats for the libraries
Continuation of #3
💥 Breaking changes:
Languageenum,languageparameter and Markdown/XML support was removed⭐ Added:
Tokenclass for keeping track of tokens postion within templateTemplateNotFoundErrorandTemplateSyntaxErrorerrors that are raised instead ofSyntaxErrorandOSError🗑️ Deleted:
🛠️ Updated/Changed:
offset, which enables referencing whole template even after processing certain parts, also less asign operationsSyntaxErrorandOSError, customTemplateNotFoundErrorandTemplateSyntaxErrorare raisedrender_...functions now acceptcacheparameter (Trueby default) that saves the template the first time the function is ran and then reuses it, even with different contexts, this should improve performance in most use cases as previously one had to create a class instance, store it in global scope and then call its method. Although that still works, now there is a more readable alternative.🏗️ Refactor:
This is example how the new error points to token that caused it:
Performance 🏎️:
When it comes to performance, from my testing on somewhat complicated template with 40+ tokens, multiple includes and levels of extending I noticed ~15% more time is needed, of course it only applies to creating the template itself, not using it's render methods. The additional
passstatements seem to make no difference on the speed of the function, even when there are hundreds of them.On a simple template that does not require reading a file from disk I did not see any significant increase in time needed, sometimes the new version was even faster that current one. This might be explaing by the fact thst new version does not replace the value of
template(potentially a really big string), but only moves theoffsetwhile keeping thetemplateunchanged for the most part, and that might be simply faster, even considering the need to createTokeninstances.It seems like reading templates from disk is the thing that is the biggest bottleneck.
Partly because of this I changed how caching works.
Before, all
render_functions did not cache the template, but because they are more handy to use than manually creating aTemplateinstance, now they cache created template by default, which can be disabled usingcache=Falseargument.It also makes the code cleaner, as it is no longer need to store a template in global scope.