Feature Proposal: Add an isempty operator to the IF variable

Motivation

I was trying to build an expression (think SQL query) where some parts must be added only if an URLParam is passed and is not empty.

I tried to use the IF's defined operator, and it works only if the parameter is not present in the CGI query, but it doesn't work if the parameter is present but is empty.

Description and Documentation

Instead of trying to hack an ugly TML expression (is it possible to do with pure TML?), I added the isempty operator to the IF variable? (pretty trivial to do, thanks Crawford!)

Examples

      * Set SUMMARY=%<nop>IF{"NOT isempty summary" then="Summary like '%%URLPARAM{"summary"}%%' and "}%

Impact

Implementation

The patch against rev 13708 is attached.

-- Contributors: RafaelAlvarez - 16 May 2007

Discussion

No-brainer. Looks like a useful enhancement.

-- PeterThoeny - 17 May 2007

Can the same method be used to compare a string with %TOPIC% (and %BASETOPIC%)? For instance: '"%TOPIC%" istopic "%BASETOPIC%"'?

-- ArthurClemens - 17 May 2007

What's wrong with $summary!='' ?

      * Set SUMMARY=%<nop>IF{"$summary!=''" then="Summary like '%%URLPARAM{"summary"}%%' and "}%

Arthur, yes. You can simply write '$TOPIC=$BASETOPIC'.

e.g. %IF{"$TOPIC=$BASETOPIC" then="This is a base topic" else="This is not a base topic"}% yields "This is a base topic"

• Sorry, I've missed your last comment in Bugs:Item3620. No need for another check of TOPIC=BASETOPIC. -- ArthurClemens - 17 May 2007

-- CrawfordCurrie - 17 May 2007

Crawford, that was my first try. $summary!='' does not work if summary is not present in the query (undef ='' evals to true).

-- RafaelAlvarez - 17 May 2007

ok, then IF{"defined summary && $summary!=''"... or, shorter, IF{"$summary && $summary!=''"...

-- CrawfordCurrie - 18 May 2007

Is this present syntax or a proposal to a syntax extension?

-- ArthurClemens - 19 May 2007

Present syntax

-- CrawfordCurrie - 20 May 2007

Is everyone happy with Crawfords explanation (which means rejecting this) or do we still have a feature request for isempty?

At the moment undefined ""= is returning true. Which from a programming view is correct but will be a trap for most people. So instead of introducing a new also a bit odd feature (isempty) we can leave code as it is and document the =IF{"defined summary && $summary!=''"... = in the VarIF

-- KennethLavrsen - 21 May 2007

Rafael - are you satisfied with the explanation or do you want this proposal to do through?

It is very unclear to me if this is a needed change so I will stop the 14 day clock.

-- KennethLavrsen - 27 May 2007

OK. Assume the proposal is either withdrawn or rejected then since I see no support for it. I need to consolidate the 4.2.0 release and defer not yet accepted or implemented proposals to Georgetown. But I will not leave push too large a pile to Georgetown and this one seems like a dead fish.

I can be wrong. I base this on human intuition. If I am wrong please put it back in "UnderInvestigation" and we will process it.

-- KennethLavrsen - 28 May 2007

Let's see where we are: I'm proposing a new 'keyword' to check if a variable is empty (meaning undefined or empty string). It was proposed that instead of introducing a new keyword, document the 'idiom' to perform the check.

IMO, the idiom proposed is equivalent to have the same piece of code "copy&pasted" all over the code. I would prefer to abstract that into a keyword.

I'll put the three proposed syntax (the one with the isempty operator, the other two proposed in the topic as an alternative to be documented). If there is a "consensus" that the alternative scores lower in the nedometer, I'll let this proposal drop. In the meantime, I'll change this to "UnderInvestigation", and defer it to Georgetown.

* Set SUMMARY=%<nop>IF{"NOT isempty summary" then="Summary like '%%URLPARAM{"summary"}%%' and "}%

* Set SUMMARY=%<nop>IF{"defined summary && sumary!='' " then="Summary like '%%URLPARAM{"summary"}%%' and "}%

* Set SUMMARY=%<nop>IF{"$summary && $sumary!='' " then="Summary like '%%URLPARAM{"summary"}%%' and "}%
 

Sorry for the late feedback.

-- RafaelAlvarez - 29 May 2007

To me, the abstraction of repeated "is empty" code into an "is empty" keyword reduces (ever so slightly) the required nerdiness of the syntax and would be worthwhile.

-- KeithHelfrich - 29 May 2007

IMHO this was really a no-brainer and should not have gone through the release process. Overall the release process is working well, but this is an example where a concern blocked a small enhancement from getting into the FreetownRelease. For small enhancements like this I suggest developers to be bold and implement it (after the code freeze).

-- PeterThoeny - 30 May 2007

Feature creap is feature creap - no matter what scale. Enhancing syntax is serious business because once the troll is out of the box we are stuck with it. And the IF feature is important and may need enhancements in later releases then we'd better get things right when we introduce them.

This proposal did trigger concern so it is not a no-brainer that should just creap in.

-- KennethLavrsen - 03 Jun 2007

I think this needs to pass a release meeting to be decided on. If nothing else then to trigger a final debate. So setting to ReadyForReleaseMeeting. It will go on the agenda on the first release meeting after the release of 4.2.0.

-- KennethLavrsen - 02 Jan 2008

I don't have a problem with it. isempty(x) is exactly equivalent to (defined(x) || $x=''). As Peter says, it should have gone into Freetown (but not after a code freeze. You want to make changes, get the code unfrozen first, otherwise it makes a mockery of the process).

-- CrawfordCurrie - 04 Feb 2008

Accepted - all for - GeorgetownReleaseMeeting2008x02x04

-- KennethLavrsen - 04 Feb 2008

Oops.... didn't noticed that this one was accepted.

-- RafaelAlvarez - 08 Aug 2008

Tracked by Bugs:Item5916.

Commited on r17405

-- RafaelAlvarez - 11 Aug 2008

I added a little note to Bugs:Item5916. Take a look

-- GilmarSantosJr - 11 Aug 2008

Crawford suggested a small change to this feature: To make it available for both Query Searchs and the IF. If there are no objections to this, i will do it on this weekend.

Also, may I sneak this feature into TWiki 4.2.X?

-- RafaelAlvarez - 15 Aug 2008

It's already been accepted by the release meeting, so I don't see why not. But the 4.2.3 release team are the ultimate decision point.

-- CrawfordCurrie - 15 Aug 2008

No rule without exceptions.

As I see this - it is a well contained enhancement. It does not at all change the existing way IF works. It is a pure backwards compatible enhancement and it should be fairly easy to test fully.

There are also quite many unit tests to backup the existing IF feature. I think it is OK to add it to TWikiRelease04x02 branch for 4.2.3 provided it happens now. Ie before say - 20th of August to ensure enough test time before a possible september 4.2.3 release.

Note that the IF has been refactored - also in 4.2 branch so you may need to take extra care with the merge.

-- KennethLavrsen - 16 Aug 2008