Skip to content

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

Merged
merged 18 commits into from
Apr 15, 2024

Conversation

michalpokusa
Copy link
Contributor

@michalpokusa michalpokusa commented Mar 11, 2024

Continuation of #3

💥 Breaking changes:

  • Language enum, language parameter and Markdown/XML support was removed

⭐ Added:

  • Token class for keeping track of tokens postion within template
  • TemplateNotFoundError and TemplateSyntaxError errors that are raised instead of SyntaxError and OSError

🗑️ Deleted:

  • Support for XML and Markdown, the code was over 1000 lines long, which raised error with Pylint, also, there was no really any practical use for them. It in the future an issue about that is created, then a support for custom escaping function might be a good idea, as I can imagine someone wanting to generate a huge JSON output using templating

🛠️ Updated/Changed:

  • While being processed, the template is not being replaced after processing each part, instead processed part is stored as offset, which enables referencing whole template even after processing certain parts, also less asign operations
  • Instead of SyntaxError and OSError, custom TemplateNotFoundError and TemplateSyntaxError are raised
  • Error are raised in more cases, e.g.
    • when circular extends are detected
    • when there are any tokens between blocks in extending template
    • when else, elif, empty, endwhile or similar is used without previous for, while, if
    • when extends/endblock token is found in top-level template (e.g. when was inserted from child's block)
  • When error regarding template syntax is raised, it prints the token and surrounding lines for easy determining the problem source (screenshot below)
  • render_... functions now accept cache parameter (True by 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:

  • Some minor function/variable renaming/moving for better readability

This is example how the new error points to token that caused it:

image


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 pass statements 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 the offset while keeping the template unchanged for the most part, and that might be simply faster, even considering the need to create Token instances.

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 a Template instance, now they cache created template by default, which can be disabled using cache=False argument.

It also makes the code cleaner, as it is no longer need to store a template in global scope.

@michalpokusa michalpokusa force-pushed the template-syntax-error-rework branch from 9676f80 to 5ce56d1 Compare March 12, 2024 03:33
@michalpokusa michalpokusa changed the title Reworked errors in template syntax and overall more validation Reworked errors in template syntax and overall more validation, caching by default Mar 12, 2024
@michalpokusa michalpokusa force-pushed the template-syntax-error-rework branch from 8259b61 to 8870ff6 Compare March 13, 2024 23:42
@michalpokusa michalpokusa force-pushed the template-syntax-error-rework branch from b67d871 to 2c79edb Compare March 14, 2024 00:39
@michalpokusa
Copy link
Contributor Author

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.

@michalpokusa michalpokusa marked this pull request as ready for review March 14, 2024 00:41
@michalpokusa michalpokusa marked this pull request as draft March 29, 2024 13:11
@michalpokusa michalpokusa marked this pull request as ready for review March 29, 2024 23:29
Copy link
Contributor

@FoamyGuy FoamyGuy left a 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.

@FoamyGuy FoamyGuy merged commit d2278d7 into adafruit:main Apr 15, 2024
1 check passed
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Apr 16, 2024
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
@michalpokusa michalpokusa deleted the template-syntax-error-rework branch October 29, 2024 17:10
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.

2 participants