Convert hook_config_translation_group_info to plugin

AFAIS there is no good way to let the information altered outside of an alter hook, so I think we should keep at least an alter hook, but the main definition would indeed be more standard as D8 plugins. See eg. https://api.drupal.org/api/drupal/core%21includes%21entity.api.php/funct... for altering information that comes from entity class annotations.

Good stuff!

1. +++ b/lib/Drupal/config_translation/Annotation/ConfigEntityMapper.php
   @@ -2,18 +2,20 @@
   + * @Annotation
   

   Is this all it takes to make the ConfigEntityMapper available as an annotation? Wow!

2. +++ b/lib/Drupal/config_translation/Plugin/ConfigEntityMapper/Block.php
   @@ -0,0 +1,23 @@
   + * Provides a filter to convert URLs into links.
   
   +++ b/lib/Drupal/config_translation/Plugin/ConfigEntityMapper/FilterFormat.php
   @@ -0,0 +1,23 @@
   + * Provides a filter to convert URLs into links.
   

   filter?

3. +++ b/lib/Drupal/config_translation/Plugin/ConfigEntityMapper/FilterFormat.php
   @@ -0,0 +1,23 @@
   + * Contains \Drupal\filter\Plugin\ConfigEntityMapper\Block.
   

   not block :)

Talked to EclipseGC about this. Summary:

- we better move this to a .yml file based discovery like #2065571: Add YAML Plugin discovery, tabs are going there too (#2050919: Replace local task plugin discovery with YamlDiscovery)
- add a manager then

The YAML based discovery is RTBC and close to commit.

Looking at what you want to do here, you've kind of conflated the annotation and the plugin class. This was an easy mistake to make since you need to pass everything through the same class ConfigEntityMapper, but it's actually your plugin class, not your Annotation. In cases like these (where 99% of the plugin implementations go through the same class) using a discovery that is not class based makes better sense. I noticed you don't have a plugin manager yet, so I went to the effort of building a basic one that should get you close to working.

/**
 * @file
 * Contains \Drupal\config_translation\ConfigEntityMapperManager. 
 */

namespace Drupal\config_translation;

use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\Language\LanguageManager;
use Drupal\Core\Plugin\DefaultPluginManager;
use Drupal\Core\Plugin\Discovery\YamlDiscovery;

class ConfigEntityMapperManager extends DefaultPluginManager {

  protected $defaults = array(
    'class' => 'Drupal\config_translation\ConfigEntityMapper',
  );

  public function __construct(CacheBackendInterface $cache_backend, LanguageManager $language_manager, ModuleHandlerInterface $module_handler) {
    $directories = array(); // Look at how to build up a real array of directories.
    $this->discovery = new YamlDiscovery('entity.config.mapper', $directories);
    $this->discovery = new ContainerDerivativeDiscoveryDecorator($this->discovery);
    $this->factory = new ContainerFactory($this);
    $this->alterInfo($module_handler, 'config_translation_group');
    $this->setCacheBackend($cache_backend, $language_manager, 'config_translation_group');
  }

  public function getGroups() {
    $groups = array();
    foreach($this->getDefinitions() as $definition) {
      $groups[$definition['basePath']] = $definition;
    }
    // You may want an additional alter/cache implementation here if necessary.
    return $groups;
  }
}

Eclipse

Yay #2065571: Add YAML Plugin discovery is in, so we can do this with .yml files now :)

@EclipseGC thanks for the starting manager code.

Here is the code from @EclipseGC in patch form, plus I added the directory discovery lifted from Drupal\layout\Plugin\Type\LayoutManager. This may not be the very latest best practice, but list_themes() has no nice OO equivalent that I know for example. Not sure if this even works, I did not actually try :D

Also this only works for entity mappers obviously. Should we have yet another manager for the non-entity mapping? Not sure how will this end up with good DX... I think ideally we'd have one .yml file structure with possibly entity and config file mapping.

Let's not get too hung up on that issue. I'll wade in there and see if I can make sense of the situation. :-D

Eclipse

Great, I committed this first step. I think it would be great to explore how to further improve this. I think its not very nice that we have a hook and a .yml file to say the least :D Eg. rename the *.entity.config.yml to something more generic and include the simple .yml mappings as well (eg. add a key to the data say 'type: entity' or 'type: config'. Then the dynamic entity based field mappings can go to an alter hook, which we would still keep. Moving to active since the patch was committed but still stuff to be done. Thanks all!

Here is a proof of concept to add a type key to this .yml (renamed it to *.configmap.yml to signify the mode general purpose) and add name based maps as well.

Need to update the service map.

Of course need to update the route subscriber as well.

Test fails show the menu types are not getting over properly :)

- Removed the old groups hook and support for gathering data with that
- Renamed the file (again) to *.config_translation.yml as per discussion with @vijaycs85 on IRC
- The alter hook is config_translation_info_alter(), IMHO makes lots of sense (trying to get rid of the one-off groups terminology as much as possible)
- Updated docs
- hook_menu() was using the old hook, not the plugin manager, therefore the issues with tabs not showing up, fixed
- Re-introduced default definition elements so not all keys need to be provided in the .yml file; we can maybe default the type key, but I'm not sure about that one
- Fixed the RSS publishing fail by using a separate key :D
- Moved the field entity mapping stuff into the new alter hook which is now dependent on plugin definitions

This now passes all tests for me. Let's see here.

It helps if I post the right patch. The interdiff was correct, the patch was not.

Updated some docs as well to ensure up-to-dateness.

Fixed dots at the end of @see's (as per YesCT) and updated alter hook main line. Committing this one. I consider this done for this patch/issue. I'll continue cleaning up the plugin stuff in another issue.

Reading through this. Looks good.

tiniest of nits, https://drupal.org/node/1354#see no period in the end of @see (@Gábor Hojtsy says can fix before commit)

Not blocking, just noting: to reduce the info contrib (or otherwhere in core) has to specify things, like in config_translation.config_translation.yml, base_path: 'admin/structure/block/manage/{block}', config entites would have to be refactored to have a URI pattern, maybe in an annotation. Would have to be a core issue to do that. @effulgentsia might have thoughts.

oops. crosspost.