The documentation comment for LogMessageParserInterface::parseMessagePlaceholders() is the following one.

  /**
   * Parses and transforms message and its placeholders to a common format.
   *
   * For a value to be considered as a placeholder should be in the following
   * formats:
   *   - PSR3 format:
   *     @see https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-3-logger-interface.md#12-message
   *   - Drupal specific string placeholder format:
   *     @see \Drupal\Component\Render\FormattableMarkup
   *
   * Values in PSR3 format will be transformed to
   * \Drupal\Component\Render\FormattableMarkup format.
   *
   * @param string $message
   *   The message that contains the placeholders.
   *   If the message is in PSR3 style, it will be transformed to
   *   \Drupal\Component\Render\FormattableMarkup style.
   * @param array $context
   *   An array that may or may not contain placeholder variables.
   *
   * @return array
   *   An array of the extracted message placeholders.
   */

The list about the placeholder formats is rendered as follows.

For a value to be considered as a placeholder should be in the following
formats:

  • PSR3 format:
    • Drupal specific string placeholder format:

The list items are shown indented, when they should not, and there isn't any link.

I am linking the Drupal 8.6 documentation because there isn't a link to the Drupal 8.7 documentation, so far, but the bug exists also for Drupal 8.7.

CommentFileSizeAuthor
#16 fix-documentation-markup-2998769-16.patch1.05 KBavpaderno
#14 interdiff-11-14.txt879 bytesquietone
#14 2998768-14.patch1023 bytesquietone
#11 fix-documentation-markup-2998769-9.patch803 bytesquietone
#9 fix-documentation-markup-2998769-9.patch803 byteskkalaskar
#4 fix-documentation-markup-2998769-4.patch1 KBavpaderno
#3 fix-documentation-markup-2998769-3.patch1014 bytesavpaderno

Comments

kiamlaluno created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

I would also not use the colons after each format description. The descriptions should be rendered by the following markup.

<ul>
  <li><a href="https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-3-logger-interface.md#12-message">PSR3 format</a></li>
  <li>Drupal specific string placeholder format (See <a href="https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Render%21FormattableMarkup.php/class/FormattableMarkup/8.6.x">\Drupal\Component\Render\FormattableMarkup</a>.)</li>
</ul>

What the browsers would render is the following.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new fix-documentation-markup-2998769-3.patch1014 bytes

Given that fully qualified class names are automatically linked, the documentation comment should be the following one.

  /**
   * Parses and transforms message and its placeholders to a common format.
   *
   * For a value to be considered as a placeholder should be in the following
   * formats:
   *   - @link https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-3-logger-interface.md#12-message PSR3 format @endlink
   *   - Drupal specific string placeholder format
   *     (See \Drupal\Component\Render\FormattableMarkup.)
   *
   * Values in PSR3 format will be transformed to
   * \Drupal\Component\Render\FormattableMarkup format.
   *
   * @param string $message
   *   The message that contains the placeholders.
   *   If the message is in PSR3 style, it will be transformed to
   *   \Drupal\Component\Render\FormattableMarkup style.
   * @param array $context
   *   An array that may or may not contain placeholder variables.
   *
   * @return array
   *   An array of the extracted message placeholders.
   */

There is a line that is longer than 80 characters, but it's not possible to split it over more lines.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
StatusFileSize
new fix-documentation-markup-2998769-4.patch1 KB

This is the correct patch.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: +Novice
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue summary: View changes

Fixing input filter

quietone’s picture

quietone commented
Status: Needs review » Needs work

thx, kiamlaluno. this will make it easier to read.

+++ b/core/lib/Drupal/Core/Logger/LogMessageParserInterface.php
@@ -12,10 +12,9 @@
+   *     (See \Drupal\Component\Render\FormattableMarkup.)

The full stop should be at the end of the sentence. And, I think, the parentheses are not needed anyway, so they can be removed. I cant' tell from reading API documentation and comment standards but I think the 'See' should be on the previous line.

quietone’s picture

quietone commented

Changing attribution

kkalaskar’s picture

kkalaskar commented
StatusFileSize
new fix-documentation-markup-2998769-9.patch803 bytes

Hello kiamlaluno, quietone.

I created the new patch as per the comment of quietone. Please have a look once and review.

kkalaskar’s picture

kkalaskar commented
Status: Needs work » Needs review
quietone’s picture

quietone commented
StatusFileSize
new fix-documentation-markup-2998769-9.patch803 bytes

I applied the patch and looked at the code and it is an improvement, fixing the problem found in the IS, so I think this is good to go. I'm uploading the patch again because of the errors reported in the many tests in #9 are unrelated. Just want to get a clean start.

RTBC+1, I'll just wait for the tests to pass.

quietone’s picture

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

Tests are passing, not surprised by that. I've still got a lot to learn about documentation but this does fix the issue.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Reviewed & tested by the community » Needs work

The reported issue is probably caused from the @see directive that should not be there. The patch is not removing it.

quietone’s picture

quietone commented
Status: Needs work » Needs review
StatusFileSize
new 2998768-14.patch1023 bytes
new interdiff-11-14.txt879 bytes

Ah, silly me. How about this?

Status: Needs review » Needs work

The last submitted patch, 14: 2998768-14.patch, failed testing. View results

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new fix-documentation-markup-2998769-16.patch1.05 KB
quietone’s picture

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

Thanks, kiamlaluno. I agree with that. Now, even though I made a patch here (one was a resubmission and the other was wrong) I'm going to RTBC. Improving documentation is just so helpful to so many it is worth trying the RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 6d98f6bf36 to 8.7.x and 8a96202f35 to 8.6.x. Thanks!

Let's put the @see \Drupal\Component\Render\FormattableMarkup in a good place so there is a helpful link.

diff --git a/core/lib/Drupal/Core/Logger/LogMessageParserInterface.php b/core/lib/Drupal/Core/Logger/LogMessageParserInterface.php
index d7c0360a4a..2d9223b70b 100644
--- a/core/lib/Drupal/Core/Logger/LogMessageParserInterface.php
+++ b/core/lib/Drupal/Core/Logger/LogMessageParserInterface.php
@@ -28,6 +28,8 @@
    *
    * @return array
    *   An array of the extracted message placeholders.
+   *
+   * @see \Drupal\Component\Render\FormattableMarkup
    */
   public function parseMessagePlaceholders(&$message, array &$context);

Fixed on commit.

  • alexpott committed 6d98f6b on 8.7.x 
    Issue #2998769 by kiamlaluno, quietone, kkalaskar: @see directive used...

  • alexpott committed 8a96202 on 8.6.x 
    Issue #2998769 by kiamlaluno, quietone, kkalaskar: @see directive used...
quietone’s picture

quietone commented

@alexpott, thanks, thank makes sense.

Status: Fixed » Closed (fixed)

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