Feature Proposal: Add ability to have elsif sections

Motivation

The IF macro is inconvenient for building if / else ladders.

Description and Documentation

The current %IF macro only allows for one conditional check. If more than one is needed, it would have to be done with a nested %IF:

%IF{"condition1"
    then="result1"
    else="$percentIF{\"condition2\"
        then=\"result2\"
        else=\"result3\"}$percent"
}%

Add elsif attribute to IF

%IF{"condition1"
    then="result1"
    elsif="condition2"
    then="result2"
    elsif="condition3"
    then="result3"
    else="result4"
}%

This will require changing Foswiki::Attrs to be able to parse multiple attributes with the same key.

Enhance Foswiki::Attrs

Implement the changes described in SettingAndGettingVariablesUsingMacros to add a new accessor to Foswiki::Attrs called get(). This will use wantarray - if the caller requests an array, get($key) will return a list of the values for $key. If the caller requests a scalar, then get($key) will return the last value matching $key - consistent with current Attrs parsing behavior.

Examples

See above.

Impact

%WHATDOESITAFFECT%
edit

Implementation

Change the constructor for Foswiki::Attrs to store list values in the hashmap, in a new internal attribute called _LIST. This will be a flat list of key/value pairs (with keys stored in even positions, and values stored in odd ones).

%MACRO{foo="bar" foo="baz" aaa="bbb"}%

Will result in this perl data structure:
{
    aaa => 'bbb',
    foo => 'baz',
    _LIST => [ foo => 'bar', foo => 'baz', aaa => 'bbb' ]
}

  • get() will return _LIST verbatim, when called with no arguments.
  • get($key) will scan _LIST, and return the corresponding values (if called in list context). If called in scalar context, it will return the scalar value from the hash.

-- Contributors: KipLubliner - 17 Feb 2012

References

Discussion

Looking good; but what happened to the elsif sections?

-- CrawfordCurrie - 17 Feb 2012

I mentioned on IRC that Foswiki::Attrs is a hotspot, only because I consistently see it there in NYTProf.

Apart from %MACRO decoding, it also decodes each embedded %META:FOO datum (see Foswiki::Meta::setEmbeddedStoreForm).

You see this more when you're SEARCHing over thousands of topics; you won't see Foswiki::Attrs demand too much time on simple page views.

-- PaulHarvey - 17 Feb 2012

Crawford: I'm not sure what you mean. Nobody expressed interest in the more complicated syntax that I proposed in the brainstorming topic.
  • Sorry, I wasn't paying attention; I thought you were finished after the attrs parsing extension. of course you have the hard bit - extending the %IF macro - still to come. - C.

Paul: For the other place where Foswiki::Attrs is created, we could change the constructor - add an additional arg to suppress the creation of _LIST.

-- KipLubliner - 18 Feb 2012

