RFC: <txp:if_body> tag

etc · 2014-02-13 20:42:19

This tag could replace all <txp:if_first_... /> / <txp:if_last_... /> tags:

function if_position($atts, $thing)
{
	extract(lAtts(array(
		'context' => 'article',
		'position' => 'first'
	),$atts));
	if(!in_array($context, array('article', 'section', 'category', 'link', 'image', 'file', 'comment'))) return;

	global ${'this'.$context};
	$assert = 'assert_'.$context;
	$assert();
	return parse(EvalElse($thing, !empty(${'this'.$context}['is_'.$position])));
}

Just call e.g. <txp:if_position context="image" position="last" />, and so on. One task = one tag.

Same idea for <txp:if_field context="article" name="excerpt" />, that could replace <txp:if_excerpt />, <txp:if_expires />, the future <txp:if_body />, and the like.

philwareham wrote #278932:

I do agree all these conditionals get a bit confusing when they are nested.

That would be even more true, I’m afraid :)

Last edited by etc (2014-02-13 20:46:18)

Bloke · 2014-02-13 21:55:55

I’ve always been up for a chameleonic conditional tag: that’s why smd_if was invented :-) But I have a programmer’s mindset and, while it makes perfect sense to me to have a tag like etc proposes which save umpteen similar tags, others think differently. We can’t cater for all types of user with a multi-headed mega drill when a simple screwdriver is all some people want.

That said, I really like the above if_position/if_field tags. I’d have no problem introducing something like that, even alongside the existing tags for now — perhaps even converting the internals of the remaining conditionals to just call these two generic tags (at the expense of another item on the call stack). Having them available means that anyone can construct conditionals without going through the txp:variable dance first.

And it makes things easier for any new stuff added to the this arrays: anything there is immediately available for comparison, which is a massive boon for doing geeky things like dropping into PHP at the top of a Form, stuffing some derived (prefixed) data in one of the globals based on some combination of things that have been set (e.g. if this article is in Section ‘zippy’ and has comments enabled and has both article categories set, then add $thisarticle['smd_combopage'] = '1': any template tags later can test such values directly to make templating decisions. Edge cases, for sure, but nice to have such flexibility.

This will become even more interesting when/if the new custom fields system comes online because $thisarticle might well become overloaded with fields, or we may bring in a dedicated $thisfield variable which holds the field data (partly for namespacing reasons). Undecided. But a generic conditional tag would mean it is simple to augment the tag to work with any core objects. Subsequently, if we find that some dedicated conditional tags aren’t used as much we can maybe deprecate the stragglers over time.

The only major things in favour of the convenience tags are readability and amount of typing (kind of both sides of the same coin). This is especially true when nesting. It’s quicker to see and parse the following by sight:

<txp:if_article_image>
   <txp:if_first_image>
      <div dir="rtl">
   </txp:if_first_image>
   <txp:images />
   <txp:if_last_image>
      </div>
   </txp:if_last_image>
</txp:if_article_image>

than this:

<txp:if_field context="image" name="article_image">
   <txp:if_position context="image">
      <div dir="rtl">
   </txp:if_position>
   <txp:images />
   <txp:if_position context="image" position="last">
      </div>
   </txp:if_position>
</txp:if_field>

That’s a contrived example, but you get the idea. On the other hand, with generic conditionals you could abstract that sort of stuff away into a Form that you include on all your pages and then just test a single conditional for both cases, relieving one nested set, which is perhaps even more readable than even the first case.

So it’s a balancing act, as always. And (again, as always) I’m on the fence.

One final mention regarding colak’s suggestion. Remember that txp:variable can be used as a container and is often neater when you want to construct more complicated custom conditionals. Another contrived example:

<txp:variable name="haz_cheezburger"><txp:if_custom_field name="food" value='<txp:section title="0" />'><txp:custom_field name="food" /></txp:if_custom_field></txp:variable>

<txp:if variable name="haz_cheezburger" value="">
Go to McDonalds
<txp:else />
Om nom nom: <txp:variable name="haz_cheezburger" />
</txp:if_variable>

Much nicer than trying to get the apostrophes and double-quotes right inside the value attribute! Though you do have to be careful with newlines, otherwise the if_variable conditional passes control to the else branch if a rogue newline crops up. The great thing is we have so many ways to do this kind of thing.

etc · 2014-02-13 23:04:21

Nice to see Stef’s posts getting longer — I hope your bad form is only a bad souvenir now. :)

Re code readability, context could be set by container tags (article, images, linklist etc), so the code would be

