Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why this one line tag with multiple attributes was changed? #12

Open
idgserpro opened this issue Aug 2, 2019 · 5 comments
Open

Why this one line tag with multiple attributes was changed? #12

idgserpro opened this issue Aug 2, 2019 · 5 comments

Comments

@idgserpro
Copy link
Member

From https://zopetoolkit.readthedocs.io/en/latest/codingstyle/zcml-style.html#attributes:

If all the attributes fit on one line with the tag name, do that:
<class class=".foo.Foo">

But as shown in plone/plone.api#219 (comment):

 <!-- Set overrides folder for Just-a-Bunch-Of-Templates product -->
  <include package="z3c.jbot" file="meta.zcml" />

was changed to:

  <!-- Set overrides folder for Just-a-Bunch-Of-Templates product -->
  <include
      package="z3c.jbot"
      file="meta.zcml"
      />

Since "If all the attributes fit on one line with the tag name, do that" is already met, why the line was changed? I'm not sure if this is really a bug...

@ale-rt
Copy link
Member

ale-rt commented Aug 2, 2019

zpretty does always write one attribute per line which is in contrast with the zcml style.
Probably having generally all the attributes on one line if they fit some reasonable length is a good idea, but for the moment I have no spare time to provide a patch for that.

@idgserpro
Copy link
Member Author

Probably having generally all the attributes on one line if they fit some reasonable length is a good idea,

I think the length could follow zcml style itself:

If all the attributes fit on one line with the tag name, do that:

The ZCML style guide has been developed with the following qualities in mind:

Lines under 80 characters wherever possible

@jensens
Copy link
Member

jensens commented Jun 24, 2021

Given to have easy diffing support, I would keep them on single lines.

@ale-rt ale-rt added the prio:low label May 8, 2022
@goyalyashpal
Copy link

Given to have easy diffing support, I would keep them on single lines.

i was of similar opinions, unless i saw that it quickly becomes very difficult to read in an example like below.
also, the opinionated black-formatter (aimed at easy diffing) for python does the same; till long-line if possible, else one per line

      <div class="collapse navbar-collapse" id="navbar">
          {% if session["user_id"] %}
          <ul class="navbar-nav me-auto mt-2">
            <li class="nav-item">
              <a class="nav-link" href="/quote">Quote</a>
            </li>
            <li class="nav-item">
              <a class="nav-link" href="/buy">Buy</a>
            </li>
            <li class="nav-item">
              <a class="nav-link" href="/sell">Sell</a>
            </li>
            <li class="nav-item">
              <a class="nav-link" href="/history">History</a>
            </li>
          </ul>
          <ul class="navbar-nav ms-auto mt-2">
            <li class="nav-item">
              <a class="nav-link" href="/logout">Log Out</a>
            </li>
          </ul>
          {% else %}
          <ul class="navbar-nav ms-auto mt-2">
            <li class="nav-item">
              <a class="nav-link" href="/register">Register</a>
            </li>
            <li class="nav-item">
              <a class="nav-link" href="/login">Log In</a>
            </li>
          </ul>
          {% endif %}
        </div>

@jensens
Copy link
Member

jensens commented Dec 4, 2023

Black allows some control here, the magic trailing comma: https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#the-magic-trailing-comma

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants