RFC: <txp:if_body> tag

etc · 2014-02-13 16:43:56

colak wrote #278916:

I guess the advantage of the variable over the other methods is that it only makes one db query.

Yiannis, none of these methods queries the DB, all data is already retrieved by <txp:article[_custom] /> tag. Actually, if_custom_field solution is potentially cleaner, since it does not require parsing body field when checking.

etc · 2014-02-13 16:53:02

philwareham wrote #278918:

For consistency, there probably should be a core <txp:if_body> tag.

Please, no! Multiplying similar tags ad infinitum is a bad idea. For consistency, I would rather suggest replacing all these <txp:if_article_something /> by <txp:if_article type="something" />. And the weirdest code-replicating tags are <txp:if_first_something />, but it’s too late now(?).

philwareham · 2014-02-13 19:30:07

Ok then, don’t open an issue! :)

I did put emphasis on ‘probably’. I do agree all these conditionals get a bit confusing when they are nested.

Maybe give an extended example of how this conditionals stuff all could be cleaned up.

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.

Textpattern CMS

Textpattern CMS support forum

#13 2014-02-13 16:43:56

Re: RFC: <txp:if_body> tag

colak wrote #278916:

#14 2014-02-13 16:53:02

Re: RFC: <txp:if_body> tag

philwareham wrote #278918:

#15 2014-02-13 19:30:07

Re: RFC: <txp:if_body> tag

#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:

Board footer