Mention in the comments that menu_name is only global available in menu.html.twig

Thanks for creating this issue. One thing to note is likely you wouldn't be using that menu_name class on every ul/li. So more practically speaking you should use it outside the macro surrounding the call to the macro.

eg.

<menu class="{{ menu_name }}">{{ menus.menu_links(items, attributes, 0) }}</menu>

Or better:

<menu{{ attributes.addClass(menu_name|clean_class) }}>{{ menus.menu_links(items, attributes, 0) }}</menu>

Do you have a proposal for how the comment should read? I'd like to have something short and to the point that can explain that since it's really the only macro used in core (except for the variations for book and toolbar which do the same thing).

Yeah this is a tricky one, we don't want to confuse things either. I would lean towards more of a message saying that menu_name isn't passed into the macro by default. There are three places that you'd need to change to make menu_name available in all levels of the macro so not sure how we explain that.

Or what if we just pass menu_name into the macro so it is available? Would that break anything? Macro arguments are optional so seems like it would be fine…

@4aficiona2 if you're up for it I think uploading a patch changing all the menu and menu-like templates to include menu_name as relevant would be a good next step. We can at least see if any of the core automated tests fail as a result of that, and go from there.

Don't know if we are leaning toward just updating the comment (see attached). As a pseudo novice to the theme layer (and not finding documentation specifically on it), where are the twig macros to modify?

And whoops. My comment patch was supposed to be geared more toward the 'not by default' response. Updating.

@laughnan re #11 the macro is the function looking thing in that file.
http://twig.sensiolabs.org/doc/tags/macro.html

We've opted to keep it in the menu.html.twig and self import it as to avoid finding some generic place to put it and still have it available to the theme system template overrides.

The documentation patch will be easy to change in > 8.0.x. If we actually change the macro that will be 8.1.x material and may not be worth your while as the use-case for it may be slim(I could be wrong here). I'm completely on board with clear documentation to help people out though.

Adding it to the macro might be more of a curiosity at this point, but also interesting to think about best practices for macros and how much variables should we be passing in. Or should we always pass in the _context like the docs show, that's another option.

My new idea for the docs side is to use @code to show how the macro would need to be changed. As I said three lines need to be changed and I don't think trying to explain that in words will be effective.

Thanks for that @joelpittet. Forever learning, forever learning...

Aren't we all;)

I've added a patch which passes the menu_name into the macro, as mentioned in #8.
Please review.

Test should be performed using Stark theme because fix has already been applied to Classy, which rolls down to Bartik.

Test passed.

@radubutco I don't think we should change this. My thought is that it's not common need to add the menu name to all ul/li/a and that could cause some CSS cascading issues in styles due to the recursiveness of this macro. So I'd much rather like the title says, add notes to why it's not available.

To inform those who are arrive here because of facing the problem of using menu_name I wrote a blog post related to the problem (and using the information) of this issue: https://medium.com/integral-vision/drupal-8-twig-add-custom-css-classes-...

Based on joelpittet's comment in #20, The scope of this issue should be to only add a comment in the twig template that explains that {menu_name} can be added to the Twig call. The patch should not actually attempt to change the default Twig code.

Novice tasks:

• The Issue summary should be updated, and
• a new patch should be created.

@example_mentee is then comment/patch #12 sufficient?

I don't think the patch in #12 is sufficient to explain the actual problem.

This patch adds the comment where the problem exists, inside the macro itself.

Hmm… http://twig.sensiolabs.org/doc/tags/macro.html uses the term "arguments" instead of "parameters". We should keep the terminology the same. Here's a corrected patch.

@JohnAlbin Maybe it would be nice to help newbies by mentioning that we are thinking of the variables listed in the intro comment of the file. Or is it obvious?

Add test against 8.2.x on #27

This patch seems good to me and apply without problems

+++ b/core/modules/system/templates/menu.html.twig
@@ -29,6 +29,11 @@
+    access aditional variables, add more arguments to this macro; or see

s/aditional/additional/

Perhaps we should add to this documentation that already exists in these files?

{#
  We call a macro which calls itself to render the full tree.
  @see http://twig.sensiolabs.org/doc/tags/macro.html
#}

I'm actually not sure what our policies are on this, but I wonder if we should consider creating docs on drupal.org and linking to them from these files? It would be easier to provide sample code and explain recursion, scope, etc. in that format, rather than a small inline comment.

Patch to change aditional for additional.

Perhaps we should add to this documentation that already exists in these files?

{#
  We call a macro which calls itself to render the full tree.
  @see http://twig.sensiolabs.org/doc/tags/macro.html
#}

@Cottser as you said on #32 the documentation already exists in these files.
We should change the latest patch to remove the extra documentation and finally do something like this?:

+++ b/core/modules/system/templates/menu.html.twig
@@ -29,6 +29,11 @@
+  {#
+    This macro can only access the variables passed to it in its arguments. To
+    access aditional variables, add more arguments to this macro.
+  #}

@dimaro thanks! I'm suggesting we consider expanding that existing documentation. So not removing documentation but adding/combining with the existing docs in these files.

@Cottser thanks for the explanation.
It may be interesting to include all the information.
However, I think that we should update the scope of this issue to do that, right?

This issue came up and was discussed in a #documentation group triage session.

We think this issue is really about the concept of what a macro is and an initial confusion about how they work. There is readily discoverable information both in Twig's docs and the Drupal Wiki (https://www.drupal.org/docs/theming-drupal/twig-in-drupal/macros-in-twig...) about what a Twig macro is and how to use it.

If anything, examples from menu's template files could be added to the Drupal Wiki page on macros.

Adding documentation to every usage of macro (there are 12 at last count) in Drupal core template files creates an undue maintenance burden.

Closing as "won't fix" with an encouragement to review and edit (if necessary) the Drupal Wiki page on the subject: https://www.drupal.org/docs/theming-drupal/twig-in-drupal/macros-in-twig...

Thank you all for your input and work on various patches.