Better class attribute rendering

[GeertDD]

When setting the firstClass and/or lastClass options to null or an empty string, like this:

{{ knp_menu_render('AcmeDemoBundle:Builder:mainMenu', { 'firstClass': null, 'lastClass': null }) }}

A menu with the following class attributes is rendered:

<ul>
    <li class="current ">...</li>
    <li class="">...</li>
</ul>

Even though this is completely valid markup, it would be nice if the trailing space after the class current would be gone, as well as the empty class attributes:

<ul>
    <li class="current">...</li>
    <li>...</li>
</ul>

[Jbekker]

I agree on this, I wouldn't mind to create a pull request to fix this. I think all empty attributes shouldn't be printed, right?

Editing the attribributes macro in Resources/views/knp_menu.html.twig to something like I did below is al we need to do:

{% macro attributes(attributes) %}
{% for name, value in attributes %}
    {%- if value is not empty -%}
        {{- ' %s="%s"'|format(name, value is sameas(true) ? name|e : value|e|trim)|raw -}}
    {%- endif -%}
{%- endfor -%}
{% endmacro %}

[stof]

If the value of an attribute is null or false, it is already omitted.

However, the class attribute is indeed not handled properly when empty as string concatenation will still produce an empty string

[dbu]

i guess in version 2, the tricky bit is here: https://github.com/KnpLabs/KnpMenu/blob/master/src/Knp/Menu/Resources/views/knp_menu.html.twig#L78 or https://github.com/KnpLabs/KnpMenu/blob/master/src/Knp/Menu/Resources/views/knp_menu.html.twig#L5

@Jbekker @GeertDD is one of you up to write a pull request to fix this?