Problem/Motivation

API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Annotatio...

Enter a descriptive title (above) relating to Annotation for translatable text, then describe the problem you have found:

@ Translation("Bundle !title", arguments = {"!title" = "Foo"}),

Proposed resolution

should read
@ Translation("Bundle @title", arguments = {"@title" = "Foo"}),

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aklump created an issue. See original summary.

valthebald’s picture

Title: change !title to @title » Fix usage of unsupported !title placeholder in Translation.php
Version: 8.7.x-dev » 8.9.x-dev
Issue tags: +Novice, +API Documentation, +Amsterdam2019
gdejonghe’s picture

Hello, I'm at Drupalcon Amsterdam and I'm going to work on this issue with @solide-echt and @mbovan .

solide-echt’s picture

Currently at Drupalcon2019 AMS and going to work on this issue with @mbovan and @gdejonghe.

gdejonghe’s picture

Issue summary: View changes

Updated the issue summary

wengerk’s picture

Hey there ! I'm mentoring - alongside with @ mbovan - @gdejonghe and @solide-echt at the Drupalcon in Amsterdam 2019.

Let's create a patch for this issue and having fun !

mbovan’s picture

Hey! We have our local setups ready and looking into fixing the issue now with @solide-echt and @gdejonghe.

gdejonghe’s picture

Status: Active » Needs review
FileSize
4.63 KB

I fixed the translation placeholder.

valthebald’s picture

Status: Needs review » Needs work

@gdejonghe: looks like you've submitted default.settings.php that doesn't belong to this issue. Can you reroll with only Translation.php please?

solide-echt’s picture

I addressed the comment from #9 in the interdiff

The last submitted patch, 8: fix-title-placeholder-3084345-8.patch, failed testing. View results

Sutharsan’s picture

I've checked the patch, it looks good not. It complies to the example in the issue summary.
The line is part of a code example and is not actually used. Therefore it does not need a test.

When the test bot agrees and goes green, this patch is ready to go and can be set RTBC.

Sutharsan’s picture

Just to be sure, I further checked if any other cases of ! appeared within @Translation(). I used this simple and not too strict regex Translation\(["|'].* !. I did not find any other case.

mbovan’s picture

Status: Needs review » Reviewed & tested by the community

Looks great! We checked the remaining uses of ! placeholder with @gdejonghe and did not anything remaining.

alexpott’s picture

Crediting all the people who worked together on this patch. Plus @aklump for filing the issue.

alexpott’s picture

Version: 8.9.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 6ca44f7895 to 9.0.x and eb2b7a4065 to 8.9.x and 87e53830b6 to 8.8.x. Thanks!

Backporting to 8.8.x because this is a docs fix.

  • alexpott committed 6ca44f7 on 9.0.x
    Issue #3084345 by solide-echt, gdejonghe, valthebald, aklump, mbovan,...

  • alexpott committed eb2b7a4 on 8.9.x
    Issue #3084345 by solide-echt, gdejonghe, valthebald, aklump, mbovan,...

  • alexpott committed 87e5383 on 8.8.x
    Issue #3084345 by solide-echt, gdejonghe, valthebald, aklump, mbovan,...

Status: Fixed » Closed (fixed)

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