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:
- 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 → .
- Use mb_convert_case \MB_CASE_LOWER_SIMPLE there.
- 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.
- 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().
- Write a change record.
- 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.
ghost of drupal past → created an issue.
As I was extending on the documentation of the attribute I realized prefix/suffix is not needed at all.
The rector rule already has the battle tested code to convert something like form_FORM_ID_alter to a preg so we can just add the name of the hook documented here and be done.
$parts = explode('_', $hook);
$isUppercase = false;
foreach ($parts as &$part) {
if (!$part) {
continue;
}
if ($part === strtoupper($part)) {
if (!$isUppercase) {
$isUppercase = true;
$part = '[a-z0-9_]+';
}
}
else {
$isUpperCase = false;
}
$hookRegex = implode('_', $parts);
Yes. The core tenet of the proposal is this: there's no known advantage of final over @final while there are certainly downsides so let's always use @final.
Could you please elaborate on what advantage final offers that @final does not?
> can we make it compatible with the object way of doing things,
It's Turing complete, nothing stops you :P
Can is not the problem, want is. It's a bit painful to do it.
It is there because it is patterned after AsEventListener, the class level could already be deprecated and removed in 12 , it's not really needed.
Ah no, not linking won't work because it's not just api.drupal.org which would make this mostly a core concern -- but it's every other tool out there too. Then it doesn't need framework review because things no longer break.
I can tell you with authority that linking an attribute to its defining class is working with api module because I wrote the attribute integration for api module :) 🐛 Attributes are not rendered when showing the source code Active
Now, that's just attributes in general not hooks. But this change would mean that api does not need to do anything else. Right?
Hook specific behavior, once again would be linking a method marked with #[Hook('form_filter_admin_overview_alter')] to the class page of FormAlter where you can find the api documentation, much like on the current page. My battle plan was to take all PREFIX and SUFFIX pairs from classes extending Hook, form a massive preg and match. You can not do this with $hook = ($form_id === NULL) ? 'form_alter' : 'form_' . $form_id . '_alter'; that's not something a simple parser can understand.
But, if the moving of api documentation is conditional with changing #[Hook('form_filter_admin_overview_alter')] to #[FormAlter('filter_admin_over')] then it just works by the nature of FormAlter being a class which links to defining page. As you can see on the test, AttributeTest is already a link: Nothing hook specific needs to happen. This does make the life of the api maintainer easier for sure.
It's a decision core leadership needs to make: is it acceptable that while #[Hook('form_filter_admin_overview_alter')] works in Drupal it will not link to the documentation page on api.drupal.org?
📌 [PP-1] Make doctrine/lexer:^3.0 compatible with \Drupal\Component\Annotation\Doctrine. Active is a great example of why using final is bad.
DocParser was written for Lexer 1 when token was an array, 2 added an object with ArrayAccess support and 3 dropped this support finalizing the move to object. "Luckily" the class is final so there is no easy way to add ArrayAccess back for the short time annotations are still supported before everything becomes an attribute. (I will crosslink this issue from the @final issue as a textbook example of why final is bad.) Also, why they dropped ArrayAccess I can't even imagine, it was four one line methods.
Recommendation: do not upgrade. Let this be until it's removed.
If that's not desirable I guess I can write a rector rule to convert every array access to an object access unless the variable is called $metadata and pray it works.
The original reason was, I thought we need to be able to link #[Hook('form_filter_admin_overview_alter')] to hook_form_alter documentation which will live on the class doxygen of FormAlter.
But maybe that's unnecessary? Just link FormAlter directly and while #[Hook('form_filter_admin_overview_alter')] will continue to work in Drupal , it won't work on api.drupal.org and that might be just fine if hooks get converted to their respective attribute when the doxygen is moved. Yeah. It might just work.
It's my fault for not realizing this sooner. Yes, that can work certainly that makes api.drupal.org implementation indeed a lot easier if we don't parse hook names and only link on attributes. But that is a decision far above my current paygrade.
I would put @final to the end I know the documentation currently calls for annotations to go there but since annotations are being removed, this is not a biggie. Doctrine can pick them up even if there is something after.
It's worth noting how this came to be because, well, it's not like this code is AI written to hallucinate interfaces and yet a web search quickly shows this interface only seems to exist in these comments. How odd. Is there perhaps more to this? No: ListDefinitionInterface was added in 2013 in
#2047229: Make use of classes for entity field and data definitions →
and got renamed to ListDataDefinitionInterface in 2014 in
#2002134: Move TypedData metadata introspection from data objects to definition objects →
still within the 8.x development cycle.
If I had the powers to change issue metadata which I don't then I would change the title to
Use @final to define classes that are NOT extension points
I've had to do a lot of deep debugging of the form and render APIs whilst working on Experience Builder.
Out of this I feel I've gotten a very deep understanding of how they work
Want a hug?
Whatever is composer is doing they need to roll it back, upstream usually uses security as an excuse to violate semver but neither 2.8.7 nor 2.8.8 is marked as a security release. What is even the point in semver if something like this can happen?
My two cents: already the security team has the policy to mark unmaintained projects unsupported. Spraying a probabilistic series of PHP tokens into git is not maintainership.
But, it's up to the security team whether they share my concern of being overrun.
ghost of drupal past → made their first commit to this issue’s fork.
There's a much worse problem: people now post AI generated modules which were clearly never even read by a human. The cherry on the cake is they have the audacity to opt such code into security coverage. I just reported a security hole in such and I haven't even started looking seriously. These modules should be removed from security coverage and their publisher needs to lose their security coverage privileges IMO. I doubt we can stop the proliferation of them because they will just post this garbage to github otherwise but no one should for the security team to deal with slop.
@see NestedArray::unsetValue()
ghost of drupal past → created an issue.
About api module, that's my bad, originally I said I do not know how it will fall out but api already has a "class hierarchy" option which we can reuse.
This is bogus so far. PantheonDocumentWithSmartComponentTest::setUp calls documentTraitSetup which calls parent::setUp.
And I do not think any twig nodes are directly instantiated by this module.
Just grab the three relevant config files from standard/config/optional and install them programatically.
I tried to understand how runtime works and I tried to contribute my fresh understanding: I submitted https://github.com/symfony/symfony-docs/pull/20909 and created https://app.infinitymaps.io/maps/mnJJ9FR4FND. I hope this helps someone.
My worry about unit tests would be performance but I am so often wrong in what is useless micro optimization and what is legit worry.
I simply refuse to add more bloat. But, if it's required, I can just won't fix this, it's not so important.