FormIt icon indicating copy to clipboard operation
FormIt copied to clipboard

Update fihooks.class.php

Open Bournwog opened this issue 12 years ago • 10 comments

when I add 10-20 snippet calls in emailTpl, only 3 or 5 first calls will be processed

with this patch you can use correctly more than 5 snippets calls in emailTpl

Bournwog avatar Aug 31 '13 07:08 Bournwog

Thanks for this @Bournwog. Are there any specific tests that come to mind to make sure this doesn't introduce any new issues? I've got this merged into a testing branch over at the new repository location modxcms/FormIt https://github.com/modxcms/FormIt/commit/42f4808d83bc9246697dbae907ceb0695a7bc110

jpdevries avatar Oct 02 '13 05:10 jpdevries

I know the parser a bit.. It seems to be a right thing :-) saw that stuff before :+1:

bertoost avatar Oct 25 '13 13:10 bertoost

@Bournwog why not use $this->modx->parser instead of calling $this->modx->getParser() twice?

jpdevries avatar Feb 05 '14 09:02 jpdevries

What if the parser isn't loaded (theoretical it can be possible)? I also agree, the getParser() should check that and only init a new instance when not already is loaded.. changed like:

public function getParser() {
    $parserClass = $this->getOption('parser_class', null, 'modParser');
    if(isset($this->parser) && is_object($this->parser) && $this->parser instanceof $parserClass) {
        return $this->parser;
    }
    return $this->getService('parser', $parserClass, $this->getOption('parser_class_path', null, ''));
}

bertoost avatar Feb 05 '14 09:02 bertoost

@bertoost i'm talking about this in the PR

// parse all cacheable tags first
$this->modx->getParser()->processElementTags('', $str, true, false, '[[', ']]', array(), 10);
// parse all non-cacheable and remove unprocessed tags
$this->modx->getParser()->processElementTags('', $str, true, true, '[[', ']]', array(), 10);   

is there a problem with $this->modx->parser->processElementTags as it was before?

jpdevries avatar Feb 05 '14 09:02 jpdevries

I think I copied this from a place somewhere else :-) Don't know exactly.. it's been a while

bertoost avatar Feb 05 '14 09:02 bertoost

Also; when I look logical;

$this->modx->getParser();
$this->modx->parser->processElementTags('', $str, true, false, '[[', ']]', array(), 10);
$this->modx->parser->processElementTags('', $str, true, false, '[[', ']]', array(), 10);

Should be fine too..

bertoost avatar Feb 05 '14 09:02 bertoost

i guess i don't understand the need to call $this->modx->getParser(); since it wasn't doing so before. is it to ensure the parser is loaded?

jpdevries avatar Feb 05 '14 09:02 jpdevries

Yes indeed. As said.. Theorethical it's possible that it isn't loaded. Always good to call the load method. That said; I also think the getParser() method should be improved, to avoid double loadings

bertoost avatar Feb 05 '14 10:02 bertoost

+1 on getParser() only getting an instance if not already loaded or a different parser class is specified

opengeek avatar Feb 05 '14 15:02 opengeek