Can we have it off by default? Would be nice to avoid adding another cut (Foswiki's performance suffers a death of a thousand cuts smile
  • Yes - some macros already enable what's called "friendly syntax" based on what the macro is. So the concept of enabling macro-specific syntax extensions is already there. - C.
Not to mention, it might be easier to identify & maintain compatibility with legacy code if we selectively request the new functionality only where it's needed.

-- PaulHarvey - 19 Feb 2012

Paul:

setEmbeddedStoreForm calls
_readMETA calls
_readKeyValues which directly parses the contents of { ... }.

I confirmed this in both trunk in 1.1. Are you sure that Foswiki::Attrs is in the critical path, or am I misunderstanding something? I just want to understand where the slowdowns are (and hopefully, identify an area that will be useful for performance testing).

If we want to selectively request the new functionality, here's one way that we can proceed: Extend the $friendly parameter to take an 'enum'. We will define constants in the class $SYNTAX_FRIENDLY = 1 and $SYNTAX_LIST = 2, and change the c'tor to set _LIST if one of these two options is selected. (Note that %IF already uses friendly syntax). We could also interpret it as a bitmask, so that way we could have more room for evolution.

-- KipLubliner - 22 Feb 2012

You are correct! Foswiki::Attrs is not used by meta. I must be thinking of a profile a really large nested search which resulted in many hundreds of macros. I'll try to dig up the scenario again.

From Foswiki::Func:
Foswiki::Func::registerTagHandler( $var, \&fn, $syntax )

  • $syntax can be 'classic' (the default) or 'context-free'. (context-free may be removed in future) 'classic' syntax is appropriate where you want the variable to support classic syntax

-- PaulHarvey - 23 Feb 2012

We could change the public API thusly:

Foswiki::Func::registerTagHandler( $var, \&fn [, list => 1] [, context-free => 1 ] )

  • After the second argument, the remaining arguments will be interpreted as key/value pairs. We can detect usage of the original API by seeing if there are exactly three arguments.

Should the documentation for registerTagHandler continue to include the context-free option? Or is it deprecated?

-- KipLubliner - 23 Feb 2012

I'm afraid I probably lack the experience necessary to make a call there. I feel that we should leave it in if there's no effort required and does no harm.

-- PaulHarvey - 23 Feb 2012

This is a nice one. I'd love to see Foswiki::Attr keep params of the same name in a list, as well as having elsif in %IF. Would be cool to see this in 1.2. Rock it.

-- MichaelDaum - 24 Apr 2013

[07:42] <kip3f> I can work on  http://foswiki.org/Development/EnhanceIfStatementsAndAttrsParsing  but I thought that it would be wisest to wait until recent 1.2 changes have stabilized (e.g. configure)
[08:15] <MichaelDaum> kip3f, no just go ahead. Your intended changes aren't really related to configure.

Are they?

-- MichaelDaum - 25 Apr 2013

As Michael says, go ahead. But note the proposal above implies a mojor change to the way attributes are parsed and handled. At the moment attribute names are unique and unordered; the proposal requires strict ordering such that succeeding elsif clauses cascade. I don't know how you're going to solve this. Some generic solution that applies a partial ordering on attributes without changing existing APIs, I guess.

-- CrawfordCurrie - 25 Apr 2013

I can see the the point of this proposal, I really can. However, there may be ways to satisfy the same goals without the risk of affecting backwards compatibility and possibly also without hurting existing performance:

(1) Instead of modelling this on an "if...elsif...elsif...else" approach, why not consider something like a "switch...case" model, especially something flexible like perl's newish Switch module. This would involve leaving %IF{}% alone and adding (for example) a %SWITCH{}% macro.

(2) Regardless of which model is better ("if...elsif...elsif...else" or "switch...case"), you could avoid having to overhaul Foswiki::Attr if you used unique attribute names. For example:
%IF{"condition1"
    then="result1"
    elsif2="condition2"
    then2="result2"
    elsif3="condition3"
    then3="result3"
    else="result4"
}%

-- MichaelTempest - 26 Apr 2013

Yes, enumerating the different branches makes things a lot easier. However I am pretty sure that keeping an ordered list of all params doesn't hurt much. The parser already iterates over all params anyway keeping the last ones of the same name while throwing away the previous.

if-elsif-else as well as switch-case are two different things. Yet a lot of programmers use the first when the second would be more efficient testing different values of the same query:

%SWITCH{"query"

   case="result"
   then="body"

   case="result"
   then="body"

   default="body"
}%

Would be cool to have that as well.

-- MichaelDaum - 26 Apr 2013

At the moment a Foswiki::Attrs object behaves as a simple perl hash. To keep an ordered list of parameters without changing that behaviour is not rocket science, but requires some careful thought to avoid disrupting users of the current implementation, and avoid adding extra cycles into the parsing/access process, which is one of the most critical in Foswiki.

The most obvious - and probably simplest - would be to reserve an additional key in the top level hash (keys like _RAW and _DEFAULT are already reserved) and simply record a list of tuples. Most users of Foswiki::Attrs would simply ignore this list; specialist uses like %IF and %SWITCH could use an extended API, for example:
my $attrs = Foswiki::Attrs->new($params);
my $iter = $attrs->keyValuePairs() ;
while ($iter->hasNext()) {
    my ( $key, $value ) = @{$iter->next());

    if ( $key eq 'elsif') {
        ...
    elsif ( $key eq 'then') {

-- CrawfordCurrie - 27 Apr 2013

Crawford - that's exactly what I have in mind. With the new behavior to only be used if the macro explicitly asks for it (ala "friendly" syntax). I'm not sure how this should be exposed, and would like feedback from you about whether it would be more appropriate to add the behavior to the friendly syntax, or to change registerTagHandler? Something like:

Foswiki::Func::registerTagHandler( $var, \&fn [, list => 1] [, context-free => 1 ] )

  • After the second argument, the remaining arguments will be interpreted as key/value pairs. We can detect usage of the original API by seeing if there are exactly three arguments.

-- KipLubliner - 12 May 2013

The Foswiki::Attrs class has got value in its own. It is a general purpose class to parse the key-value parameters as being used in macros. But they are not strictly bound to macros. So the behavior of the Foswiki::Attrs class shoudn't better not be parametrized via a macro registration call. Macro implementations can decide on their own how to make use of parameters without the TML parser knowing about it, as far as I can see.

I think a better way of thinking - and that's what I read out of Crawford's reply as well - is to have an extra api that is able to build up an iterator over all key-value parameters, based on an additional data model where these key-value pairs are stored in a bit different way as they are done now, so that duplicate keys aren't thrown away.

-- MichaelDaum - 13 May 2013

Agree with Michael. Rather than adding more parameters (and complexity) to the constructor I'd be inclined to allow Foswiki::Attrs to always deliver an ordered list. I just ran an experiment and it makes very little difference to performance of the parser.

Frankly I'd quite like to get rid of the "friendly" distinction. It's currently used for %IF, but it doesn't need to be. It is used in a couple of plugins, but for no useful reason that I can see.

BTW you need to be careful about what happens when someone does this:
my $x = Foswiki::Attrs->new('a="1" b="2"');
$x->{c} = 3;
print $x-stringify();
At the moment, c will be a fully fledged member of $x - but where does it appear in the order? Does it replace any pre-existing value of c? If so, what if there's more than one?

-- CrawfordCurrie - 13 May 2013

How about this:

my $x = Foswiki::Attrs->new('a="1" b="2"');
$x->{c} = 3;
After which, the internal hash is:
{
    a => '1',
    b => '2',
    c => '3',
    _LIST => [ a => '1', b => '2' ]
}
We can change stringify() so that it relies on the new method keyValuePairs().
sub keyValuePairs {
# first, get everything from _LIST
my %lastValueSeen;
while (($key,$val) = next_LIST_pair()) {
    # add them to output iterator
    $lastValueSeen{$key} = $val;
}
# and now the remaining elements
foreach $key ( sort keys %$this ) {
    if ($this->{$key} ne $lastValueSeen{$key}) {
        # add them to output iterator
    }
}

This will add new & changed elements to the end. This will keep the important invariant:
$anAttrs2 = Foswiki::Attrs->new($anAttrs->stringify())
$anAttrs and $anAttrs2 will have the same hash table values.

We will need more methods that give the ability to change any element anywhere. Perhaps like this:
my $x = Foswiki::Attrs->new('a="1" b="2" a="3" a="4"');
my $list = $x->getListValues('a');  # returns ("1", "3", "4")
$x->setListValue('a', 1, 'three');
my $list2 = $x->getListValues('a');  # returns ("1", "three", "4")

-- KipLubliner - 13 May 2013

I was thinking along the same lines though I'd prefer to access the values through an iterator - the relationship between different-named attributes may be important.

Another use-case to consider:
my $x = Foswiki::Attrs->new('a="1" b="2"');
$x->{b} = 3;
$x->stringify() => a="1" b="....hmmm....";
I'd vote for $x->{b}=3 blowing away all existing values of b, and replacing with the single new value.

-- CrawfordCurrie - 13 May 2013

Are you thinking about TIEing the hash? I don't like that idea at all, I think that it could slow down rendering.

sub keyValuePairs {
    my %isLastValueConsistent;
    # 1 -> the last value in $this->{_LIST} for the given key is consistent with $this
    # -1 -> the last value in $this->{_LIST} for the given key is NOT consistent with $this
    # not defined -> haven't processed the key yet

    # first, get everything from _LIST ... backwards
    while (($key,$val) = next_LIST_pair_BACKWARDS()) {
        my $ilv = $isLastValueConsistent{$key};
        if ($ilv == 1) {
            # UNSHIFT them onto the output iterator
        } elsif (! defined $ilv) {
            if (exists $this->{$key}) {
                $isLastValueConsistent{$key} = ($this->{$key} eq $val) ? 1 : -1;
                # UNSHIFT them onto the output iterator
            } else {
                $isLastValueConsistent{$key} = -1;
            }
        }
    }

    # and now the remaining elements
    foreach $key ( sort keys %$this ) {
        if (! exists $isLastValueConsistent{$key}) {
            # PUSH them to output iterator
        }
    }
}

my $iter = $attrs->keyValuePairs() ;
while ($iter->hasNext()) {
    my ( $key, $value ) = @{$iter->next()};
}

sub keyValueRefs {
# just like the above except returns refs
}


my $iter = $attrs->keyValueRefs() ;
while ($iter->hasNext()) {
    my ( $keyRef, $valueRef ) = @{$iter->next()};
    $$valueRef = 'ZZZ';
}

-- KipLubliner - 15 May 2013

No, I'm trying to express requirements without constraining your implementation. A tie would be one way to do what I described, though there are others (such as recording a signature/checksum for the original params and detecting a change in stringify that way).

-- CrawfordCurrie - 15 May 2013

Kip, Any progress on this. I'm going to defer it to 2.0, as there is not task and nothing has been checked in.

-- GeorgeClark - 03 Apr 2014

Changing to Parked. No progress in 3 years, and Developer appears to have left.

-- GeorgeClark - 19 Nov 2015
 
Topic revision: r27 - 19 Nov 2015, GeorgeClark
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy