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

Passing url_type into template context. #2545

Merged
merged 2 commits into from Oct 23, 2016

Conversation

felixfontein
Copy link
Contributor

Also modifies base theme to use that when calling url_replacer. This allows to fix the Nikola core part of getnikola/plugins#176.

@@ -1306,6 +1306,7 @@ def render_template(self, template_name, output_name, context, url_type=None):
for k in self._GLOBAL_CONTEXT_TRANSLATABLE:
local_context[k] = local_context[k](local_context['lang'])
local_context['is_rtl'] = local_context['lang'] in LEGAL_VALUES['RTL_LANGUAGES']
local_context['url_type'] = self.config['URL_TYPE'] if url_type is None else url_type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we’re doing all this, perhaps we could pass url_type into this function from callers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should callers pass the argument twice (both into context and to render_template)? After all this value is used in the default base theme in essentially every template.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a reason why anyone calling this function does NOT want to have url_type in context (except that it causes all pages to recompile, that is).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, generic_ don’t use url_type themselves. Feel free to merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks, I'll merge then.

@felixfontein felixfontein merged commit fd273fd into master Oct 23, 2016
@Kwpolska Kwpolska deleted the passing-url-type-to-context branch May 20, 2017 15:29
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.

None yet

2 participants