-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Refs #910: Skip template-based widget templates by default #942
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
Refs #910: Skip template-based widget templates by default #942
Conversation
Codecov Report
@@ Coverage Diff @@
## master #942 +/- ##
=======================================
Coverage 83.43% 83.43%
=======================================
Files 31 31
Lines 1672 1672
Branches 249 249
=======================================
Hits 1395 1395
Misses 200 200
Partials 77 77
Continue to review full report at Codecov.
|
debug_toolbar/settings.py
Outdated
'SKIP_TEMPLATE_PREFIXES': ( | ||
'debug_toolbar/', | ||
'django/forms/widgets/', | ||
'admin/widgets', |
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.
Missing trailing slash. If it's intentionally omitted, the docs change is wrong instead :)
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.
Thanks! I meant to include the trailing slash.
@@ -85,7 +85,8 @@ def _store_template_info(self, sender, **kwargs): | |||
|
|||
# Skip templates that we are generating through the debug toolbar. | |||
if (isinstance(template.name, six.string_types) and | |||
template.name.startswith('debug_toolbar/')): |
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.
By making this configurable, it becomes possible to unsilence (accidentally by deleting the wrong line in your own SKIP_TEMPLATE_PREFIXES
, or intentionally) the djdt templates themselves.
Presumably, it's never been requested to be able to do so (or has been rejected for introducing too much configuration when it wasn't previously potentially necessary) so is it worth keeping the hard-coded extant functionality and making the SKIP_TEMPLATE_PREFIXES
an additional set of skips?
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 think I agree with @kezabelle . It's probably going to cause fewer headaches for people if we keep the 'debug_toolbar/' skip hard-coded instead of requiring users who customize the list to include it.
On the other hand, if it makes developing and debugging the toolbar itself easier, I don't think it's a big deal, as long as the default value is clearly documented.
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.
Good point. We can do that later should the need arise. I changed this.
This looks like a simple, effective fix, if not as fancy as de-duplicating or lazy loading (which would allow users to still see skipped templates and contexts if they needed to). |
No description provided.