Add language code option

+1 patch works fine, thanks

Thanks for proposing this useful option! I reviewed the code:

+// Language display options
+const LANGUAGE_DISPLAY_NATIVE = 0;
+const LANGUAGE_DISPLAY_LANGCODE = 1;

Global constants are no longer allowed in the Drupal 8 coding standards. These should be moved into a class or interface. I suppose the LanguageiconsAdminSettings class is the perfect choice since it is the only class in the entire module. Normally it is not considered best practice to make public constants available inside a form class, but it would be overkill to introduce an interface just for this.

Also let's not use magic numbers for these options, let's use human readable strings like native and langcode so that site builders can make sense of the options when they are reading the config YML files.

It makes it a bit more difficult since the Drupal form builder expects numbers for select dropdowns and not string keys, but perhaps we can solve this by enumerating the options in a single constant like this?

  public const DISPLAY_FORMATS = [
    0 => 'native',
    1 => 'langcode',
  ];

+    $language_options = array('' => t('None'), LANGUAGE_DISPLAY_NATIVE => t('Language Native Name'), LANGUAGE_DISPLAY_LANGCODE => t('Language Code'));

According to coding standards we have to use the short array syntax [] instead of array().

+    $form['display_format'] = [
+      '#type' => 'select',
+      '#title' => t('Language format'),
+      '#options' => $language_options,
+      '#default_value' => $this->config('languageicons.settings')->get('display_format'),
+      '#description' => t('Display language native name: english or language code: en'),
+    ];

This new config key display_format needs to be added to the schema in config/schema/languageicons.schema.yml.

I like this idea and I'm maintainer now. Could someone prepare a MR for review?

Hello ,
I have applied patch #3 on 8.x-1.x and 2.x but it does not apply cleanly.
I have implement the language code by referencing #3 and #5 comment.
Please review the MR.
If any mistake please guide me.
Thanks

I tested the issuse fork and it was successful.

Thanks for the great work on this!

I had a look at the merge request. It looks good, but we are adding a new option to the configuration, so this also needs a config schema update and a simple post-update hook to set the default value for existing installations.

Updated the config schema and provided an update hooks, and did some minor cleanup. I have not tested my changes.

Thanks all, looking really good!

@pfrenssen I might be wrong, but shouldn't "simple" config updates like this use regular update hooks?
See https://www.specbee.com/blogs/update-and-post-update-hooks-for-successfu...

I'm always confused about this. According to the documentation, config updates are supported in hook_update_N(), but I always thought it is best practice to only use those for data model updates nowadays. I saw a bunch of config updates in the post-update hooks of the core system module so I went with that. E.g. system_post_update_service_advisory_settings()

I guess both work. If you want I can change it.

@pfrenssen all good, I'm totally fine with that and I'm also confused, what's the right way ... we can keep it like this.

So only needs final testing?