Opened 17 years ago

Closed 4 months ago

#6378 closed New feature (duplicate)

Capture arbitrary output as a template variable

Reported by: Kenneth Arnold Owned by: Carl Meyer
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: kenneth.arnold@…, charette.s@…, oss@…, loic84, cmawebsite@…, Carlton Gibson, Sébastien Corbin Triage Stage: Unreviewed
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I'd like to suggest including ​http://www.djangosnippets.org/snippets/545/ in Django because of its generality and simplicity, compared to adding the equivalent functionality to each individual tag, or trying to use the block tag for this purpose as has been suggested.

Attachments (1)

6378-into.diff​ (3.8 KB ) - added by Eric Holscher 16 years ago.
Initial patch that is a bit rough.

Download all attachments as: .zip

Change History (34)

comment:1 by Simon Greenhill <dev@…>, 17 years ago

Triage Stage: Unreviewed → Design decision needed

comment:2 by Malcolm Tredinnick, 17 years ago

Search out the discussions from last year on django-dev where we were talking about how to extend the "with" tag to work with tags, not just variables. I don't think any real consensus was reached, but something that is a block-based version of "with" (and spelt more or less the same) is much more likely to have legs here. Making people learn yet another tag that isn't related to the existing thing that works like this ("with") isn't good API.

comment:3 by Kenneth Arnold, 16 years ago

Cc: kenneth.arnold@… added

comment:4 by Eric Holscher, 16 years ago

@Malcolm: I couldn't find the discussion from django-dev, "with" is stripped out by google's search. Anyway, here is an idea I've had about this functionality.

How about implementing this as an optional "into" (colored appropriately) where you could have a statement along the lines of 

{% with person.get_absolute_url as main_url into content %}
check out my page: {{ main_url }}
{% endwith %}

This would then instead of outputting the included block into the template, stuff it into the content variable.

However, this doesn't cover the situation where you don't want to put a new variable into the context, but you simply want to put that output into a variable. Something along the lines of {% with [None|magic_word] into content %} was the best I could come up with.

I think that capturing the output of a block is a different operation than including a variable into the inner block's context. Maybe a better solution would be to have a {% withblock as content %} tag that is a counterpart to the "with" tag.

It feels like this functionality shouldn't go in two places, so maybe "withblock" would also have the ability to set the context variables inside. Seems bad to have the functionality in two places either way. The best situation would probably be a sane way to define the capture syntax on the "with" tag, without settings the context variable.

comment:5 by Eric Holscher, 16 years ago

I wrote up a patch with this code in it. It has tests, but the test:

            'with04': ('{{ content }}-{% with dict.key as key into content %}{{ key }}{% endwith %}-{{ content }}', {'dict': {'key': 50}}, '--50'),

Is failing only when

Template test (TEMPLATE_STRING_IF_INVALID='INVALID'): with04 -- FAILED. Expected '--50', got u'50--50'

When the TEMPLATE_STRING_IF_INVALID="", it works fine and only inserts into the context afterwards. When it is set to INVALID, it appears to break. I don't really understand this but will investigate further.

by Eric Holscher, 16 years ago

Attachment: 6378-into.diff​ added

Initial patch that is a bit rough.

comment:7 by nathan, 15 years ago

This would be a great addition to the template language. All to often I add "wrapper" blocks to cancel out HTML so its style doesn't effect the layout. This leads to added template code in child templates.

I've always seen {% with %} more for reassigning variables but if we can augment it to hand code snippets that would be nice but I don't mind learning new tags :)

Scenario: ​http://gist.github.com/221803

comment:8 by jtinsky, 15 years ago

I agree this would be helpful in a wide variety of situations. "{% with %}" doesn't always get the job done.

comment:9 by nathan, 15 years ago

Another use-case that would be handy if you wanted to reuse block content:

{% capture as title %}
  {% block title %}{% endblock %}
{% endcapture %}

<html>
    <head>
        <title>{{ title }}</title>
    </head>
    <body>
        <h1>{{ title }}</h1>
    </body>
</html>

comment:10 by Julien Phalip, 14 years ago

Has patch: set
Needs documentation: set
Patch needs improvement: set
Severity: → Normal
Type: → New feature

Patch needs to be updated as the proposed API really isn't suitable. The "capture as" tag above would certainly stand a better chance. See also #14262 for a related feature request.

comment:11 by Carl Meyer, 13 years ago

