Configuration

[colinodell]

Parsers and renderers will need to be configurable at some point. For example, some users may want to whitelist certain HTML elements as "safe", ensuring that others get escaped. If we were to allow such configurations now, the user would need to manually build their Environment and set the options on each individual parser/renderer:

$environment = new Environment();
$linkParser = new LinkParser(['defaultScheme' => 'https']);
$htmlParser = new HtmlParser(['allowedTags' => ['a', 'strong', 'p', 'dl', 'dt', 'dd']]);
// ... Repeat for every single parser/renderer

// Also repeat this for every new parser/renderer you add
$environment->addInlineParser($linkParser);
// ... etc

It would be much easier if the user could obtain the pre-built Environment and simply pass all their configurations in at once. Perhaps it would look like this:

$environment = Environment::createCommonMarkEnvironment([
    'links' => [
        'defaultScheme' => 'https',
    ],
    'html' => [
        'allowedTags' => ['a', 'strong', 'dl', /*...*/ ]
    ],
]);
$docParser = new DocParser($environment);
$htmlRenderer = new HtmlRenderer($environment);

So the questions I have are:

1. Is this a good idea?
2. How do the parsers/renderers define which options they support?
3. How do the config values get set on the parsers/renderers?
4. Do we use something like symfony/options-resolver, simple arrays, or roll our own solution?
5. Should users be able to change the configuration on-the-fly, or is it an immutable private/protected variable than cannot be changed once the Environment is created?

[aleemb]

1. Something simple would be nice.

2. Developers can dump the environment to see what's supported or go to the docs. You can also enforce getOptions via the Renderer interface making it easier to var_dump the default options array for a renderer as $imageRenderer->getOptions().

3. Not sure but I do like the idea of a one true source for all configuration, it makes things so easy (I love $_SERVER). Something like $environment['renderers']['image'] or $environment->getRenderers() which returns the a collection of Renderer objects that can be manipulated. Not sure though, depends on your internals.

4. For the configuration manipulation my personal preference is nested-array merges that I have been using for quite some time without issue:

   // $defaultEnvironment = ["allowsTags" => "b", "i", "img" => ["alt", "src"]];
   $myEnvironment = ["allowedTags" => ["img" => "class"]];
   $environment->merge($myEnvironment);
   // ["allowsTags" => "b", "i", "img" => ["alt", "src", "class"]];
   

   I have only needed additive operations so haven't thought about removals but maybe something like:

   // $defaultEnvironment = ["allowsTags" => "b", "i", "img" => ["alt", "src"]];
   $environment->unset(["allowedTags" => ["img" => "alt"]]);
   // ["allowsTags" => "b", "i", "img" => ["src"]];
   $environment->unset(["allowedTags" => ["img"]]);
   // ["allowsTags" => "b", "i"];
   

   I prefer this for personal use because other than merge() and unset() you are dealing with just an array. Helpers can be added as needed. You could even directly expose the $environment array variable and let the users do whatever they want. Seriously. That's the norm for Javascript options and works great. At the end of the day, it's just an array so you can't go wrong. push, merge, splice, ..., it's all there.

5. Why not? Users should have full control over the environment.

[colinodell]

Thanks for the feedback! After considering this for the past week, I've come to these conclusions:

1. Configuration options are necessary.
2. Parsers/renders won't specify their required options, as this is more complex than it really needs to be right now. This can be revisited later if needed.
3. Options will be set as a simple array on the Environment. Parsers/renders will be able to access the full array to look at whatever options they need.
4. We'll use a simply config array. Parsers/renders can use nested array merges to combine the given config with their defaults.
5. The config would ideally become "locked" once extensions have been initialized, though I may not enforce this in the next release.

[colinodell]

I have implemented a possible solution in PR #56. I'd greatly appreciate any feedback.