Skip to content
Snippets Groups Projects

Issue #3086941: Update experimental theme/module installation warning to include Oxford comma

Issue #3086941: Update experimental theme/module installation warning to include Oxford comma

Closes #3086941

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Tyler Staples added 1 commit

    added 1 commit

    • 07470b6d - Issue #3086941: Added two tests to DependencyTest to test for 1 module...

    Compare with previous version

  • nicxvan resolved all threads

    resolved all threads

  • nicxvan
  • Tyler Staples added 1 commit

    added 1 commit

    • b1a137bd - Issue #3086941: Created a new test function to test all the messages

    Compare with previous version

  • Tyler Staples added 1 commit

    added 1 commit

    Compare with previous version

  • Tyler Staples resolved all threads

    resolved all threads

  • Jess
    Jess @xjm started a thread on the diff
  • 117 117 '@theme' => $get_label($theme),
    118 118 // It is safe to implode this because theme names are not translated
    119 119 // markup and so will not be double-escaped.
    120 '@required' => implode(', ', array_map($get_label, $dependencies)),
    120 '@required' => (function ($items) {
    121 $count = count($items);
    122 if ($count <= 1) {
    123 return implode('', $items);
    124 }
    125 if ($count == 2) {
    126 return implode(' and ', $items);
    127 }
    128 $last = array_pop($items);
    129 return $items ? implode(', ', $items) . ', and ' . $last : $last;
    130 })(array_map($get_label, $dependencies)),
    • Comment on lines -120 to +130
      Maintainer

      Isn't this introducing a regression for non-English translations? Suddenly they have the English word "and" in a non-translated string placeholder that they can't manipulate.

      I believe the correct way to fix this is something like:

      You must install the following modules to install Foo:

      • Bar

      With the plural case being:

      You must install the following modules to install Navigation:

      • Many
      • Different
      • Things
    • Please register or sign in to reply
    Please register or sign in to reply
    Loading