Introduce #type 'links'

sub

Hell yes! :D I love love love this idea.

One other option is in http://drupal.org/node/1295958. Dunno.

@dozymoe, I like generalized solutions as much as anyone, but as you point out in comment #10, on #1295958: Arguments to theme as renderable array in a render array. Allow better nesting of html element., the code there could really lend itself to noticable performance issues for deeply nested use of render arrays.

On the other hand, in the case of links, where often lists of links are multi-level -- I'm wondering how this solution differs from that one. It looks potentially like by limiting this to a single #links option, second and third level links would have to be pre-rendered into html as their own list of links, before being added to the parent (which well could solves the performance problem of nested arrays of hell).

There is really good info here. Tagging for visibility and consideration in theme cleanup

Let's fix the @todo in the patch

Related #1828536: Rename 'type' variable of theme_item_list() to 'list_type'

node_page_default is removed in #1806334-168: Replace the node listing at /node with a view so I exclude it form the patch. Added new @todo.

Related #2005970: In renderable arrays, #type should provide a #theme suggestion, similar to how drupal_prepare_form() works

also related #2025629: [meta] Ensure that all #theme/#theme_wrappers hooks provided by core are associated with a sensible, re-usable element #type

The patch no longer applies for me:

error: patch failed: core/modules/system/system.module:545
error: core/modules/system/system.module: patch does not apply

reroll

theme_item_list() does not render the #children property, so it's not suitable for #theme_wrappers.

Apart from that I think we should be moving this towards the approach from #2025629: [meta] Ensure that all #theme/#theme_wrappers hooks provided by core are associated with a sensible, re-usable element #type.

Working on this.

Removing "Needs reroll" tag.

And now for real.

The focus of #2025629: [meta] Ensure that all #theme/#theme_wrappers hooks provided by core are associated with a sensible, re-usable element #type is on existing render arrays that declare a '#theme' property but not a type. I'm going to compile a list of files to deal with.
Replacing direct calls to theme() with render arrays is dealt with elsewhere and should probably be left out here.

$ grep -R --files-with-matches "'#theme' => 'links'" core
core/modules/block/lib/Drupal/block/Plugin/PluginUI/BlockPluginUI.php
core/modules/node/lib/Drupal/node/Plugin/views/area/ListingEmpty.php
core/modules/system/lib/Drupal/system/Tests/Theme/FunctionsTest.php
core/modules/system/tests/modules/ajax_test/ajax_test.module
core/modules/views/views.api.php
core/update.php

From the above files only views.api.php was left untouched.

Removed the #theme property.

The patch in #25 looks good from the perspective of #2025629: [meta] Ensure that all #theme/#theme_wrappers hooks provided by core are associated with a sensible, re-usable element #type.

I don't think it addresses @sun's concerns in the issue summary and subsequent comments though.

The very fist patch, in the issue summary introduces a new theme function that can be used in #theme_wrappers - although I'd prefer to call it theme_links_wrapper than theme_wrap_links to be more consistent with similar functions elsewhere. Could we try to incorporate that idea, or the #pre_render function proposed in #5 into #25?

I don't think we should be looking into anything like #2 and #4 here though.

The main reservation I have with the idea of any kind of processing step that comes after the render step (like #pre_theme_wrappers) is that I don't see (and nobody has ever explained to me) how that processing could ever be compatible with the "drillability" idea #2008450: Provide for a drillable variable structure in Twig templates, in the same vein as #2009132: Remove #pre_render and #post_render from drupal_render() as it prevents proper Twig drillability.

At this point I'm not sure if that means "drillability" is just a fantasy we've made up recently or a criticism of arbitrarily interleaving multiple render and processing steps.

Regardless, IMO this issue shouldn't be introducing any new rendering/processing steps into drupal_render(), that kind of thing needs discussion, tests written and to have its own issue. If we can address @sun's problems with #theme_wrappers or #pre_render then great, otherwise I think #25 will have to do as a first "baby step" for now and we can split out some follow-up issues to address whatever is getting in the way.

Or... maybe we shouldn't be trying to render the individual links at all because in general I believe we're supposed to be as lazy as possible about calling drupal_render() so we can keep our data as structured data for as long as possible rather than "flattening" them into strings...

In which case, #25 is probably exactly what we should be doing here.

Related: #311011: Replace links.html.twig with item-list--links.html.twig. Haven't had the time to really look into it, but fixing that one first might get us towards a much, much more interesting implementation of #type links.

#25: 1300744-25.patch queued for re-testing.

Working on this one during today's DrupalItalia code sprint

Removed the part of the patch related to the files which have been removed from the core (Issue #2056513). Extended the patch to all the classes in core which uses the links.

Related to the comment #32

Looking good guys.

I think you missed one though.

core/modules/system/system.module:547:    '#theme' => 'links',

that wasn't need work... I miss the preview button.

Should it be changed back to Reviewed & tested by the community?

Yes, sorry. Everything looks good.

A new #type links should not only resolve the problems stated in the OP, but also incorporate the fact that each rendered child is just a list item; i.e., the #pre_render should drupal_render() each child element and then pass off the rendered children into theme_item_list().

Item lists are a Twig template not a theme function now. Could we avoid touching drupal_render() in #pre_render if at all possible?

This patch actually abstracts the theme calls in all theses places into the #type so it could have a follow up to replace how its rendered or makes removing theme_links easier by having a central location. I still think this is RTBC.

Well, we need some kind of consensus, but it sounds like either way we're due for an issue summary update.

Adding parent relationship.