better set_list branch

[GDmac]

Mark, the version in better set_list branch is not storing/returning one empty fields. looks like the regex doesn't grab empty stash:varname tags.

// current
 preg_match_all('#'.LD.'(stash:[a-z0-9-_]+)'.RD.'.+?'.LD.'\/\g{1}'.RD.'#ims', $this->EE->TMPL->tagdata, $matches);
//test fix
 preg_match_all('#'.LD.'(stash:[a-z0-9-_]+)'.RD.'(.+)?'.LD.'\/\g{1}'.RD.'#ims', $this->EE->TMPL->tagdata, $matches);

[croxton]

OK, thanks, your fix should work - will test.

[croxton]

Not sure if this is the problem you were seeing, but I found that when a list row was missing a variable pair (not an empty value, but the pair that wraps it) then the list was not setting a NULL value. This is changed in the latest commit, please check it out

https://github.com/croxton/Stash/tree/better_set_list

Example:

  {exp:stash:set_list name="list_name"}

    {!-- row 1 --}
    {stash:title}Item 1{/stash:title}
    {stash:desc}Description 1{/stash:desc}

    {!-- row 2 --}
    {stash:title}Item 2{/stash:title}
    {stash:desc}Description 2{/stash:desc}

    {!-- row 3: missing variable pair --}
    {stash:title}Item 3{/stash:title}

    {!-- row 4: empty variable pair --}
    {stash:title}Item 4{/stash:title}
    {stash:desc}{/stash:desc}

    {!-- row 5 --}
    {stash:title}Item 5{/stash:title}
    {stash:desc}Description 5{/stash:desc}

  {/exp:stash:set_list}

The specially formatted 'flattened' string is then SET correctly as:

  title|=|Item 1|&|desc|=|Description 1|+|title|=|Item 2|&|desc|=|Description 2|+|title|=|Item 3|&|desc|=|__NULL__|+|title|=|Item 4|&|desc|=|__NULL__|+|title|=|Item 5|&|desc|=|Description 5

[GDmac]

First tests seem great.

Another (unrelated) smallish issue: As mentioned on twitter, i'm hooking on the channel query_result, and am just passing the data-array to TMPL->parse_variables. (0.46)secs. Then Stash:set_list takes another (0.2)secs. to parse and save. If i only retrieve (replace="no") with get_list, the whole page takes (0.16)secs.

However, if i set parse_depth="2" then it seems that stash is going over the entire string again, taking (4.2)secs. again. There are no exp:tags left to parse, e.g. only {stash:title}Item{/stash:title} tags. I'm unsure what is going on. But the extra _parse_sub_template() seems to have a hard time wading thru the big string another time.

Again, thanks for the heads up. (0.16)secs. get is nice, and (0.7)secs. is totally workable for saving a fresh list.

[croxton]

Why not just set a Stash list directly from your extension? No need to parse strings at all, just create the list from the query result array. https://github.com/croxton/Stash/wiki/Using-stash-methods-in-your-own-add-ons

I guess I could add a check for { at the start of the _parse_template_vars() function before variable replacements? Stash already checks for {exp: prior to tag parsing.

[GDmac]

I will try that, however for the moment, during dev, not all needed custom fields and names are settled yet. (passing directly to stash means, it has to keep track of the field_id and names in the extension). At the moment the extension is rather generic, it just tests for a custom parameter on the channel:entries tag, replaces field_id_x with fieldnames from channel->cfields[..] and then bypasses the entries_parser with a simple TMPL->parse_variables().

The extra 4 seconds for parse_depth="2" comes (again) from the assign_variables() inside _parse_template_variables(). Note! this is with the break-patch, 20 secs. otherwise.

[GDmac]

I'm unsure but maybe something like this to fetch singles? http://regex101.com/r/yM2oG3/1 (i noticed that if you remove the closing } from stash:key it still matches, but i doubt that assign_variables can catch those as well

[croxton]

Looks good to me. Have you tested on a really big string? Just wondering how much memory that negative lookahead would use.

[GDmac]

hmmm, that regex only cuts the 4 secs. in half.

        if (count($this->EE->session->cache['stash']) > 0 && preg_match_all("#{(stash:\w+)}(?!.*?{/\g{1}})#ims", $template, $matches))
        {   
            // We want to replace single stash tag not tag pairs such as {stash:var}whatever{/stash:var}
            // because these are used by stash::set() method when capturing multiple variables.
            // So we'll calculate the intersecting keys of existing stash vars and single tags in the template 
            $tag_vars = array_unique($matches[1]);

edit: also any date formatting in get_list almost doubles output time again (from .18 to .31), so, doing as much preprocessing as possible.

[croxton]

We only need unique occurrences of each key, so this should be more efficient I think:

{(stash:[a-z0-9\-_]+)}(?!.+\1)

[croxton]

Date formatting more or less follows what the template class does. Arguably though we should only process those variables once, since it is formatting information. Could be done after 'the loop' of template tag parsing.

[GDmac]

Nice, that latter regex 0.8 secs (parse_depth=2). one more assign_variables left in set() method :-)

[croxton]

Yeah that's a tricky one. Has to support nested pairs too!

[GDmac]

it's not being called in this instance e.g. if !! $name

[croxton]

Hold on.

In set_list(), when using

 // get stash variable pairs (note: picks up outer pairs, nested pairs and singles are ignored)
 preg_match_all('#'.LD.'(stash:[a-z0-9-_]+)'.RD.'.+?'.LD.'/\g{1}'.RD.'#ims', $this->EE->TMPL->tagdata, $matches);

 if (isset($matches[1]))
 {
     $this->EE->TMPL->var_pair = array_flip(array_unique($matches[1]));
 }

Instead of

        $tag_vars = $this->EE->functions->assign_variables($this->EE->TMPL->tagdata);
        $this->EE->TMPL->var_pair = $tag_vars['var_pair'];

This fails to capture the map_.. pairs:

{exp:stash:set_list name="list_name"}

    {stash:image_thumb}{/stash:image_thumb} 
    {stash:map_southwest}27.98, -24.54 {/stash:map_southwest} 
    {stash:map_northeast} 71.11, 44.81{/stash:map_northeast} 

    {stash:image_thumb}{/stash:image_thumb} 
    {stash:map_southwest}{/stash:map_southwest} 
    {stash:map_northeast}{/stash:map_northeast} 

{/exp:stash:set_list}

Thoughts?

[croxton]

Ah, easy, + needs to match something so use *.

  {(stash:[a-z0-9-_]+)}.*?{/\g{1}}

[GDmac]

see original top comment, .*? is ok. i think (.+)?, as originally commented, also makes it optional, no?.

[croxton]

Uh, of course sorry! Latest changes have been implemented in latest commit to the better_set_list branch.

[croxton]

Found a nasty bug in that regex:

{(stash:[a-z0-9\-_]+)}(?!.+\1)

Doesn't match {stash:channel} when you also have {stash:channel_id}, i.e. when variables share the same text at the start of the string. Ironically, EE had a similar bug for years until recently.

Anyway, fix is:

{(stash:[a-z0-9\-_]+)}(?!.+\1})

[GDmac]

without a good test suite ee and it's stuff becomes almost unmaintainable. good catch.