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

Don't copy entire Context objects for template tags when all that is needed is their data #178

Merged
merged 1 commit into from
Aug 31, 2017

Conversation

ababic
Copy link
Collaborator

@ababic ababic commented Aug 29, 2017

All menu templates currently work by creating a copy of the current context, then updating the copy with new values before rendering it. I think the tags were originally developed with a misunderstanding about what 'context' is, and treating it as a dictionary.

Since we only need the data from the current Context object, and not any other part of it, we should simplify the code by using context.flatten() to create a dictionary, update the the dictionary as required with new values, then create a new Context object from that dictionary to use for rendering.

This will also help with #89, as it seems that context objects themselves are immutable when using Jinja2.

…g the context.flatten() to create a dict from the original context, updating the dict with new values as necessary, then turn into a new Context object for rendering.
@ababic ababic changed the title Don't copy whole Context objects for template tags when all that is needed is their data Don't copy entire Context objects for template tags when all that is needed is their data Aug 29, 2017
@codecov
Copy link

codecov bot commented Aug 29, 2017

Codecov Report

Merging #178 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #178      +/-   ##
==========================================
- Coverage   98.54%   98.54%   -0.01%     
==========================================
  Files          68       68              
  Lines        2269     2268       -1     
==========================================
- Hits         2236     2235       -1     
  Misses         33       33
Impacted Files Coverage Δ
wagtailmenus/templatetags/menu_tags.py 99.13% <100%> (-0.01%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21e4f5c...39b7421. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 29, 2017

Codecov Report

Merging #178 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #178      +/-   ##
==========================================
- Coverage   98.54%   98.54%   -0.01%     
==========================================
  Files          68       68              
  Lines        2269     2268       -1     
==========================================
- Hits         2236     2235       -1     
  Misses         33       33
Impacted Files Coverage Δ
wagtailmenus/templatetags/menu_tags.py 99.13% <100%> (-0.01%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21e4f5c...39b7421. Read the comment docs.

@ababic ababic merged commit a8c81d4 into jazzband:master Aug 31, 2017
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