Ideas

[thunderer]

Just a list of issues to remember:

• [ ] custom exceptions for specific cases rather than using generic ones,
• [ ] recipes for specific tasks (like find all shortcode names in text),
• [ ] upgrade RegularParser to not split text into tokens at once but iterate over characters,
• [ ] shortcodes with children to represent their structure in text,
• [ ] shortcode's getTextUntilNext() which returns the content up to the next shortcode opening tag,
• [ ] shortcode name wildcard,
• [ ] more complex positional parameters [x=http://x.com], [x x=http://x.com],
• [ ] shortcode escaping by doubling opening / closing token [[x /]], [[y a=b]],
• [ ] tools for text analysis (nesting depth, count, name stats, etc.)
• [ ] configurable shortcode name validation rules,
• [ ] solve BBCode ambiguity issues in [x=] and [x= arg=val],
• [ ] add ProcessedShortcodeInterface and Processor::withShortcodeFactory() (think of a better name) to allow creating custom shortcode objects using ProcessorContext that are compatible with ProcessedShortcode. This will allow users to put their information inside while still maintaining how library works,
• [ ] fix inconsistency between getText and getShortcodeText between ParsedShortcode and ProcessedShortcode,
• [x] make ShortcodeFacade a mutable class with all the shortcuts to ease library usage (#36),
• [ ] new regex parser that catches only opening/closing tags and handles nesting manually,
• [ ] repeated shortcodes, ie. ability to repeat inner content with collections of data, [list]- [item/],[/list] which renders multiple "item" elements, children of list (item shortcodes) receive context from data passed to list,
• [x] better documentation than it is now in README (#34),
• [x] extract several event listener classes with __invoke() to ease events usage (#33),
• [x] find a way to strip content outside shortcodes in given shortcode content without losing tree structure (apply results event),
• [x] ~~configurable handler for producing Processor::process() return value using array of replacements~~ (events FTW),
• [x] issue with losing shortcode parent when many same shortcodes are on the same level,
• [x] ~~HUGE BC (just an idea, no worries): Change ProcessorInterface::process() to receive array of parsed shortcodes to allow greater flexibility (eg. filtering by parent)~~,
• [x] find a way to retrieve original shortcode content even when auto-processing,
• [x] prevent the ability to create shortcode with falsy name (require non-empty string value),
• [x] resolve naming problem for shortcode parameters (current) vs. arguments,
• [x] suggest symfony/yaml in composer.json for YAML serializer,
• [x] allow escaping with double open/close tags like in WordPress ([[code]value[/code]]),
• [x] add Wordpress parser (regex parser with code from WP).

Regular parser:

• [x] fix BBCode parsing (unused variable).

BBCode:

• [x] decide whether bbcode value should be:
  • merged with parameters as parameter with shortcode name, no API change, easier version,
  • a separate element, fourth parameter in Shortcode constructor, and separate getter,
  • a separate shortcode type (requires changes in both Parsed and Processed as well).

Built-in handlers:

• [x] ~~DeclareHandler should typehint interface in constructor,~~ (needs add() method)
• [x] EmailHandler could be a BBCode,
• [x] PlaceholderHandler should have configurable placeholder braces,
• [x] UrlHandler could be a BBCode,
• [x] WrapHandler could have several most common variants (eg. bold) created as named constructors.

[rhukster]

I have a few of ideas to add to the list:

1. Option to only process registered handlers.
   Related to the performance issues I have seen in RegularParser it might be good to have more checks to ensure that only registered handlers are processed. During my testing of a page in my documentation that was using square brackets to highlight method signatures, these were being treated as shortcodes.

   I think that either by default, or at the very least an option should be to ignore all shortcodes that don't explicitly match the registered handlers. This would improve performance dramatically and limit the amount of work the library has to do.

2. Direct control over nested tags. With regards to nested handlers, currently you can nest any shortcode. However there are situations where you a particular shortcode can only be nested inside another. In fact in many complex nested shortcodes, there are situations where the exact nesting of the shortcodes is important, and should only work if the correct nesting is found.

   A simple example of this is with tabs:

   [ui-tabs]
   [ui-tab title="Tab 1"]
   This is some content for Tab 2
   [/ui-tab]
   [ui-tab title="Tab 2"]
   This is some content for Tab 2
   [/ui-tab]
   [/ui-tabs]
   

   As you can clearly see, a ui-tab shortcode only makes sense inside a ui-tabs shortcode. Other shortcode libraries I have come across allow you to define which child/nested shortcodes should be processed by the parent shortcode. This could increase performance and increase reliability of nested shortcode setups.

3. Shortcode ID. This is something I ran into when developing my Tabs shortcode. Basically I needed to get a unique ID for each particular shortcode. To accomplish this I created an id based on the shortcode 'text':

   private function getShortcodeId($shortcode)
   {
       return substr(md5($shortcode->getShortcodeText()), -10);
   }
   

   This is not ideal, but serves my purposes. It would be really useful if an ID could be generated and assigned to each unique shortcode instance.

4. Shortcode getChildren() The ProcessedShortcode class provides a really useful getParent() method which I use extensively when working with nested shortcodes. This allows me to get at a parents parameters when working with children. However, it would be really useful to be able to get an array of child shortcodes from the parent. Currently I get around this by creating an array at the object level and assigning all children to a unique parent id. This way I can retreive the shortcodes directly from the parent:

    $this->handlers->add('ui-tabs', function(ShortcodeInterface $shortcode) {
   
         // Add assets
         $this->assets->add('js', ['jquery', 101]);
         $this->assets->add('js', 'plugin://shortcode-ui/js/ui-tabs.js');
         $this->assets->add('css', 'plugin://shortcode-ui/css/ui-tabs.css');
   
         $hash = $this->getShortcodeId($shortcode);
   
         $output = $this->grav['twig']->processTemplate('partials/ui-tabs.html.twig', [
             'hash' => $hash,
             'active' => $shortcode->getParameter('active', 0),
             'position' => $shortcode->getParameter('position', 'top-left'),
             'theme' => $shortcode->getParameter('theme', $this->grav['config']->get('plugins.shortcode-ui.theme.tabs', 'default')),
             'child_tabs' => $this->child_states[$hash],
         ]);
   
         return $output;
     });
   
     $this->handlers->add('ui-tab', function(ShortcodeInterface $shortcode) {
         // Add tab to tab state using parent tabs id
         $hash = $this->getShortcodeId($shortcode->getParent());
         $this->child_states[$hash][] = $shortcode;
         return;
     });
   

   This works but could be more elegant if i didn't have to have manage my own child_states array.

FYI Some of these ideas stem from the discussions in PR #26

[thunderer]

@rhukster these are all great ideas, let me explain how they could be achieved:

1. shortcodes without handlers are not processed (they are returned verbatim), but their content may be processed if Processor::withAutoProcessContent() is set to true. If you want to remove them then look at the branch runtime-events, there is an event FilterShortcodesEvent using which you could remove all matched shortcodes at given level without registered handler. I'm still working on the events subsystem, but you get the idea. There remains one question though: what to do with nested shortcodes with handlers?
2. as for direct control over nested shortcodes I'd suggest to invert the control and validate parent shortcode name in child, as you mentioned there is ProcessedShortcode::getParent() method which could come in handy. If the parent is invalid, just return ProcessedShortcode::getContent(). Would it solve your problem?
3. shortcode ID is a cool idea, I came to the conclusion that the only truly unique information about given shortcode is an offset from the beginning of the whole text. That's why I plan to introduce ProcessedShortcode::getBaseOffset() which will return the number representing an offset from the beginning of the whole text. What do you think?
4. getting children of given shortcode is as easy as creating a parser object and calling parse() method on the shortcode's content. I see that this could be a lot easier if I exposed the Processor::getParser() method so that you could write in handler (assuming that $s is an instance of ProcessedShortcode) that $children = $s->getProcessor()->getParser()->parse($s->getContent()); or even $children = $s->getChildren() that would encapsulate the behavior for you.

Please let me know what do you think.

[rhukster]

1. I think an event model would be handy to customize the processing rules as long as it was not overly complex or expensive in processing time. iv'e not looked at that branch, but I like the concept.

2. Your solution should solve that problem yes.

3. The getBaseOffset() would work as a unique id. So yes, that would do nicely.

4. Yes, getting access to the parser would be most helpful.

Thanks!

[thunderer]

@rhukster getBaseOffset() was introduced in changesets https://github.com/thunderer/Shortcode/compare/c87b3e97714dcf98ae9ae7a5a26af883b534e2a4...9f74c82d723529b0f2e3de7f7d22512efe5f6fe1 . Also you may be interested in new ShortcodeFacade (with README update) PR that was merged today. That should sum up all the ideas you came up with so far. :smile: I got no more ideas so v0.6.0 should be tagged after weekend.

[rhukster]

Yup, i saw you merged your facade updates. I'm going to take a look and probably update my Grav shortcode-core plugin to use it. I'll test the getBaseOffset() and with my recent changes it should be a good solution for getting a unique id of each shortcode.

Prior to recent updates, i was caching independently of the page, so each shortcode needed a unique id site-wide, which a simple offset would not provide. However, now, things are cached per-page, so should be good to go.

Thanks!

[dimayakovlev]

Can you add option for method getParameter to return parameter value as boolean?

What I mean:

public function getParameter($name, $default = null, $boolean = false)
{
  $value = $this->hasParameter($name) ? $this->parameters[$name] : $default;
  return $boolean ? filter_var($value, FILTER_VALIDATE_BOOLEAN) : $value;
}

[thunderer]

@dimayakovlev can you show me some use cases for this? Isn't casting to boolean sufficient?

$value = (bool)$s->getParameter('name');

[dimayakovlev]

It can be useful if parameter value is 'true' or 'false' (strings).

This is example from Grav Shortcode plugin: Safe-Email Address: [safe-email autolink="true" icon="envelope-o"][email protected][/safe-email].

It does not matter if autolink="true" or autolink="false", this part of code would be executed https://github.com/getgrav/grav-plugin-shortcode-core/blob/develop/shortcodes/SafeEmailShortcode.php#L29-L30.

Simple casting to boolean in this scenario does not work (no problems if parameter value can be only '1' or '0', but). So it will be easier if the method getParameter or other special method, can return Boolean, because strings false and true are often used as parameters values.

[thunderer]

@dimayakovlev Thanks, but in this case, I would rather prefer that you call filter_var() in your own shortcode handler. This leaves you in control and does not introduce unnecessary optional parameters in library code.

$value = filter_var($s->getParameter('name'), FILTER_VALIDATE_BOOLEAN);

[dimayakovlev]

@thunderer ok. :)

[jenstornell]

I have an idea. The first thing I noticed was that it quite much code, even for the most simple task, as we can see here:

use Thunder\Shortcode\ShortcodeFacade;
use Thunder\Shortcode\Shortcode\ShortcodeInterface;

$facade = new ShortcodeFacade();
$facade->addHandler('hello', function(ShortcodeInterface $s) {
    return sprintf('Hello, %s!', $s->getParameter('name'));
});

$text = '
    <div class="user">[hello name="Thomas"]</div>
    <p>Your shortcodes are very good, keep it up!</p>
    <div class="user">[hello name="Peter"]</div>
';
echo $facade->process($text);

Maybe for this case simplify it even more. Something like this?

shortcode::create('hello', function($s) {
  return sprintf('Hello, %s!', $s->get('name'));
});

$text = '
    <div class="user">[hello name="Thomas"]</div>
    <p>Your shortcodes are very good, keep it up!</p>
    <div class="user">[hello name="Peter"]</div>
';
echo shortcode::process($text);

[thunderer]

@jenstornell I already had that discussion back when I released first versions, you can read my answer here. The ShortcodeFacade class is already a compromise that provides sane defaults and allows the simplest possible usage while maintaining high software quality. Singletons and the global state itself can cause major issues with stability and testability of your implementation. Though, if you really want to build a singleton to wrap Shortcode then it is very easy to write something like this in your project:

<?php
declare(strict_types=1);
namespace X;

use Thunder\Shortcode\HandlerContainer\HandlerContainer;
use Thunder\Shortcode\Parser\RegularParser;
use Thunder\Shortcode\Processor\Processor;

final class ShortcodeSingleton
{
    /** @var HandlerContainer */
    private static $handlers;
    /** @var Processor */
    private static $processor;

    private static function instance(): Processor
    {
        if(null === static::$processor) {
            static::$handlers = new HandlerContainer();
            static::$processor = new Processor(new RegularParser(), static::$handlers);
        }
    }

    public static function add(string $name, callable $handler): void
    {
        static::instance();
        static::$handlers->add($name, $handler);
    }
    
    public static function process(string $text): string
    {
        static::instance();
        return static::$processor->process($text);
    }
}

[jenstornell]

I did something similar to my Kirby CMS plugin:

https://github.com/jenstornell/kirby-shortcode/blob/master/lib/shortcode.php

echo shortcode::parse($text);

But I understand your concerns with this approach. For my plugin it's probably the best but not for all other scenarios.