🇨🇦Canada @Charlie ChX Negyesi

If I weren't paid to fix this I would kick this back to the core queue saying stop writing documentation for phpcs and start writing documentation for developers but as is, I will of course fix it but I couldn't resist noting this.

Note for the archives, to see what is in the members table, one can take the query from the EmptyClassTest and log like this:

    file_put_contents('/tmp/before', print_r(\Drupal::database()->query('        SELECT member.object_name AS member, class.namespaced_name AS class
        FROM {api_branch_docblock_class_member} dcm
        INNER JOIN {api_branch_docblock} member ON dcm.docblock = member.id
        INNER JOIN {api_branch_docblock} class ON dcm.class_docblock = class.id
        ORDER BY class, member')->fetchAll(\PDO::FETCH_ASSOC), TRUE));

and change the log file to /tmp/after in the version you want to see the changes in and then diff -u /tmp/before /tmp/after|grep -v Array shows crystal clear what are the changed members.

To sum up a very hard and long debug session: On DocFile::delete needs to remove all the docblocks for the file being deleted but this creates another bug because DocBlock presumed DocFile::delete doesn't clean up so that delete call needed to be removed.

ghost of drupal past → made their first commit to this issue’s fork.

ghost of drupal past → made their first commit to this issue’s fork.

That didn't work out quite right but I think 📌 Form element document for 10.3.x does not include any elements Active solves this one too.

📌 Form element document for 10.3.x does not include any elements Active subsumed more than half of this issue. One link remains to be solved. Once that's in I will rewrite the IS.

ghost of drupal past → made their first commit to this issue’s fork.

Sorry , wrong branch after commit. It's a different issue. Nevermind it.

my brother, you’re not helping these issues with your drama.

And derailing the issue is? We were near agremeent.

I tried. xjm returned and killed the issue because she hates me, sorry it's a fact, it's been six years and even that's not enough.

We might as well say that everything should just be marked public

The fundamental difference is how protected properties can have logic hardwired to them. Once Drupal core requires PHP 8.4 and property hooks become available this is a proposal I would wholeheartedly support, however.

Regarding final

the actual language feature, which is useful

Useful for what? For example: it explicitly tells the developer "this is not intended for extension". -- yes, @final does exactly that.

Ah but that was the listing page and the bug report is about the object page. I have updated Formatter::pageFunction to use the override documentation for method, param and return documentation.

This is unusual: there's testing ensuring this doesn't happen, in ComplexItemPagesTest:

    $this->assertSession()->linkExists('SubInSubDirSample::bar', 0, 'Overridden function link is there');
    $this->assertSession()->responseContains('A public method');
// the method in question:
class SubInSubDirSample extends SampleInSubDir implements SampleInterfaceTwo {
  /**
   * {@inheritdoc}
   */
  public function bar() {
  }
// the interface in question
interface SampleInterfaceTwo {
  /**
   * A public method.
   */
  public function bar();
}

I will investigate what breaks in the core case.

ghost of drupal past → made their first commit to this issue’s fork.

ghost of drupal past → made their first commit to this issue’s fork.

ghost of drupal past → made their first commit to this issue’s fork.

ghost of drupal past → made their first commit to this issue’s fork.

I already reported the AI slop as spam, you didn't need to act on it. This change has been reverted.

ghost of drupal past → made their first commit to this issue’s fork.

Maybe SYMFONY_DEPRECATIONS_HELPER is parsed but it does absolutely nothing now and 📌 Upgrade PHPUnit to 10, drop Symfony PHPUnit-bridge dependency Fixed added displayDetailsOnTestsThatTriggerDeprecations="true" which is the only thing that now does something and of course it went in undocumented because there was no phpcs rule to demand documentation and heaven forbid documentation gets written which helps developers.

ghost of drupal past → made their first commit to this issue’s fork.

ghost of drupal past → created an issue.

I tried #142 but the amount of methods you'd need to add is staggering and at the end of the day since people use arbitrary properties in render arrays you would need to support some sort of set/get anyways so I leaned heavily on that and so 📌 Slowly, very slowly start OOPifying the render system Needs review is born. If that issue gets accepted this issue might be outdated.

There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant.

No, it suggests people are writing documentation to satisfy a completely outdated policy enforced by automated tools instead of trying to help fellow developers. FieldTypeCategoryInterface gets committed with a comment "Provides an object that returns the category info about the field type." and AuthenticationProviderInterface has "Interface for authentication providers." and I updated #2745947: Adopt a method of documenting service tags → which this issue is postponed upon back in 2023 but absolutely nothing happened. There's no documentation team leader for so many years because, as I said, no one cares one whit about documentation any more aside from bloating core and putting up yet another barrier to core contribution by demanding "documentation" on everything.

Due to #76 I'm honestly not sure how to "properly" build a Docker image that works with Drupal 11 and SQLite.

Not sure about proper but here's what Drupal Code Generator does: https://github.com/Chi-teck/drupal-code-generator/blob/4.x/.github/workf...

Hold, the entire comment will be allowed to be dropped once https://www.drupal.org/project/coding_standards/issues/3376518 📌 Allow omitting doxygen when type and variable name is enough Active is done.

I presume something like this would go under the guidelines section: "Unless a class is explicitly intended to be extended mark it with @final to signal it is not supposed to be extended. Any bug reports regarding the extension of a class marked by @final will be closed. Nonetheless, core does not use the PHP final keyword so developers can still extend such classes at their own risk. It's a "break glass in emergency" solution."

Currently you can enable test modules with a simple setting , it's extremely useful for debugging is that kept?

I did a test first with the casefolding unicode text linked then just to be sure over all 65536 BMP Unicode codepoints and the only character where mb_strtolower($utf8Char) differs from mb_convert_case($utf8Char, MB_CASE_LOWER_SIMPLE) is indeed 0130 Latin Capital Letter I with Dot Above.

Recommended next steps:

1. Add a strtolower method to the Unicode utility class which will be a partial revert of #2850046: Remove usages of \Drupal\Component\Utility\Unicode() functions → .
2. Use mb_convert_case \MB_CASE_LOWER_SIMPLE there.
3. Leave a comment this is the same as mb_strtolower except for U+0130 Latin Capital Letter I with Dot Above which this method lowercases to U+0069 Latin Small Letter I unlike mb_strtolower which lowercases it to U+0069 Latin Small Letter I followed by U+0307 Combined Dot Above.
4. Change EntityAutocompleteController and Drupal\Core\Config\Entity\Query and any other mb_strtolower you can find which looks relevant with a call to Unicode::strtolower().
5. Write a change record.
6. Pending approval upgrade https://www.drupal.org/node/2850048 → as well.

That took me a bit , thanks for the challenge, I enjoy them (this is not a joke). I did try with mariadb and saw no problems https://dbfiddle.uk/S1UWV8sj but when I actually tried locally (I run MySQL 8.0) I saw something interesting:

Look at the results of https://3v4l.org/0TJmt

in theory, both "İstanbul" and "istanbul" should be normalized to "istanbul" before being matched.

Once we see the PHP output disproving this theory, it becomes very easy to search for the problem and it is indeed a known problem. https://stackoverflow.com/a/42887898/308851 says

Unicode solves this problem as follows: when İ is converted to lowercase, it's actually converted to the standard latin i plus the combining character U+0307 "COMBINING DOT ABOVE".

I am not sure what Drupal could do here.

Is your database MySQL? If yes then what does SELECT VERSION(); show and what does show full columns from node_field_data where field='title'; show in the Collation column?

This is very odd. I just checked the install file and it definitely installs the media type and the config files. Could you check you have this file please:
https://git.drupalcode.org/project/pantheon_content_publisher/-/blob/1.0...

phenaproxima → credited ghost of drupal past → .

phpcs won, I am done.

ghost of drupal past → created an issue.

ghost of drupal past → created an issue.

I went down a different route and alas this won't work with that.

ghost of drupal past → created an issue.

Thanks for the PoC.

I left you a couple nits in Slack but I wanted to note: as far as I can tell user supplied jsonpath goes into the query unescaped, that looks like
a problem to me.

Also, , this is what backend overridable services were invented for. I would put the mysql specific logic into one such making eventual postgresql and sqlite implentations seemlessly integrate.

services:
  json_field.query:
     class: this could be an abstract class
    tags:
      - { name: backend_overridable }
  mysql.json_field.query:
     class: Some\Helper\Containing\The\MySQL\Logic

Check the views.date_sql service and friends for an example.

ghost of drupal past → created an issue.

Due to the well known sorry state of documentation and the fact that LLMs can only work from what they ingest it follows that LLMs are typically not capable of good -- or even working -- Drupal code. (Despite this, a few people can wield LLMs successfully to write Drupal code but they are typically exceptionally well versed in writing Drupal code themselves.) If we add the, once again, well known long list of harms LLMs inflict on our society as a whole and the typical inclination of open source developers to do, in a very broad and vague sense, good it is very easy to see why introducing entirely LLM written modules into the Drupal ecosystem was met with significant resistance and disdain. Likely getting emotional over this was a mistake but it's really hard to separate a single act from the widespread wanton destruction wrought by LLMs. The reaction to this was not taking a step back and asking what is our problem but laughing at our concerns which really did not help elevating the tone of the discussion. This retaliatory behavior repeated when, as catch mentions, "two reports were sent by @bigbabert to the security team for modules authored by the same people".

But labeling my work as "spammy", suggesting revoking access, or removing projects without any dialogue is neither fair nor inclusive.

It's exceptionally hard to find inclusive thoughts after all this. Luckily, I am no longer a maintainer in any capacity and the deterioration of the tone of the discussion was partially my fault without a doubt but still, I wanted to give some context why that happened.

ghost of drupal past → created an issue.

The custom attributes were added because I confused the attribute constructor parameters with the hook parameters. It's not a viable idea. That's on me.

Now that we know the documentation stays in api.php their disadvantage shows: they always will be controversial, which hooks get separate attributes and which don't and how many? Trying to decide how entity hooks for example should be sliced will be a lot of fun. And another controversy is this very issue, we spent so much back and forth and there's still new voices and discussion to be had.

For api.module, not having custom attributes is easier since it always needs to support #[Hook('form_foo_bar_alter')] so if it doesn't to support FormAlter on top, that's easier.

I have no problems with fixing this issue by deleting FormAlter and Preprocess attribute classes.

Then let's do this. The name was originally suggested by larowlan and now we have buy in from catch and quietone. It was ready before, the rename was simple and automated.

So do we have an agreement on HookDocumentation then?

No, that's premature, let's wait for quietone.

That's two core committer votes for HookDocumentation so it's done. I reset back

There's nothing to do. While leaking memory in long running PHP processes is, alas, all too frequent there is no need to be long running, the queue is perfectly fine to run one process for each item -- possible even in an parallel.

@sher1 thanks for you report, could you edit your comment and attach the list as a text file please? The issue is becoming very long with these super long comments.

ghost of drupal past → created an issue. See original summary → .

ghost of drupal past → created an issue.

I can trace this if needed but I will bet you anything the problem is Drupal\Core\Command\InstallCommand not flushing caches after install -- this could be hardwired in the command or added to install_tasks.

Another very long Slack discussion. First the agreement reach is the documentation remains in api.php for the time being.

This, however, circles back to the original issue which PREFIX/SUFFIX attempted to address: how does api.module recognize the documentation for #[FormAlter('filter_admin_overview')] can be found at hook_form_FORM_ID_alter?

There was immediate agreement there should be at least an attribute on the FormAlter attribute class itself which helps with this.

While it would be possible to add base_form_id to the FormAlter constructor and use the new attribute like

#[HookPattern('form_alter')]
#[HookPattern('form_FORM_ID_alter'), ['form_id]]
#[HookPattern('form_BASE_FORM_ID_alter'), ['base_form_id]]

Then a method marked with #[FormAlter(base_form_id: 'node_form')] could be linked to form_BASE_FORM_ID_alter.

The complexity doesn't seem to be worth it, the documentation for form_FORM_ID_alter and form_BASE_FORM_ID_alter is nearly identical, merging them seems to be the better road forward.

Briefly we considered an #[Implements] optional attribute at the implementation point as a modernization of the current Implements hook_foo() docblook but if it's always required then it's too boilerplate-y and if only occasionally used then the confusion on when it should be used and when not would be a challenge.

Thus, we converged on a simple solution: just use HookPattern , nothing fancy and merge the documentations where needed. It will be rare, note how hook_entity_query_alter is careful with its underscores so the matching is not ambiguous.

I checked the original issue and one significant change from the original intent is the doxygen was meant to be on the class constructor and not the class itself. This might or might not help. I committed the change to move there (but it's easy to revert).

It's entirely possible I intended to move over to the constructor because I thought we could transplant the arguments to the constructor and we clearly can't do that, the constructor is defining the params of the attribute not the hook. I am all too often wrong and too stubborn to realize. Thanks for keeping me straight.

But since DocumentsHook now targets a method, it could be used anywhere where documentation is convenient.

My suggestion would be to put it on EntityStorageBase::invokeHook() as an example. And where such a method doesn't exist, write one. The amount of boilerplate seems to be the same: one function currently vs one method factored out solely for documentation purposes. One issue certainly is @return -- if we were to factor out \Drupal::moduleHandler()->invokeAll('field_info_max_weight', $args) into a method that would return a list of integers while hook_field_info_max_weight returns an int. That's a problem. Maybe @return-hook? if phpstan can define custom tags can't Drupal do the same?

Or maybe only use DocumentsHook on methods when it's convenient and keep hook_foo_bar in *.api.php alive. For consistency, marking those functions with DocumentsHook would be nice.

This, however, would completely separately DocumentsHook from this issue and so if the decision is to go in this direction then this issue could go back to just ripping out prefix/suffix and then DocumentsHook can be resolved in a separate issue.

The feedback has been addressed where it was relevant. Parts of it, I believe was made before the whole doxygen was read so they are not relevant.