Easy pickings: unset
Resolution: → wontfix
Status: new → closed
UI/UX: unset

This needs a stronger use-case justification. Many of the use-cases for this are indications that the template should be re-structured, or an {% include %} could be used instead. Capturing output like this is yet another entirely different mental model for organizing and reusing template code, and we already have at least two of those - we need strong justification to add another to core (especially since it can work just fine as an external snippet/library).

comment:12 by anonymous, 13 years ago

Does this justify as a general use-case?

{% capture as subject %}
	{% if user = request.user %}My{% else %}{{ user.first_name }}'s{% endif %}
{% endcapture %}
...
<div id="profile-photo">
	My Photo
	<img />
</div>
...
<div id="profile-friends">
	<h3>{{ subjects }} Friends</h3>
	List of friends...
</div>
<div id="profile-groups">
	<h3>{{ subjects }} Groups</h3>
	List of groups...
</div>
<div id="profile-friends">
	<h3>{{ subjects }} Friends</h3>
	List of friends...
</div>
...

comment:13 by anonymous, 13 years ago

Resolution: wontfix
Status: closed → reopened

comment:14 by anonymous, 13 years ago

Owner: changed from nobody to Carl Meyer
Status: reopened → new

comment:15 by Carl Meyer, 13 years ago

Triage Stage: Design decision needed → Accepted

I've changed my opinion on this; I do think there are cases where the "capture" model is the appropriate one, and it ought to be built-in. Particularly in cases like the above example, where a non-trivial block of template should be repeated. Inheritance doesn't support this. Includes could be used, but impose a significant performance penalty, both for doing the include and because the repeated block has to be evaluated multiple times. Otherwise you're reduced to a custom template tag.

Marking accepted, pending any argument from other core developers :-)

comment:16 by Aymeric Augustin, 13 years ago

The use case doesn't seem that compelling to me — the logic calculating "subject" /could/ be handled in the view.

That said, I don't have a strong opinion on this feature.

in reply to:  16 comment:17 by Carl Meyer, 13 years ago

Replying to aaugustin:

The use case doesn't seem that compelling to me — the logic calculating "subject" /could/ be handled in the view.

It could be, but it shouldn't be. Display logic in the view is ugly, just like business logic in a template. And HTML in strings in the view is even uglier.

comment:18 by Simon Charette, 13 years ago

Cc: charette.s@… added

comment:19 by aaron@…, 12 years ago

Another use-case

cmsplugin_zinnia/templates/cmsplugin_zinnia/entry_detail.html:

{% load i18n placeholder_tags %}
{% for entry in entries %}
{% captureas content %}
{% render_placeholder entry.content_placeholder %}
{% endpatureas %}
  {% with object=entry object_content=content continue_reading=1 %}
  {% include "zinnia/_entry_detail.html" %}
  {% endwith %}
{% empty %}
  <p class="notice">{% trans "No entries yet." %}</p>
{% endfor %}

This is because rendering entry.content_placeholder generates sekizai_tags that need to get picked up.

comment:20 by oss@…, 12 years ago

Cc: oss@… added

comment:21 by Collin Anderson, 12 years ago

Cc: cmawebsite@… added

This would make the template language turing-complete. I would use it. :) You could even just name the tag var. Though we may be opening a can of worms :)

Last edited 12 years ago by Collin Anderson (previous) (diff)

comment:22 by loic84, 11 years ago

Cc: loic84 added

comment:23 by anonymous, 10 years ago

Any work on this ?

comment:24 by Collin Anderson, 10 years ago

Cc: cmawebsite@… removed

Btw, if we did this, we would need to remove the line in philosophy docs about intentionally not allowing assignment to variables.

​https://docs.djangoproject.com/en/dev/misc/design-philosophies/#don-t-invent-a-programming-language

comment:25 by Collin Anderson, 10 years ago

Cc: cmawebsite@… added

comment:26 by Preston Timmons, 10 years ago

Isn't the assignment to variables line kinda wrong anyway? Most template tags accept the "as" argument, and the with tag also assigns variables, albeit in a scoped manner.

comment:27 by Matthew Somerville, 9 years ago

Repeated includes hopefully no longer impose a significant performance penalty since #23516, and blocktrans has a variable setting argument since #21695, which was a reason given for this functionality (see e.g. similar #14078).

comment:28 by Collin Anderson, 9 years ago

