Opened 4 weeks ago
Closed 3 weeks ago
#37050 closed Cleanup/optimization (wontfix)
Improve simple_tag / simple_block_tag error message when as is used without a variable name
| Reported by: | Shima Fallah | Owned by: | Shima Fallah |
|---|---|---|---|
| Component: | Template system | Version: | 6.0 |
| Severity: | Normal | Keywords: | |
| Cc: | Shima Fallah | Triage Stage: | Unreviewed |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
When using Library.simple_tag() or Library.simple_block_tag(), templates that end with as but omit the target variable currently raise a generic argument error. This is valid behavior (it fails), but the message is unclear for template authors.
Examples
{% load custom %}{% one_param 37 as %}
{% load custom %}{% div as %}content{% enddiv %}
Current behavior
A generic TemplateSyntaxError is raised from argument parsing, which does not clearly explain that the variable name after as is missing.
Expected behavior
Raise a clear, direct error, e.g.:
'one_param' tag requires a variable name after 'as'
'div' tag requires a variable name after 'as'
What this change does
Adds an explicit check in both compile paths (simple_tag and simple_block_tag) for trailing as without a following variable.
Raises a specific TemplateSyntaxError message before normal argument parsing.
Tests
Adds 4 regression tests covering invalid trailing as usage:
simple tag without args: {% no_params as %}
simple tag with args: {% one_param 37 as %}
simple block tag without args: {% div as %}...{% enddiv %}
simple block tag with args: {% one_param_block 37 as %}...{% endone_param_block %}
These tests verify the new explicit error messages.
Change History (2)
follow-up: 2 comment:1 by , 4 weeks ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:2 by , 3 weeks ago
| Easy pickings: | unset |
|---|---|
| Resolution: | → wontfix |
| Status: | assigned → closed |
Replying to Shima Fallah:
I have submitted a pull request here: https://github.com/django/django/pull/21134
I've fixed the code styling issues (line length) in my branch. Ready to open a new PR as soon as the ticket is accepted.
Hello Shima Fallah, thanks for the report.
After some local testing and investigation, in my opinion the current error message 'one_param' received too many positional arguments is sufficiently informative. A template author can reasonably diagnose the problem from it. The as keyword without a following variable name is an unusual mistake, and adding a special case for it would increase code complexity without a bigger benefit.
Additionally, the proposed fix does not account for the case where a context variable named as is passed as an argument to a multi-parameter tag. Using your examples above, a context variable named as is valid and the following is a valid usage of the div tag:
{% div as %}Something{% enddiv %}
Because of all the above, I don't think this change is warranted.
I have submitted a pull request here: https://github.com/django/django/pull/21134
I've fixed the code styling issues (line length) in my branch. Ready to open a new PR as soon as the ticket is accepted.