-
-
Notifications
You must be signed in to change notification settings - Fork 289
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
fmt: preserve spacing in templates to allow for organizing code #374
Comments
+1 It would help with the readability of the code when I'm working with a div soup 😄 |
I've set this as needs decision, I personally don't have any reservations against this, open to other opinions. |
I ran into this issue as well, and the default formatting was a bit unexpected. I too would prefer newline preservation. Perhaps it could be an option, similar to how the vscode html formatter has the "HTML>Format: Preserve New Lines" toggle (which I believe is enabled by default)? This would allow people the option for either, and help prevent code churn for those that are fine with the existing formatting. |
I think the solution is rather simple: |
What about if duplicate new lines were removed?
formats to:
|
I think the goal of a formatter is to be consistent, but by retaining whitespace, you end up with oddities. templ Form() {
<a href="event/new">
+ Create
</a>
<form method="post" action="/event/new">
<label for="title">Title</label>
<input id="title" name="title" type="text" required class="border rounded-lg block"/>
<label for="date">Title</label>
<input id="date" name="date" type="date" required class="border rounded-lg"/>
<button type="submit">Create</button>
</form>
} Would we want to leave that as-is? There's two pairs of label/input, but they're not grouped. And there's an extra line at the end that doesn't add any value. I think that if we're adding line breaks to show logical grouping, then we're actually missing a HTML element that makes that grouping explicit. For example, the Hypermedia book groups with a <p>
<label for="first_name">First Name</label>
<input name="first_name" id="first_name" type="text" placeholder="First Name" value="{{ contact.first or '' }}">
<span class="error">{{ contact.errors['first'] }}</span>
</p>
<p>
<label for="last_name">Last Name</label>
<input name="last_name" id="last_name" type="text" placeholder="Last Name" value="{{ contact.last or '' }}">
<span class="error">{{ contact.errors['last'] }}</span>
</p>
<p>
<label for="phone">Phone</label>
<input name="phone" id="phone" type="text" placeholder="Phone" value="{{ contact.phone or '' }}">
<span class="error">{{ contact.errors['phone'] }}</span>
</p> I introduced a formatter into templ because in my commercial work, developers write HTML in all sorts of tortured ways, with some adding loads of pointless whitespace, others putting every attribute on a newline etc. etc. I'm up for making the change to not remove extra whitespace if there's broad support, but if a silent majority are happy (or don't care), then I'd like to keep it, since there's value in the consistency that removing non-essential whitespace brings. |
Thanks for being willing to make changes and for explaining the abuses to HTML files people make on your experience... I'm glad I don't run into that often. It would be quite the lengthy task to have a formatter for sane grouping. Imagine taking into account every possible combination of HTML tags that can be grouped.
I agree with @joerdav and have a strong opinion that "yes," keep it as is with one small change: respect 1 blank line but remove any subsequent ones.
I kind of agree with the pointless whitespaces. But attributes on newlines could be allowed after some arbitrary number (like 5 attributes) or if one line would result in a certain length (maybe 80). I'd propose this ticket takes care of the whitespace formatting only and another issue is created for attribute formatting, tag grouping, etc., if those things are worth discussing. |
It would be nice if it were an option for the formatter. I like how it compacts everything, but tbh it wouldn't be bad if it followed gofmt and preserved spaces as long as it's consistent. |
+1 for this! |
@leaanthony I think the conversation here is mainly talking about the |
Oh perfect! Thank you for replying 🙏 Also, what a fantastic project! 🚀 |
I think following the same rules as gofmt would be more consistent, I can group lines in a
This seems equivalent to arguing that instead of whitespace in go code you should just create a new block ( |
I'd like to chime in here to +1 the comment @andradei, where if there's a newline, it most likely is due to the developer wanting the organisation. Me, personally, use single newlines to group relevant code in my HTML (React in reality, which for me to be readable needs grouping due to the mixed conditionals and HTML). The standard formatter for most modern (JS) web development is Prettier, which does respect single newlines. It collapses multiple newlines into a single one, which I believe is the absolute correct way to go. I'm not trying to compare a JS formatter with a Go formatter, but Finally, @smithamax brings up the most important point (in my opinion); consistency. Templ is inconsistent even within itself: func getFullName(store *Store) string {
user := store.Get("user") // Newline below was kept
return user.FirstName + " " + user.LastName
} // Even the newline below here was kept
templ page(store *Store) {
<div>
<span>Welcome,</span> // Newline below was removed
<span class={ "bold" }>{ getFullName(store) }</span>
</div>
} The formatter keeps the newline (a single newline, collapsed if multiple) in the function, but it removes the newline in the template. From my perspective, this inconsistency should be enough to argue that something needs to change.
I've never heard this argument in my career, but it is an interesting topic for sure. Since the HTML specification is odd how it considers the formatting of HTML, I wouldn't consider this relevant though. When you make a newline in HTML, the specification ignores all newlines and whitespacing (because of indentation), meaning how you format your code is only relevant within a single line in HTML. |
Pinging to see if we're close to a solution here. And perhaps add something similar to |
@a-h It seems we have a majority of users who are wanting to have whitespace preserved, or atleast collapsed to a single line. I'll move this to the implementation stage. |
Hello hello, I've done some work on this and would appreciate if @joerdav and/or @a-h takes a look at my PR. I'm sure I could add way more tests for this, but this shouldn't affect any existing codebase. |
They are useful to organize HTML tags, but the formatter removes all empty lines.
Example:
This
becomes
It would be great if at least 1 blank line was kept on those situations.
Environment
The text was updated successfully, but these errors were encountered: