Tags:
create new tag
, view all tags

Issues with %IF%

Like all TWiki tags, %IF% is evaluated from "the inside out". This has a few non-trivial implications:

Both the then clause and the else clause are be evaluated, even though only one will wind up being used. In the case of both of them being simple text, it's irrelevant, because nothing is evaluated. If the "false" clause contains tags, however, it will be evaluated even though the results will be discarded. This can, at the very least, cause performance hits and possibly unexpected side effects.

Take, for example, this real-life example:

%IF{'NONE%SEARCH{ "^%TOPIC%$" scope="topic" regex="on" nosearch="on" nototal="on"
format="$parent" }%=NONE' then='(none)' 
else='%SEARCH{ "^%TOPIC%$" scope="topic" regex="on"
nosearch="on" nototal="on" format="$parent" }%' }%

The else clause uses a SEARCH which, as we all know, is expensive, and will always be evaluated, even if the test evaluates to true.

A second problem occurs when an %IF% is inside of another tag, such as a current example:

%INCLUDE{"%IF{"context can_login" then="%TWIKIWEB%.!WebLeftBarLogin"}%" warn="off"}% versus %IF{"context can_login" then="%INCLUDE{"%TWIKIWEB%.!WebLeftBarLogin" warn="off"}%"}%

In the former case, all works fine iff "context can_login" is true. Otherwise, the %INCLUDE% expression turns into %INCLUDE{"" warn="off"}% which results in including WebHome, which was the behaviour that I observed. In fact, I can't figure out a way to fix it by adding an else clause; hopefully someone else can.

In the latter case, all should be well as long as %INCLUDE{"%TWIKIWEB%.!WebLeftBarLogin" warn="off"}% evaluates to something reasonable. (Kenneth says that the latter broke his TWiki-4 installation, which doesn't make sense to me unless there's a problem with %TWIKIWEB%.!WebLeftBarLogin, but that's a separate, if related, issue.)

Meredith, you must need glasses. It did not only brake my installation. It also brakes develop.twiki.org. The top of the left bar in the main web is broken. It shows some garbage characters. -- KennethLavrsen - 03 May 2006

Kenneth, you need a pair of X-Ray glasses : The INCLUDE expression that Meredith checked in is the same that was found in DEVELOP before the skin-crash, and it was working perfectly. If it broke your installation, it's because some other change in the codebase. -- RafaelAlvarez

Now you are also saying "broke your installation". It is a problem in develop.twiki.org. I have taken a screenshot from develop.twiki.org and put an arrow pointing at the problem so ensure that noone leaves this topic with the impression that the recent code change and left bar changes only brakes my installation. It brakes any normal installations and we cannot possibly release 4.0.3 like this. My merlin.lavrsen.dk is TWiki4 installed straight out of the box. It is what people see with a default TWiki4. I doubt there are many TWikis being installed without authentication of some kind. The root cause may be a code change, yes. -- KennethLavrsen - 03 May 2006

leftbarerror.png

-- Contributors: MeredithLesly

Discussion

Possibly contrived but valid examples

-- MeredithLesly - 03 May 2006

At this point %IF% is probably being used almost entirely by core developers. In order to forestall incompatible changes, this issue should be resolved ASAP.

-- MeredithLesly - 04 May 2006

On the IF discussion I tend to agree much with Meredith. The IF is a new TWikiVariable introduced in TWiki4 and a very strong and useful new feature. But also a feature that I am sure has given many people problems because of the unexpected and surprising way it works.

Logically from a code point of view it works just like any other TWikiVariable like also Crawford has pointed out. Ie. all variables inside the IF are all expanded before the IF. But for the normal user this is not really what you would expect would happen. You expect the first statement to be evaluated. And if it returns something true you expect the first then expression evaluated using the normal rules. Otherwise the else statement.

Very creative people may have taken advantage of the fact that both then and else are always executed but only one displayed. But I really doubt anyone has really done that. I'd rather think many have been fighting with the current implementation. I do not remember any examples used in the Pattern Skin for example where the then expression is used for something even if the result is not displayed. I seriously doubt we have a compatibility issue to worry about here. But maybe someone can prove me wrong by giving examples.

Meredith's search example is a good example of the fact that the current IF implementation probably cost some performance in many TWiki Apps.

-- KennethLavrsen - 04 May 2006

Meredith's example is dammed good. Pitty the server is showing an error all the time.

I created an extended example on my test server that shows the same example of unnecessary performance hit when not needed. Same example also shows that Meredith has indeed found an issue with the left bar when authentication is disabled.

http://merlin.lavrsen.dk/twiki/bin/view/Myweb/IncludeTest

It takes some time to load and that it the whole point.

-- KennethLavrsen - 04 May 2006

sigh I had a horrible suspicion this might come up at some point.

So why didn't I implement %IF that way?

  1. Because it would require a special case in the parser, which would reverse the evaluation order if the tag seen was %IF. This is very very difficult and very high risk.
  2. Because it is back-to-front w.r.t every other tag, and TWiki has far too many special cases already.
  3. Every case I could think of had an alternative way of writing the IF such that it came out efficiently.
Having said that, I agree it is more intuitive (for programmers). But the efficiency argument is weak, as I think it is possible to write efficient conditions in most cases.

I am far more concerned about the fact that the contents of an else or then could expand in such a way as to make the resulting expression unparseable.

Before you ask, it is very very difficult because TWiki doesn't express a constraint on matching "'s. So "outside-in",

%IF{"$burble" then="%IF{"$burbleburble" then="fred" else="joe"}%"}%
can be parsed two ways:
%IF{"$burble" then="
   %IF{"$burbleburble" then="fred" else="joe"}%
"}%
OR
%IF{"$burble" then="%IF{"$burbleburble" then="fred" else="joe"}%
"}%
the only solution to this would be to restrict what you can put in a then (which is effectively what I did with %TMPL:P) e.g.
   * Set FIRST = fred
   * Set SECOND = joe
%CHOOSE{"$burble" then="FIRST" else="SECOND"}%
i.e. instantiate %FIRST% or %SECOND% as the result after evaluating the condition. (VARIF, PICK, CHOOSE, SELECT all possible names).

-- CrawfordCurrie - 04 May 2006

VARIF's TWiki web topic would be TWiki.VarVARIF, which might get confused with TWiki.VarIF.

Other suggestions: DO, DOIF, ONLYIF.

ProposedFor DakarRelease as I assume this is going to get included in a patch/point release.

-- SamHasler - 04 May 2006

Yes, it should always be possible to write efficient code in most cases. But I don't think it's a good idea to rely on that.

Since one presumes that a) not many people are using IF yet, and b) probably none of those are using it in such a way that would break, it probably should be fixed before a) isn't true any longer.

