Apply coding standards and best practices

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

echo15 created an issue. See original summary.

echo15’s picture

Providing patch.

echo15’s picture

Status: Active » Needs review
nikolas.tatianenko’s picture

Status: Needs review » Reviewed & tested by the community
sosevich.v’s picture

Status: Reviewed & tested by the community » Needs work

After the patch #2 module still needs work on the following files (checked with Drupal and DrupalPractice standards):

/languagefield.info.yml
/src/Controller/LanguageAutocompleteController.php
/src/CustomLanguageListBuilder.php
/src/Entity/CustomLanguage.php
/src/Entity/CustomLanguageInterface.php
/src/Entity/CustomLanguageManager.php
/src/Form/CustomLanguageForm.php
/src/Plugin/Field/FieldFormatter/LanguageFormatter.php
/src/Plugin/Field/FieldType/LanguageItem.php
/src/Plugin/Field/FieldWidget/LanguageAutocompleteWidget.php
/src/Plugin/Field/FieldWidget/LanguageSelectWidget.php
/src/Plugin/views/filter/LanguageFilter.php

This is true for the module:
version: '8.x-1.4'
core: '8.x'

echo15’s picture

@sosevich.v this issue can stay opened for future codestyle updates

echo15’s picture

Status: Needs work » Needs review
sosevich.v’s picture

Status: Needs review » Reviewed & tested by the community

Ok, I see. In that case the patch fixed given files according to coding standards.

For further improvements see comment #5.

  • johnv committed 3b0d136 on 8.x-1.x authored by echo15
    Issue #3036979 by echo15: Apply coding standards
    
johnv’s picture

Status: Reviewed & tested by the community » Active

Committed. Thanks. Back to active for more files.

phjou’s picture

Status: Active » Fixed
phjou’s picture

Status: Fixed » Active

Sorry wrong edit.

  • johnv committed bfd988e on 8.x-1.x
    Issue #3036979: Apply coding standards
    
lamp5’s picture

Assigned: echo15 » Unassigned
Status: Active » Needs review
FileSize
35.21 KB

  • johnv committed 09e4a5d on 8.x-1.x authored by lamp5
    Issue #3036979: Apply coding standards, add dpendency injection
    
johnv’s picture

Version: 8.x-1.x-dev » 8.x-1.5
Status: Needs review » Fixed

Committed, thanks.
Only the following parts have not been changed, since it changes behaviour without justification:

-            $subsettable_languages[$langcode] = t($language_names[0]);
+            $subsettable_languages[$langcode] = $language_names[0];
         LanguageInterface::LANGCODE_NOT_SPECIFIED => $this->t('Language neutral'),
-        'current_interface' => $this->t('Current interface language') . '*',
-        'authors_default' => $this->t("Author's preferred language") . '*',
+        'current_interface' => $this->t('Current interface language'),
+        'authors_default' => $this->t("Author's preferred language"),
       switch ($format) {
         case 'en':
-          $language_name .= $this->t($language_names[0]);
+          $language_name .= $language_names[0];
           break;
lamp5’s picture

Translating Variables

You should never use t() to translate variables, such as calling

t($text);

, unless the text that the variable holds has been passed through t() elsewhere (e.g., $text is one of several translated literal strings in an array). It is especially important never to call

t($user_text);

, where $user_text is some text that a user entered - doing that can lead to cross-site scripting and other security problems. However, you can use variable substitution in your string, to put variable text such as user names or link URLs into translated text. Variable substitution looks like this:

$text = t("@name's blog", array(
  '@name' => format_username($account),
));

@from https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/t/7.x

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

nagy.balint’s picture

Hi!
The following change in getLanguageConfigurationValues caused an issue:
- $value = $code;
+ $value = LanguageInterface::LANGCODE_NOT_SPECIFIED;

Its fixed at #3125977: Widget 'Language select list' does not save selected value