Improve Classy template in-line documentation

Would this be just Classy? The issue summary makes it sound so, so updating the component.

I think if we add the example text to Classy we should really think about adding it to the core templates as well. They both have the same problem. Also, changing component to Documentation so docs team people are more likely to see it.

+1!

I'm only referring to the example usage text (where you can see this template in action.) That part is useful to both sets of templates. If it is useful for one, and we define a standard, there shouldn't be anything stopping us from using it for both since the text itself would be the same.

David Hernandez put a tell in IRC to ask me to comment on the parent issue; I'm assuming the comments are requested here?

On the parent issue, I see two options being proposed.

Comment #143:

{#
/**
 * @file
 * Two column template for the block add/edit form.
 *
 * Original template: core/themes/classy/templates/admin/block-list.html.twig
...

(adds the "original template" line)

And comment #144:

...
 *
 * @see /core/modules/block/templates/block-list.html.twig
 *
 * @ingroup themeable
 */

(instead adds @see near the end)

Here are my thoughts:

a) There's already a lot of duplicated information in theme templates... I have been meaning to bring this up for a while actually. What I mean is that for example if you're looking at the Classy version of block-list.html.twig, it has the same docs as the Core version of the same template. This seems ... tedious, silly, and hard to maintain, at best.

b) @see does not currently work with full file paths, so the proposal of using @see will probably not work well in practice. If you're on api.drupal.org, however, you'll already see links at the top of each template page to other templates with the same end file name. For instance, look at the top of:
https://api.drupal.org/api/drupal/core!themes!classy!templates!block!blo...
where it has links to the two versions, in core/themes/classy/templates/block, and core/modules/block/templates

c) Note that the existing .html.twig files, at least most of them, are in violation of our current docs standards, especially the first line:
https://www.drupal.org/node/1354#templates

d) The goals here are:
- Make sure themers have the information they need: variables available, what the base template is
- Make sure Core developers do not have a huge burden to maintain docs

I have some suggestions about how to resolve this, but need to run so I will add them in a separate comment.

That makes sense to me, regarding the duplication of docs.

And thanks for reminding me I didn't get around to my additional comment -- time got away today.

So. given #9 and #8, what I'd like to suggest is:

-----
a) Templates in core/modules should be fixed so that their first lines comply with our current standards:
https://www.drupal.org/node/1354#templates
Basically they should look like this:

/**
 * @file
 * Displays a list of forums.
 *
 * Available variables:
(list of the variables)
 *
 * @see template_preprocess_forum_list()
[links to any other preprocess functions if necessary]
 *
 * @ingroup themeable
 */

The current first lines are more like "Default theme implementation for the administrative toolbar. " This is not horrible but isn't according to our current standards, which would make this "Displays the administrative toolbar.", which is shorter.

b) A Core base theme template that overrides a core template directly (same filename) would be the same, but it would have an additional line, so the top would look like:

/**
 * @file
 * Displays a list of forums.
 *
 * Overrides core/modules/foo/templates/foo.html.twig.
 *
 * Available variables:

In addition, the @themeable line would ***NOT*** be included, because that should only be on the main core implementation.

c) A Core non-base theme would have an abbreviated header:

/**
 * @file
 * Displays a list of forums.
 *
 * Overrides core/modules/foo/templates/foo.html.twig. See that file for complete
 * documentation.
 */

d) If a theme (base or non-base) has a specific override that uses theme suggestions, such as defining a block--foo.html.twig file for specific foo-types of blocks:

<code>
/**
 * @file
 * Displays a foo block.
 *
 * Theme suggestion for block.html.twig. See that file for complete documentation.
 */

And in this case, the suggestion file would hopefully be in the same directory as the main template.

-----

Thoughts? Let the bikeshedding begin!

#10 looks pretty good to me. We keep the variables documentation, but remove the @themable and preprocess link, which might be confusing to people anyway.

Do we have leeway to add paths to the description. This is one thing we were looking to do, because finding a place where the template is actually used can be difficult for the more obscure ones. For example, I encountered this recently when working on the indentation template. It's description would be something like "Displays a set of indentation divs." but that doesn't tell you where you can see an example of the template in action. One of the only places to see it used is on the page to admin menu links, which took a little searching.

In the parent issue I had this line:

Example usage: Drag and drop table indentation (admin/structure/menu/manage/admin)

Could we do something like this, or work that concept into the description?

The one-line description needs to stay under 80 characters, but of course you can add additional documentation. I didn't put that in the example... so how about this:

/**
 * @file
 * Displays a list of forums.
 *
 * Optional additional documentation, such as a path where this is used, or further
 * description of what it does.
 *
 * Available variables:
 --- (list of the variables) ---
 *
 * @see template_preprocess_forum_list()
---  [links to any other preprocess functions if necessary] ---
 *
 * @ingroup themeable
 */

That works for me. Thanks, Jennifer! Any other thoughts?

I think we should wait until the Classy templates are re-organized, which should be soon, and then think about setting up a meta/sprint to redo all the template docs.