<txp:if_field name="article_image">
  <txp:images ...>
   <txp:if_position>
      <div dir="rtl">
   </txp:if_position>
   <txp:image />
   <txp:if_position position="last">
      </div>
   </txp:if_position>
  </txp:images>
</txp:if_field>

We could also split if_position into if_first/if_last tags, though losing position="even/odd" possibility.

Re chameleons, etc_query replaces for me these and many other tags, but to each his own toy.

NicolasGraph · 2016-07-26 08:03:26

I was wondering why to keep <txp:if_excerpt> or <txp:if_article_image> when we can use the probably badly named (and too long) <txp:if_custom_field> for all article fields (at least for Title, Body etc.)… Is there a chance to see that <txp:if_field> replacing all these conditional tags in 4.7?

etc wrote #278945:

[…] context could be set by container tags […]

+1

Last edited by NicolasGraph (2016-07-26 08:03:50)

Bloke · 2016-07-26 10:18:49

NicolasGraph wrote #300399:

Is there a chance to see that <txp:if_field> replacing all these conditional tags in 4.7?

It would give us some flexibility, yes. And makes it more obvious that the conditional test allows you to test fields from the context that are not just “custom” fields, albeit that the reason it can do so is technically a bug. I’m in no hurry to fix that bug as it is pretty handy!

Shortening if_custom_field to if_field would be fine by me, obviously retaining the old tag for backwards compatibility. But replacing all the other conditionals with it? I don’t know. If nothing else, it’s a lot of upheaval in terms of documentation to update everything to use the proposed tag.

We’ve introduced more if_first_* and if_last_* tags in 4.6 to align them with other things. A good idea? Not sure. But I’m not feeling positive about etc’s proposed if_position tag. However, I would be in favour of a pair of tags instead:

<txp:if_first> ... </txp:if_first>
<txp:if_last> ... </txp:if_last>

Those could default to the current context, e.g. if inside an images tag it would perform the same action as if_first_image / if_last_image. For nested conditionals or where you wanted to break out of the default context, a suitable attribute could allow you to do so, e.g.:

<txp:images>
   <txp:if_first>
      First image: <txp:image />
   </txp:if_first>
   <txp:if_first type="article">
      <txp:excerpt />
   <txp:else />
      <txp:image_info />
   </txp:if_first>
</txp:images>

So that would display the image if it existed and then display the caption beneath it unless the visitor was viewing the first article, in which case it would display the article’s excerpt instead.

Just an idea.

gomedia · 2016-07-26 12:32:58

Bloke wrote #300405:

Just an idea.

Sounds like a great idea to me … code readability, consistency between tags and the provision of basic tools of the trade, and to carry on the conversation from Variable trim attribute? – global attributes: trim, class, html_id, wraptag etc etc.

Bloke · 2016-07-26 12:57:43

gomedia wrote #300407:

global attributes: trim, class, html_id, wraptag etc etc.

Without trying to sideline this thread too much, remember that at the parser level these proposed attributes would apply equally to conditional tags as they do to regular tags. The notion of wraptag, class, html_id, etc. throws up some interesting situations on conditionals:

<txp:images>
   <txp:if_first txp:wraptag="div" txp:class="first-image">
      <txp:image />
   <txp:else />
      <txp:thumbnail />
   </txp:if_first>
</txp:images>

Would that only apply the wraptag and class around the image (i.e. the true branch) or to the thumbnail in the false branch as well? It could be argued that it’s a logic error to apply it to anything but the true condition.

Ponder…

gomedia · 2016-07-26 13:15:49

Bloke wrote #300410:

It could be argued that it’s a logic error to apply it to anything but the true condition.

I would say that it should apply to anything that’s generated inside <txp:if_first>...</txp:if_first>. If you just want to wrap the TRUE content – put a wraptag on the <txp:image /> tag.

If the conditional was called test_first we wouldn’t be pondering at all!

Bloke · 2016-07-26 13:35:50

gomedia wrote #300412:

I would say that it should apply to anything that’s generated inside <txp:if_first>...</txp:if_first>. If you just want to wrap the TRUE content – put a wraptag on the <txp:image /> tag.

True dat. It’s just something to be aware of as we explore the notion of global attributes.

NicolasGraph · 2016-07-26 21:44:06

Bloke wrote #300405:

Shortening if_custom_field to if_field would be fine by me, obviously retaining the old tag for backwards compatibility. But replacing all the other conditionals with it? I don’t know. If nothing else, it’s a lot of upheaval in terms of documentation to update everything to use the proposed tag.