-- MeredithLesly - 05 May 2006

I am not going to "change the rules" for the IF tag. The parsing implications are too horrendous to contemplate. Unless someone cares to prove me wrong.

Another approach, possibly more palatable.

   * Set FIRST = fred
   * Set SECOND = joe
%IF{ cond thenvar="FIRST" elsevar="SECOND"}%

Where then and thenvar are mutually exclusive, and else and elsevar are similarly exclusive. Of course %IF{ "cond" thenvar="WIKINAME" else="NitWit"}% would be perfectly legal.

-- CrawfordCurrie - 06 May 2006

Can you use that to avoid evaluating both sides though? It would still have to evaluate whatever the variables were set to.

What about if it could be any tag so you could do something like:

   * Set SECOND = blah blah blah
%CHOOSE{ cond thenvar="INCLUDE{\"%TWIKIWEB%.WebLeftBarLogin\" warn=\"off\"}" elsevar="SECOND"}%

-- SamHasler - 06 May 2006

I believe that what CDot's saying is that the parser goes inside out no matter what, so the innermost tags will always be expanded before outer ones. If I'm right about this, then putting anything inside an

IF{ }: Empty expression that could include a double-quote will break things.

This presumably explains why putting the

Warning: Can't find topic ""."" inside the %IF resulted in <DIV\=. Probably there was a double quote after the equals sign, and so that "ended" the then.

-- MeredithLesly - 06 May 2006

Correct.

Instead of thenvar and elsevar, thenexpand and elseexpand are more intention-revealing.

-- CrawfordCurrie - 07 May 2006

Topic attachments
I Attachment History Action Size Date Who Comment
PNGpng leftbarerror.png r1 manage 74.6 K 2006-05-04 - 05:04 KennethLavrsen  
Edit | Attach | Watch | Print version | History: r14 < r13 < r12 < r11 < r10 | Backlinks | Raw View | Raw edit | More topic actions
Topic revision: r14 - 2006-05-07 - CrawfordCurrie
 
  • Learn about TWiki  
  • Download TWiki
This site is powered by the TWiki collaboration platform Powered by Perl Hosted by OICcam.com Ideas, requests, problems regarding TWiki? Send feedback. Ask community in the support forum.
Copyright © 1999-2017 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.