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.

Sign up for GitHub

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

Jump to bottom

Avoid multiline opening tag for single attribute #42

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Avoid multiline opening tag for single attribute #42

wants to merge 7 commits into from

Conversation

UnknownPlatypus
Copy link
Contributor

Opening tags with a single attribute exceeding line width are wrapping but it's not really helping readability and adds a lot of vertical space (especially when nested). I added a bunch of test cases to showcase the formatting causing some churn on my work codebase (see the first commit).

-<div class="my_very_long_class_name_because_bem_or_tailwind_that_should_not_wrap_pls grey_color">
-    <div class="my_very_long_class_name_because_bem_or_tailwind_that_should_not_wrap_pls">
-        <div class="my_very_long_class_name_because_bem_or_tailwind_that_should_not_wrap_pls">
-            Deeply nested string
-        </div>
-    </div>
-</div>  
+<div
+  class="my_very_long_class_name_because_bem_or_tailwind_that_should_not_wrap_pls grey_color"
+>
+  <div
+    class="my_very_long_class_name_because_bem_or_tailwind_that_should_not_wrap_pls"
+  >
+    <div
+      class="my_very_long_class_name_because_bem_or_tailwind_that_should_not_wrap_pls"
+    >
+      Deeply nested string
+    </div>
+  </div>
+</div>

This patch try to keep an opening HTML tag with a single attribute on a single line when applicable. This was really successful for me on my work codebase (~2k html files, 250k LOC, using mostly BEM classes or tailwind classes) with over 300 files fixed with clear readability wins!

Prettier suffer from the same issue (see prettier/prettier#10918 (comment)) and because of that, the tailwind team (and others) are using some crazy printwidth to workaround this behavior.

It could maybe be behind a configuration flag like keepSingleAttrOnSingleLine or any other better naming you find. Let me know if I should add such flag.

UnknownPlatypus
UnknownPlatypus commented Aug 13, 2024
View reviewed changes
markup_fmt/src/printer.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll suggest to hide whitespace and use split view to review this file.
Most of the change is just indentation added by the match block
image

@g-plane
Copy link
Owner

g-plane commented Aug 14, 2024

This must be behind a configuration option, but I'm not sure its naming.

@UnknownPlatypus
Copy link
Contributor Author

This must be behind a configuration option, but I'm not sure its naming.

Maybe preferSingleLineOpeningTag /preferSingleLineTag because the same type of behavior could be nice for tags with no attributes at all. The flag could cover both cases.

For example, markup_fmt currently wrap tags for the following snippet with small print width and I'd like to do a followup PR to avoid that and keep the snippet unchanged.

-<html>
-  <body>
-  </body>
-</html>
+<html
+>
+  <body
+  >
+  </body
+  >
+</html
+>

@UnknownPlatypus
Copy link
Contributor Author

UnknownPlatypus commented Aug 15, 2024 •
edited
Loading

I added a configuration option and updated the tests accordingly. I went for preferSingleLineOpeningTag for now, let me know if you want something else when you find a good naming for such option. I'll update the PR.

I set the default value to the current behavior, we could eventually do the opposite if you want to promote the new behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@UnknownPlatypus @g-plane