That would require changes but I think it would be easier to document in the future and would avoid eternal requests about if_title, if_body, both, or about any future field. When a bug is as handy as the one told about <txp:if_custom_field>, it should become a feature… If <txp:if_field> would be more than just a bug, we could have only one tag for all fileds; we could deprecate <txp:if_article_image>, <txp:if_excerpt> and others and remove them in a future version.

We’ve introduced more if_first_* and if_last_* tags in 4.6 to align them with other things. A good idea? Not sure. But I’m not feeling positive about etc’s proposed if_position tag. However, I would be in favour of a pair of tags instead:

<txp:if_first> ... </txp:if_first>

<txp:if_last> ... </txp:if_last>

Those could default to the current context, e.g. if inside an images tag it would perform the same action as if_first_image / if_last_image. For nested conditionals or where you wanted to break out of the default context, a suitable attribute could allow you to do so, e.g.:

<txp:images>…

Just an idea.

Sure, <txp:if_first> and <txp:if_last> could be great; <txp:if_position> would be interesting if we could check for any position like <txp:if_position position="3">.
I also agree about the current context by default, and I’m not even sure that it is necessary to add an attribute to change that; I’m ok with the following (and it seems more clear too me — for now at least):

Your code:

<txp:images>

   <txp:if_first>
      First image: <txp:image />
   </txp:if_first>

   <txp:if_first type="article">
      <txp:excerpt />
   <txp:else />
      <txp:image_info />
   </txp:if_first>

</txp:images>

Mine:

(<txp:article>)

    <txp:images>
        <txp:if_first>
            First image: <txp:image />
        </txp:if_first>
    </txp:images>

    <txp:if_first>
        <txp:excerpt />
    <txp:else />
        <txp:images>
            <txp:image_info />
        <txp:images>
    </txp:if_first>

(</txp:article>)

gomedia wrote #300412:

I would say that it should apply to anything that’s generated inside <txp:if_first>...</txp:if_first>. If you just want to wrap the TRUE content – put a wraptag on the <txp:image /> tag.

Seems logical to me too, even it was not obvious at the first read.

Last edited by NicolasGraph (2016-07-28 07:37:52)

Bloke · 2016-07-27 07:08:36

NicolasGraph wrote #300418:

<txp:if_position> would interesting if we could check for any position like <txp:if_position position="3">.

Absolutely. Very handy.

I’m not even sure that it is necessary to add an attribute to change [context]; I’m ok with the following <snip>

OK, but your code doesn’t do the same as mine. Yours repeats the first image caption in the second block, and, crucially, isn’t as efficient as it requires repeating the images call. In this case it probably doesn’t require a second trip to the database but with other tags it might. There’d be no requirement to use the context attribute as it’d do what you expect most of the time, but it’d be there if you need it.

NicolasGraph · 2016-07-27 09:38:40

Bloke wrote #300420:

OK, but your code doesn’t do the same as mine. Yours repeats the first image caption in the second block…

Right! :-/

…and, crucially, isn’t as efficient as it requires repeating the images call. In this case it probably doesn’t require a second trip to the database but with other tags it might. There’d be no requirement to use the context attribute as it’d do what you expect most of the time, but it’d be there if you need it.

Yes, that’s a good argument.
This context attribute just changes the way I see contexts (as something defined by Txp), I probably need to change my mind.

Textpattern CMS

Textpattern CMS support forum

#16 2014-02-13 20:42:19

Re: RFC: <txp:if_body> tag

philwareham wrote #278932:

#17 2014-02-13 21:55:55

Re: RFC: <txp:if_body> tag

#18 2014-02-13 23:04:21

Re: RFC: <txp:if_body> tag

#19 2016-07-26 08:03:26

Re: RFC: <txp:if_body> tag

etc wrote #278945:

#20 2016-07-26 10:18:49

Re: RFC: <txp:if_body> tag

NicolasGraph wrote #300399:

#21 2016-07-26 12:32:58

Re: RFC: <txp:if_body> tag

Bloke wrote #300405:

#22 2016-07-26 12:57:43

Re: RFC: <txp:if_body> tag

gomedia wrote #300407:

#23 2016-07-26 13:15:49

Re: RFC: <txp:if_body> tag

Bloke wrote #300410:

#24 2016-07-26 13:35:50

Re: RFC: <txp:if_body> tag

gomedia wrote #300412:

#25 2016-07-26 21:44:06

Re: RFC: <txp:if_body> tag

Bloke wrote #300405:

gomedia wrote #300412:

#26 2016-07-27 07:08:36

Re: RFC: <txp:if_body> tag

NicolasGraph wrote #300418:

#27 2016-07-27 09:38:40

Re: RFC: <txp:if_body> tag

Bloke wrote #300420:

Board footer