Problem/Motivation

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

Enter a descriptive title (above) relating to interface ContextProviderInterface, then describe the problem you have found:

The documentation block in code looks like this:

/**
 * Defines an interface for providing plugin contexts.
 *
 * Implementations only need to deal with unqualified context IDs so they only
 * need to be unique in the context of a given service provider.
 *
 * The fully qualified context ID then includes the service ID:
 * @{service_id}:{unqualified_context_id}.
 *
 * @see \Drupal\Core\Plugin\Context\ContextRepositoryInterface
 */

But in the API page this shows up mangled, because the "@{service_id}" is interpreted as a doxygen tag. It ends up looking like this:
The fully qualified context ID then includes the service ID: service_id}:{unqualified_context_id}.
While it should read:
The fully qualified context ID then includes the service ID: @{service_id}:{unqualified_context_id}.

Proposed resolution

There are two viable patches posted here:

  1. #3014969-2: Unescaped "@" in ContextProviderInterface doc comment escapes the @ - this is the same method as used elsewhere in core, see #213120: Escape \@.
  2. #3014969-4: Unescaped "@" in ContextProviderInterface doc comment puts the problem text in a @code/@endcode block.

Remaining tasks

None

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

None

CommentFileSizeAuthor
#22 3014969-patch-applied.png16.05 KBabhijith s
#18 3014969-18.patch783 byteslongwave
#4 3014969-4-escape-at-sign.patch653 bytestr
#2 3014969-2-escape-at-sign.patch630 bytestr

Comments

TR created an issue. See original summary.

tr’s picture

tr commented
Status: Active » Needs review
StatusFileSize
new 3014969-2-escape-at-sign.patch630 bytes

Patch.

joachim’s picture

joachim commented
Status: Needs review » Needs work

I'm not keen on escaping things in docs -- they are meant to be readable by humans in text editors, primarily.

Would the problem be fixed by putting this in a @code block instead?

tr’s picture

tr commented
Status: Needs work » Needs review
StatusFileSize
new 3014969-4-escape-at-sign.patch653 bytes

Doc blocks are for the API module, to provide readable documentation on the api.drupal.org site.

It's broken now. I don't care how it's fixed. Escaping is used in other places in core, and I consider it semantically wrong to use @code for non-code text simply because we don't have a "preformatted" tag, but if you prefer @code instead of escaping here's an alternative patch ...

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

tr’s picture

tr commented
Priority: Minor » Normal

I encountered this again yesterday. It's still broken ... see the documentation linked in the original post.

There are two viable patches here:

  1. #2 escapes the @ - this is the method used elsewhere in core, e.g. in lib/Drupal/Core/Entity/entity.api.php and lib/Drupal/Core/Language/language.api.php
  2. #4 puts problem text in a @code block
shimpy’s picture

shimpy
Location India
commented

I also went through few core files.I think #2 works well because this doc is not a code related. So putting in @code block doesnot make any sense. Rather escaping it is a better idea.

joachim’s picture

joachim commented

Escaping looks ugly and confusing when you read the documentation directly in the code files.

tr’s picture

tr commented

@joachim: Patch #4 fixes this with @code - if that's what you prefer, how about a confirmation that it fixes the bug and an RTBC?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I *think* the problem is that @{ identifies the start of a docblock group. As this is just a pair of placeholders to show the format, can we sidestep the issue by using different syntax for the example? Could we say something like:

 * The fully qualified context ID then includes the service ID, e.g.
 * "@service_id:unqualified_context_id".

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

tr’s picture

tr commented

Bump.

sivaji_ganesh_jojodae’s picture

sivaji_ganesh_jojodae commented
Issue summary: View changes

@code looks like a dapper way of handling this.

tr’s picture

tr commented
Issue summary: View changes
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

Both #2 and #4 solve the problem, as would my suggestion in #11. RTBC for core committers to make a decision.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

To be honest I would go for the suggestion in #11 here - solves the problem with the least visual disruption.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
StatusFileSize
new 3014969-18.patch783 bytes

Implemented #11.

Status: Needs review » Needs work

The last submitted patch, 18: 3014969-18.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review

Random fail.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

abhijith s’s picture

abhijith s commented
StatusFileSize
new 3014969-patch-applied.png16.05 KB

Applied patch #18 on 9.2.x and its works fine.

after

abhijith s’s picture

abhijith s commented
Status: Needs review » Reviewed & tested by the community

  • catch committed 2b76f57 on 9.2.x 
    Issue #3014969 by TR, longwave, Abhijith S, joachim, shimpy: Unescaped...

  • catch committed dd91bf1 on 9.1.x 
    Issue #3014969 by TR, longwave, Abhijith S, joachim, shimpy: Unescaped...
catch’s picture

catch
he/him
Primary language English
commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!

  • catch committed 4dce8e5 on 9.0.x 
    Issue #3014969 by TR, longwave, Abhijith S, joachim, shimpy: Unescaped...
catch’s picture

catch
he/him
Primary language English
commented
Version: 9.1.x-dev » 8.9.x-dev

Actually all the way to 8.9.x since this is a docs fix.

  • catch committed 1370b83 on 8.9.x 
    Issue #3014969 by TR, longwave, Abhijith S, joachim, shimpy: Unescaped...

Status: Fixed » Closed (fixed)

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