Posted by xjm
Problem/Motivation
This is a followup issue for #1535868: Convert all blocks into plugins.
Most (if not all) classes, methods, etc. for the new blocks architecture have the minimum required documentation, but there are no extended descriptions of how the architecture works and how the various components fit together, and it is very difficult to understand without a prior understanding of the plugin system.
Proposed resolution
Add a detailed explanation of the architecture (probably on BlockInterface
). Explain the "big picture" of how all the pieces fit together--plugin discovery, saving to configuration, per-theme derivatives, etc. Include references to other related classes, interfaces, procedural code, etc. (The goal should not be to duplicate what is already documented on individual classes and interfaces, but to help developers understand how it all fits together.)
Remaining tasks
Some of the needed documentation (around how blocks are stored, loaded, etc.) should probably be added following #1871696: Convert block instances to configuration entities to resolve architectural issues.
Comment | File | Size | Author |
---|---|---|---|
#30 | 1871762-30_block-api-docs.patch | 3.55 KB | eojthebrave |
#28 | 1871762-28_block-api-docs.patch | 3.39 KB | eojthebrave |
#25 | 1871762-25_block-api-docs.patch | 3.42 KB | eojthebrave |
#22 | 1871762-22-block_api_defgroup.patch | 4.62 KB | eojthebrave |
#14 | drupal8.documentation.1871762-14.patch | 1.59 KB | jibran |
Comments
Comment #1
xjmComment #2
xjmComment #3
xjm@EclipseGc suggested tonight that we might be able to add an
@Plugin
to an abstract class without it being parsed as an annotation, so let's see how that goes. The attached is only the very beginning of the docs we need.Comment #4
xjmSo... nothing blew up. That's cool. The only other thing we need to take into account then is making sure the API module can parse an annotation in the middle of a docblock like this when it's not being parsed by Drupal.
Comment #5
xjmMissing comma here, and an extra newline near the end.
Comment #6
jhodgdonChange in status per #5 to get this out of my "probably ready to commit" queue. :)
Can I also suggest that the best place to make "Big Picture" documentation of how a system works is with a @defgroup rather than in a particular class's documentation? Then you can add the relevant classes to the topic using @ingroup.
Comment #7
xjm@jhodgdon, any feedback on #4? In this case here, the annotation is not parsed, because the class is abstract. I am not sure if that makes things more difficult for the API module or not.
A defgroup is a great suggestion, and that would also be a good way to get around the "how do we document an example annotation" problem. In fact, the plugin system itself should probably have one.
Comment #8
jhodgdonAh, sorry, didn't notice that question on #4.
So:
- The example of @Plugin should be put into @code/@endcode tags. Otherwise there is zero chance it will be formatted nicely on api.drupal.org.
- The API module will probably not choke on the @Plugin() where it is in the patch above. There is a special case in the API module for handling @Plugin where it assumes @Plugin() is an actual annotation and handles it like @code/@endcode. But the regexp is
'/@Plugin(.*)$/'
so unless there is a ) right at the end of the entire docblock, it shouldn't think the @Plugin in the middle is an actual annotation. It could be a problem if someone added @see function_name() at the very end though, since it would then probably think that almost the entire docblock was an @Plugin annotation.
- The API module doesn't use the actual annotation parser, just this regexp.
Does that help? Basically: Should be OK, but put it in @code/@endcode for best formatting. And I would also suggest adding something to the documentation saying that these @Plugin annotation things come at the end of docblocks. That is not explicitly stated in the documentation. I realize it's clear to people that invented the system, but... :)
Comment #9
xjmThanks @jhodgdon!
Well, that's true of annotations generally, and we do have a mention for it at http://drupal.org/node/1354#annotations (first point under the example). There should be documentation for the plugin system itself in the codebase, though (out of scope here, but definitely something most everyone who hasn't gotten used to annotations via 300 Views plugins has been confused about).
Comment #10
jhodgdonShould we have a @defgroup that explains annotations and the plugin system as well then? It seems like that would be useful if we don't yet have one. It could be a short couple of paragraphs with a link to the plugin API documentation http://drupal.org/node/1637614
Comment #11
xjmComment #12
jibranHere is the reroll.
BlockInterface
changes because seems irrelevant now.I like this suggestion but It is out of scope here because this issue related block system's architecture. We can create a follow up for this and decide where to put @defgroup that explains annotations.
Comment #13
jhodgdonThis documentation seems pretty clear to me, except this one place:
I am confused by this... what does it mean? Maybe it is just a typo and "these" in the second line should be "the", so that it is trying to say you can also put settings in your annotation to override the default settings?
I have also not reviewed the documentation for accuracy. Someone familiar with the block system needs to do that.
Comment #14
jibranReroll after #2065721: Add a dedicated @Block plugin annotation.
Comment #16
jhodgdonWhere do all of the other settings go now?
Comment #16.0
jhodgdonRemoving myself from the author field so that I can unfollow the issue. --xjm
Comment #17
jhodgdonWe actually need a @defgroup topic for this to be linked to from the new Drupal 8 api.d.o front page for API docs. See #2148255: [meta] Make better D8 api.d.o landing page, linked to high-level overview topics, and put it in Core api.php files
I'll be making a stub of the defgroup on that issue; we'll need to fill it in.
Comment #18
jhodgdonWe now have a stub for the block API stuff. It's in file core/modules/system/core.api.php where it says
@defgroup block_api
strangely enough. :)
The idea is for this to be filled in with a couple of paragraphs about how to define blocks in a module, and a link to the page on drupal.org that explains it more thoroughly, as well as @see links to relevant classes/interfaces that people would need to reference.
Comment #19
jhodgdonActually, not @see links. What we need is for the relevant classes/interfaces/etc. to have
added to their doc blocks.
Comment #20
joachim CreditAttribution: joachim commented(Passing thought: could we use wiki pages on gdo to work collectively and incrementally on long documentation texts such as this, prior to making a patch?)
Here's a first stab, drawing from https://drupal.org/documentation/blocks and the CR https://drupal.org/node/1880620
Comment #21
jhodgdonI don't think we really want an extremely long doc page added as a @defgroup. What we want is a long docs page under https://drupal.org/developing/api (and those pages are effectively "wiki" in the sense of "anyone can edit" already), and a short summary in a @defgroup with a link to the longer docs page.
So the text in #20 looks like a good start... I think I would get rid of the last sentence referencing other plugins (we want to keep this short and relevant to blocks), and maybe reverse the order of the sentences about the plugin and the config entity... I guess I would also say that the plugin "does the work of rendering the block" and "builds the plugin-specific part of the configuration form", rather than the text there -- more active and more descriptive of the reason it does what it does rather than describing the implementation details in that overview paragraph?
Comment #22
eojthebraveHere's a stab at rolling in the info from the existing patches in this thread and some additional information and examples. This fills out the @defgroup block_api, and adds a few classes to the group.
I could use some help with the annotation/comment in the example. I can't figure out the correct syntax to have a comment inside a comment and have the API module render it correctly.
And of course both a technical and grammar review.
Comment #23
joachim CreditAttribution: joachim commentedLooks like a good start!
You've just said a Block is two things. This needs to say which of the two are you are now talking about here.
Same here. Is BlockBase a plugin or the config entity base?
A great source of mystery to me is plugin bags. Should we mention those, at least briefly with a @see to the topic on that?
Comment #24
jhodgdonThanks! A few thoughts:
a)
The @ingroup block_api says the same thing as the preceding line. Let's just have the @ingroup.
b)
Please do not remove that blank line.
c) Similar to joachim's review in #23... let's think about the content of the block_api topic...
1. First paragraph:
We aren't really even defining what a "block" is in block_help() currently, which is the help displayed in Drupal by the Help module (maybe we should, but it isn't currently being done). So, I don't really know that we need to define them here in this kind of detail. I think we can make certain assumptions about people reading api.drupal.org, and one of them is probably that they know what a block is? Let's just leave this paragraph out.
2. Next paragraphs... So... Our audience here is module developers, who presumably want their module to provide a block that Drupal's block module can display, right? So ... let's make this topic describe what you actually need to do, and maybe instead of providing a worked-out example here, point people to a drupal.org docs page that does that and/or a class or two in Core that are good examples, and/or the Examples for Developers project (which has a block example).
So I think what all we should really say is (in complete sentences etc.):
- Blocks are examples of Plugins
See the @link plugin_api Plugin API topic @endlink for more information about plugins.
- To define a block, you need to make a class that extends BlockPluginInterface and has Block annotation.
See the @link annotation Annotations topic @endlink for more information about annotations.
- There is a base class that you will probably want to use, to make this easier: BlockBase [I don't think we need to include details about config]
- For examples, see (an example of a Core block without config options and one with config options)
- For more information, see https://drupal.org/developing/api/8/block_api [I added an alias to https://drupal.org/node/2168137] or the Block example in https://drupal.org/project/examples
Comment #25
eojthebraveOkay. Here's a slightly less wordy version that I think does a good job of explaining the steps necessary to implement a custom block in your module as well as linking to other blocks/documents as examples instead of having inline code examples. Thank you both for the feedback.
Comment #26
jhodgdonI think this is much better, thanks! Now we're down to nitpicks... A few thoughts:
In this section:
a) I think the 1st paragraph here could be more easily understood in the context of the bullet list below. So the bullets could be: (1) Create a block plugin class (2) It must implement () (3) usually you will want to extend () (4) It needs to be annotated with (). [schematically], and then we could get rid of the first paragraph here entirely. Does that make sense?
b) @link ... @endlink has to be all on one line, which can go over 80 characters if it has to (but start it on a new line if it won't all fit on the previous line).
c) You talk in the very first paragraph of the docs about configuration entities, and then don't mention them again. So maybe there needs to be a 5th bullet point that explains what you need to do about the config entities, if anything? Either that or maybe say something above that the config entity is something that you don't have to worry about as a developer? Because after reading this I was left thinking, huh, um, what about those config entities then?
d)
Not sure if it needs "The" at the beginning? If it does, the 2nd sentence would also need a "the" after "And", wouldn't it? I think both are better off without "the", myself.
Comment #28
eojthebraveI merged the paragraph about BlockBase into the bullet list and added a little extra text about how BlockBase allows you to interact with the configuration entity associated with the block. Getting warmer. Thanks again for the feedback.
I have no idea why all those tests failed, but let see what happens this time.
Out of curiosity "@link ... @endlink has to be all on one line" is this just part of the doc standards? The API parser handles it spread across multiple lines with no problem.
Comment #29
jhodgdonRE @link/@endlink - standards: https://drupal.org/node/1354#link -- I must have fixed the problem with the API module not recognizing it if it spans two lines. :)
RE: test fails - yeah, looks like a glitch on the bot.
So this is now very close! Just a couple of things:
a)
Can we say something here spelling out that a block plugin is a class, what namespace it needs to be in, mention PSR-4, and put in a link to the OO section (which hasn't been written yet but will be sometime)?
b)
Before "which", you need a comma.
c) next line:
I don't think the comma is good here.
d)
Start the @link on the next line, to avoid going over 80 characters.
e)
This could be combined into one paragraph, or perhaps a bullet list? "Further information and examples:" maybe?
Bullet lists are easy to scan...
Comment #30
eojthebrave"Can we say something here spelling out that a block plugin is a class, what namespace it needs to be in, mention PSR-4, and put in a link to the OO section (which hasn't been written yet but will be sometime)?"
I kind of feel like this was in my original patch but we opted to remove it because we felt it was better to point to examples of existing blocks than to write a full example in the documentation. I've made a couple of wording changes, add "create a class" and that if you want to learn more about how blocks are discovered read up on it in the plugin API topic.
If we want to say more than then I think we should just go back to having a fully fleshed out example honestly.
I cleaned up the other grammar issues. And turned the ending part into a bullet list as per your recommendation. Thanks again for the review.
Comment #31
jhodgdonI think this is great! Let's ship it! Thanks!
Comment #32
webchickPerfect! Love the pointers off to existing code for examples, etc.
Committed and pushed to 8.x. Thanks!