Fascinating. It sounds like #21695 actually solves this ticket, doesn't it? The blocktrans tag should take care of any use case here, right? You just need to do a null translation?

comment:29 by Tim Graham, 9 years ago

Resolution: → wontfix
Status: new → closed

Also #18651 enables optional assignments for simple_tag().

I guess "wontfix" is the best resolution since we didn't really address the issue directly as described in the original description.

comment:30 by Carlton Gibson, 15 months ago

Cc: Carlton Gibson added

comment:31 by Sébastien Corbin, 4 months ago

Cc: Sébastien Corbin added
Resolution: wontfix
Status: closed → new

I guess "wontfix" is the best resolution since we didn't really address the issue directly as described in the original description.

Allow me to reopen this since the original issue has not really been dealt with.

I made a summary of the arguments found in this issue, and ​in this discussion as well, with some additions from myself after ​coming from this:

  • it is a simple tag in terms of implementation, hence that should not be complex to maintain
  • it solves simple situations where an {% include %} tag would maybe be too much
    • it allows to factorize and reuse a variable, scoped to the current template (preserving template directory cleanness)
    • {% tag_name as var %} (previously known as assigment_tag), does not solve complex (conditional) blocks
    • {% include %} or {% with %} can't even solve everything, e.g. making a variable from a complex (eg: conditional, multiline, etc.) block that can be passed to {% blocktranslate %} afterwards 
      {% fragment as script_textarea %}
        <div>
            <textarea {{ copy_attrs }} rows="4"><script>const sc = document.createElement("script");sc.setAttribute("src", "{{ FRONT_BASE_URL }}{% static 'overlay.js' %}");document.head.appendChild(sc);</script></textarea>
            {% if something %}
               some contextual text
            {% endif %}
        </div>
    {% endfragment %}
    <div class="w-help-text">
        {% blocktrans with settings_url=settings_url script=script_textarea trimmed %}
            After <a href="{{ settings_url }}" target="_blank">adding your site as allowed to display the overlay</a>,
            add the following script tag to your site (preferably in the <code>&lt;head&gt;</code> tag):
            {{ script }}
            then add a link to your page with the CSS class <code>overlay-button</code>, for example:
        {% endblocktrans %}
    </div>
  • in a MVT paradigm, the manner to render HTML should be independent from the data sent by the view
    • no business should be put in the template
    • but no HTML in the view either, and calculation logic may be too nested to be put in the view
  • even if it could solve this, {% blocktrans %} is meant for translation, not reusing HTML
    • although I would retain some of the blocktranslate arguments, like trimmed
  • make some code more readable, for example, do we prefer: 
    {% dialog id="modal-cell-"|add:column.name|add:"-row-"|add:row.index title=column.label classname="large" %}
    {{ dialog_content }}
{% enddialog %}
    or 
    {% fragment as dialog_id %}modal-cell-{{ column.name }}-row-{{ row.index }}{% endfragment %}
{% dialog id=dialog_id title=column.label classname="large" %}
    {{ dialog_content }}
{% enddialog %}
  • it can work just fine as an external template library
  • that would make DTL turing-complete, but that argument is disputable, at the end it is a matter of what the template designer choose to do
  • Jinja ​allows this, use Jinja
    • yes, but jinja is not always the choice made at the beginning of a project, and that argument could be further pushed to "why DTL at all?"

comment:32 by Carlton Gibson, 4 months ago

Triage Stage: Accepted → Unreviewed

Grrr, reopening a ticket after 9 years without a clear consensus first being reached is pretty strong. Ideally that would have happened on the Forum first. If I were still Fellowing I would re-close and insist on that. See TicketClosingReasons/DontReopenTickets

I'm going to mark this as Unreviewed so the Fellows get to see it, otherwise there's a danger it just sits here as Accepted.

I'm (on the record as) leaning towards doing something here — it DOES keep coming up — but it's at least related to, if not an outright duplicate of, the new #35535, which has a PR, and for which the Forum discussion linked back here too, so there's some Triage to be done as to whether there's a separate ticket to be implemented here too or not. (IIRC the discussion leading to #35535 being accepted concluded that that would be roughly the limit of what we might add in core.)

Last edited 4 months ago by Carlton Gibson (previous) (diff)

comment:33 by Sarah Boyce, 4 months ago

Resolution: → duplicate
Status: new → closed

Agree that work here is supersede by #35535 and so closing as a duplicate.

Note: See TracTickets for help on using tickets.
Back to Top