No, I don't really have any other thoughts. The proposals in #10/#12 are basically "Follow our existing standards". We already have a precedent for "Document it once and provide links" for other stuff, so the suggestions there are just sort of a concrete template that can be followed easily. I don't think we need to have a formal "standards" discussion about it -- so yes let's turn this into a Meta when the reorg is done, and update the existing templates so they fit with these templates.

I suppose we should put these into the issue summary, after a couple of days to see if anyone else has ideas/discussion/comments?

This is looking good to me, thank you all especially @jhodgdon :)

RE #16:

The only thing to remove from Classy is the @ingroup line. Nothing else. A line is added. See #10, which spells this out in item (b).

Also the formatting/indentation in #16 is not really useful. Keep in mind this documentation is parsed and displayed on api.drupal.org. Indenting lines within a paragraph doesn't do anything useful there. So this isn't going to be effective, and it goes against our general docs guidelines:

* Example usage: The thing that does stuff at
*   admin/structure/menu/manage/admin.

I also don't think we want to suggest either that every theme template needs example usage. Like for instance table.html.twig or block.html.twig -- we don't need to provide examples for where those are used.

So... I'm not a big fan of the proposal in #16. I think we should stick with #10/#12.

Note that the code I posted was not intended to be two lines, if that is where that started. The line wrapped.

So I noticed that some of what we are discussing is already in use, just not often. For example, this example text is in the user template.

/**
 * @file
 * Default theme implementation to present all user data.
 *
 * This template is used when viewing a registered user's page,
 * e.g., example.com/user/123. 123 being the user's ID.
 *

Like for instance table.html.twig or block.html.twig -- we don't need to provide examples for where those are used.

Agreed.

RE #16 I don't think we should have two override comments. It's likely confusing. I would only add the direct parent override.

Just as a general note, while I'm generally in favor of standards, the more we specify as standards, the less room there is for common sense. So, I don't think that we should say in our standards something like "You need to give an example of where the template is used", because we probably don't need it in all cases. I also don't think we need to say that a description is "strongly encouraged", because in a lot of cases we don't need that either (for example, block.html.twig -- how much description do you need beyond the one-liner that should tell you it outputs a block?).

So. Let's keep the examples simple.

Here's one more proposal that hopefully contains all the best ideas from the last few posts:

a) Core base template in a core/modules/foo/templates directory:

/**
 * @file
 * Displays a list of forums.
 *
 * Optional description, which may include something like:
 * Used on admin/foo/bar to display the baz.
 *
 * Available variables:
(list of the variables)
 *
 * @see template_preprocess_forum_list()
[links to any other preprocess functions if necessary]
 *
 * @ingroup themeable
 */

b) Override of a base template, when it's in a base theme like Classy:
This should completely copy the base theme template's documentation, and then:
- Omit the @ingroup themeable line at the end.
- Add a line just below the one-line summary that links you to the base theme. So it would look like this at the top:

/**
 * @file
 * Displays a list of forums.
 *
 * Overrides core/modules/forum/templates/forum-list.html.twig.
...

c) Override of a base template, when it's in a non-base theme:

/**
 * @file
 * Displays a list of forums.
 *
 * Overrides core/modules/forum/templates/forum-list.html.twig. See
 * the base template for complete documentation.
 */

d) Override of a template using theme suggestions, whether it's in Classy or another base theme:

/**
 * @file
 * Displays a foo block.
 *
 * Theme suggestion for block.html.twig, used when the block ID is 'foo'.
 * See the base template for complete documentation.
 */

The assumption here is that the base block.html.twig file is in the *same directory*, so whether this is in Classy or Bartik, we should not need to duplicate all the information in block.html.twig here.

Thoughts?

I'm liking #19 overall, just want to mention that #19.d is not necessarily a safe assumption. For example Bartik overrides field--taxonomy-term-reference.html.twig but not field.html.twig.

And @jstoller/#16 I see where you're coming from with having the reference to the parent template at the bottom, but I think having it below the summary matches much closer with our preprocess standards which makes for some nice "symmetry" IMO: https://www.drupal.org/node/1354#themepreprocess

Good point in #20 about the overrides... odd, I am pretty sure in D7 you could not override just a theme suggestion for a template without also overriding the base template. 99.9% sure actually -- I got bitten by this more than once in D7. So this is possible to do in d8?

If it is, then I guess maybe we should make (d) in #19 be something like:

After those few lines, if it's in a base theme and the base template is not also overridden in this theme (block.html.twig in this example, continue with the full documentation from the base template. Omit this in non-base themes and omit it if the base template is present in the same directory.

How's that for an idea?

I am pretty sure in D7 you could not override just a theme suggestion for a template without also overriding the base template

That was true, but fixed in D8.

It's documentation so it should be RC eligable.

If we're agreed on #19 with the addition of #21, then we should probably update the issue summary.

Classy is removed from Drupal core. I am moving this to the contrib project

Wonder if this is still needed actually? Classy shouldn't be the starter point for any theme, instead the starterkit_theme should be used.