Description for reused fields not correct

I'm curious why this doesn't match more of the code in Drupal 7, or if we should cleanup what the Drupal 7 code is doing:

          if ($also_known_as = array_unique(array_diff($info[$key]['labels'], array($info[$key]['label'])))) {
            $info[$key]['description'] .= ' ' . t('Also known as %labels.', array('%labels' => implode(', ', $also_known_as)));
          }

@Dave Reid,

I'm curious why this doesn't match more of the code in Drupal 7, or if we should cleanup what the Drupal 7 code is doing:

I think we should clean the D7 version. The patch from #5 looks a lot more cleaner.

The function used there is based on field_views_field_label(), which is however in some weird views include file, so I duplicated it.

That's also why the return value is strange, that other function does it in the same way. A cleaner fix might be to change it to return an array that has the labels as value not key and possibly not even include the first one, explicitly declaring it as $additional_labels?

Here is a patch which changes the function itself. It also changes the way we call t() so that the labels are correctly put in a placeholder tag.

Thanks for working on this.

1. +++ b/token.tokens.inc
   @@ -1185,15 +1185,17 @@ function field_token_info_alter(&$info) {
            'name' => SafeMarkup::checkPlain($label),
   -        'description' => $description,
   +        'description' => t($description, $params),
   

   The problem with this is that it is no longer possible for potx to extract translatable strings.

   To keep that working, we need two t() calls in an if/else, each must be the complete sentence.

2. +++ b/token.tokens.inc
   @@ -1208,11 +1210,11 @@ function field_token_info_alter(&$info) {
   +  foreach (array_keys(\Drupal::getContainer()->get('entity_type.bundle.info')->getBundleInfo($entity_type)) as $bundle) {
   

   Use \Drupal::service(), one method call less.

3. +++ b/token.tokens.inc
   @@ -1208,11 +1210,11 @@ function field_token_info_alter(&$info) {
   +    $bundle_instances = array_filter(\Drupal::getContainer()->get('entity_field.manager')->getFieldDefinitions($entity_type, $bundle), function ($field_definition) {
            return $field_definition instanceof FieldConfigInterface;
          });
   

   I'm not sure but if the instanceof check is really necessary. We soon want to support base fields anyway too, and I very much doubt that given a configurable field, there is a non-configurable field too. that would be... very problematic. So just drop that part.

4. +++ b/token.tokens.inc
   @@ -1222,13 +1224,18 @@ function _token_field_label($entity_type, $field_name) {
   +  return [$field_name, $all_labels];
   

   $field_name vs. $all_labels is somewhat confusing. should be $most_used_label or so.

   What if we just return a simple array of labels, sorted by usage? Then we can use the function like this:
   $labels = ...
   $params['@label'] = array_shift($labels);
   if ($labels) {
   If there are more labels, do the also known as thing..
   }
   else {
   build the label for a single label.

Fixing 1, 2, and 4. I wanted to keep 3 separate so as to not break anything. I tested this manually and it works great.

Now fixing for point 3 in #11. I tested the token browser and it worked fine. This was a standard install of Drupal 8 with just token installed, cache cleared, and a shared field between two content types.

Thanks, looks good to me.

Not going to make you write tests for this when we have none yet. But it would be great if you could open a follow-up or comment in one of the existing field token related issues (there's a meta too) that we should also add fields to multiple bundles to test this part too.