- Issue created by @charginghawk
- Merge request !9362Fix "See also" section of entity.api.php's entity_crud section. β (Open) created by charginghawk
- πΊπΈUnited States charginghawk
Created MR to fix the issue. Also fixed "entity_characteristics"/"entity_type_characteristics" mixup from π Add an API docs topic to cover entity characteristics in general and how they work Fixed .
- Status changed to Needs review
4 months ago 10:47pm 28 August 2024 - Status changed to Needs work
4 months ago 3:56pm 31 August 2024 - πΊπΈUnited States smustgrave
Can you point to the coding standard or documentation rule that this is failing?
Looked for a few examples but didn't find much but the phrase "See these" doesn't appear to be used anywhere so would have to find the standrad.
- Status changed to Needs review
4 months ago 7:56pm 3 September 2024 - πΊπΈUnited States charginghawk
The current formatting is failing because the the way the section comes out is just broken. The content is written to be one section:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
But because of the mistaken "@see" usage it gets truncated and the remainder ends up in the "See also" section at the bottom:
https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Entity!entity.api...
I made an update to my MR not use "See these:" and instead put the classes inline into the sentence, eg "See\Drupal\Core\Entity\RevisionableInterface and \Drupal\Core\Entity\RevisionableStorageInterface."
I see similar formatting in core.api.php:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/core.api.php#...
https://api.drupal.org/api/drupal/core%21core.api.php/group/cache/11.x
- π³πΏNew Zealand quietone
Compare the current display to the D9.1 version of the page. The D9.1 version display correctly and the file has had minimal changes, and none that changed any of the @see.
Therefore, I think this should move to a bug in the API project β .
- πΊπΈUnited States charginghawk
IMO, the classes should be in context within the section, I genuinely believe that was how it was meant to be read, and my MR makes it a reality. As far as I can tell, @see is usually put at the bottom of the respective section (because it is by definition an afterthought), whereas here it is interspersed with the content. You could argue the behavior in the latest Drupal vs Drupal 9 is expected, to allow putting multiple lines in an @see statement.
Either the @see statements should be moved to the end of the section, or we should use my approach of "See \Drupal\ThisClass and \Drupal\ThatClass." that is also common. I think the latter makes more sense, but the current formatting doesn't make sense either way.
- πΊπΈUnited States smustgrave
Meant about moving this the API project mentioned in #8
- πΊπΈUnited States charginghawk
I do not think this is an API module issue, I think it is expected behavior that @see can encompass multiple lines. In the 8.x-1.x version of the API module, the regex that picks out @see docblock stopped at the end of the line (\n):
https://git.drupalcode.org/project/api/-/blob/8.x-1.x/api.parser.inc#L1131
In the 2.x version of the module, they updated it so that all parsing uses the same regex, which can span multiple lines (until it encounters another @tag or reaches the end of the code comment):
https://git.drupalcode.org/project/api/-/blob/2.0.0/src/Parser.php?ref_t...
https://git.drupalcode.org/project/api/-/blob/2.0.0/src/Parser.php?ref_t...So, (1) the original formatting never followed best practice in the first place. You should not intersperse @see's with content but instead put them at the end. Even with the 8.x-1.x behavior, there's no reason to intersperse them with content because they would automatically be moved to the end.
(2) The @see usage was probably mistaken, since the classes are most useful to readers in context. You read the paragraph about revisions, and at the end it points to the revision classes, then you read the paragraph about translations, and at the end it points to the translation classes, finally you read the paragraph about revision translations, and it points to the revision translation classes. That's helpful. Linking to classes at the end of a paragraph is done in many places throughout the API documentation. Dumping all the classes into the "See more" section at the bottom of the bottom outside of context is less helpful.
(3) The API's current behavior is expected and the documentation should be updated to cohere to it. Multi-line @see statements make sense. The formatting of this documentation in question just does not make sense, any way you cut it. There is no reason to expect the API module to revert to single line @see statements for this specific use case, nor should it.
I think my MR is the cleanest fix to everything here.
- π³πΏNew Zealand quietone
Well, #12 proves my point. A change in the API module has changed the way the @see is handled. That is a regression in that project and should be handled there.
Also, keep in mind that all the *.api.php files expect the @see to behave the way it did before the regression.
- πΊπΈUnited States smustgrave
Don't have an option about what project this belongs in but pipeline is failing.
- π³πΏNew Zealand quietone
I just set the issue in the API module that is fixing the regression of the @see behavior to RTBC. That should solve the regression. Looking at the MR the changes that I think should be made are the changes to use "entity_type_characteristics'. And the other concerns about the @see raised in #12, the should probably be discussed in the Coding Standards project.