Config schema definitions for plugins aren't applied to their derivatives

Woah, omg. The way we resolved it earlier is we introduced a key with the value we needed in the schema in the name of not complicating the schema format further. It was supposed to be an easy to grok and simple format.

The way we resolved it earlier is we introduced a key with the value we needed in the schema in the name of not complicating the schema format further.

agreed, but we didn't manage to get ride of "budle:type" format in config schema and may be we need something like like 'type_regex' to solve it?

I think (and HOPE :P) the patch in #1 contains another patch? :)

Well, I think the dynamic typing is pretty confusing already, would love not to make it even more confusing :/

The fix worked see #474004-113: Add options to system menu block so primary and secondary menus can be blocks rather than variables

This is probably beta-blocking (per discussion with @effulgentsia and @alexpott). We might settle on a solution that isn't but we're not there yet.

I don't think this will fix the installation issue, but in reviewing this to try to find the cause of the failure I found a typo in the spelling of the word definition in several variable names. Small fix with a correction for that.

Wondering whether that is not possible to do with type-inheritance as usual? I've never really written config schema, so it might be nonsense, but I'd have thought of defining something like:

system_menu_block*:
  type: system_menu_block

So the derivative plugin would have to add that as well. Any reasons that does not work?

I am glad that this finally got a beta blocker, as this block proper schema support for views since ever.

+++ b/core/lib/Drupal/Core/Config/TypedConfigManager.php
@@ -251,13 +265,23 @@ protected function replaceVariable($value, $data) {
+        if ($schema !== NULL) {
+          // If the schema for the value has a schema type resolver regular
+          // expression then use it to manipulate the value.
+          $definition = $schema->get($name)->getDataDefinition()->toArray();
+          if (isset($definition['schema_type_resolver'])) {
+            if (preg_match($definition['schema_type_resolver'], (string)$data[$name], $matches)) {
+              return $matches[0];
+            }
+          }
+        }
         return (string)$data[$name];
...
       else {

The problem with that approach is that it doesn't support the usecase that you might have a overridden plugin class for a specific derivative instance. For example the CommentRow plugin extends the EntityRow one with additional settings. We need to be sure that the specific one is loaded as well. The idea in #17 would solve that if I understand it correctly.

So TypedConfigManager::getFallbackName() resolves stars only following dots. In this case, the variable part of the element is after a colon not a dot. I would also prefer a simpler solution for this. The regex is totally black magic, the whole dynamic typing situation is already complex to understand. What about making stars work in general, not just after dots? That sounds like would be way simpler than regexing...

Retitling for clarity. I like the approach in #20, but given all the reviewers who commented in the earlier comments on prior approaches, let's get a couple more +1s on this before RTBC'ing it.

+1 for #20 as we have same pattern (.*) support already.

I guess this means this supports derivatives but not derivatives with different separators. I'm ok with that, since its pretty hard to implement and I don't really see that being used.

Yay, this implementation looks much better than that complicated regex stuff. I only have relatively minor comments but important for making this easy to understand. Otherwise looks good.

1. +++ b/core/lib/Drupal/Core/Config/TypedConfigManager.php
   @@ -155,7 +159,7 @@ public function clearCachedDefinitions() {
       *   Same name with the last part(s) replaced by the filesystem marker.
       *   for example, breakpoint.breakpoint.module.toolbar.narrow check for
   -   *   definition in below order:
   +   *   definitions in the following order:
   
   @@ -163,12 +167,20 @@ public function clearCachedDefinitions() {
       *     breakpoint.breakpoint.*
   ...
   +   *   Colons are also used, for example,
   +   *   block.settings.system_menu_block:footer check will for definitions in the
   +   *   following order:
   

   English should be unified on these. "check will for" definitely does not work. :D The first uses "check for definitions" which should probably also be "will check for"?

2. +++ b/core/modules/config/src/Tests/ConfigSchemaTest.php
   @@ -397,4 +397,35 @@ function testSchemaFallback() {
   +   * Tests use of schema_type_resolver to determine schema..
   +   */
   +  function testSchemaTypeResolver() {
   

   The comment and method name do not match the current implementation anymore.

Will need an issue summary update and a change notice. I'll do a draft change notice now. Needs work for the code changes needed.

Re #23: I'm not happy with removing the colon from the schema key wildcard pattern. For example, I wouldn't want image.effect.image_scale* being a fallback for image.effect.image_scale_and_crop. However, turns out that the YAML spec does allow colons to appear in unquoted keys, so long as they're not followed by whitespace, and Symfony's YAML parser respects that spec. So test.plugin_types.boolean:*: works as a line of YAML.

Re #24: Let's not hold this issue up on it, but I opened #2319503: Should TypedConfigManager::getFallbackName() support plugin derivatives whose IDs are not delimited with a colon? so we can discuss it further.

Fix minor syntax glitch in issue summary. Added change notice at https://www.drupal.org/node/2319739

The updated patch looks good to me :) I'm *really* glad we did not (need to) go with the regex solution.

Glad we sorted this one out after all. Committed to 8.0.x. Thank you!

Superb, thanks!

The regular expression here could be seriously simplified because you do not need to escape anything in a PCRE character class except the closing square bracket.

turns out that the YAML spec does allow colons to appear in unquoted keys

If anyone wonder about the above statement, please refer: http://www.yaml.org/spec/1.2/spec.html#id2788859 and raised an issue with phpStorm(http://youtrack.jetbrains.com/issue/WI-24607) as it doesn't support this atm.