a-h / templ

A language for writing HTML user interfaces in Go.
https://templ.guide/
MIT License
8.19k stars 267 forks source link

fmt: preserve spacing in templates to allow for organizing code #374

Open andradei opened 9 months ago

andradei commented 9 months ago

They are useful to organize HTML tags, but the formatter removes all empty lines.

Example:

This

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>
}

becomes

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>
}

It would be great if at least 1 blank line was kept on those situations.

Environment

axispx commented 9 months ago

+1 It would help with the readability of the code when I'm working with a div soup πŸ˜„

joerdav commented 8 months ago

I've set this as needs decision, I personally don't have any reservations against this, open to other opinions.

dropwhile commented 8 months ago

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.

andradei commented 8 months ago

I think the solution is rather simple: If a dev wrote empty lines that gofmt would preserve, it's safe to assume they expect to keep them after save. If the don't like them, they'd be writing .templ files without those empty lines. Making the formatter similar to gofmt would keep things consistent and meet already set expectations and all cases/tastes are covered.

joerdav commented 8 months ago

What about if duplicate new lines were removed?

<div />

<div />

formats to:

<div />

<div />
a-h commented 8 months ago

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> tag (see https://hypermedia.systems/a-web-1-0-application/).

        <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.

andradei commented 8 months ago

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.

Would we want to leave that as-is?

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.

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 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.

joeyak commented 8 months ago

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.

leaanthony commented 7 months ago

+1 for this! It would be really great if we could disable the formatter, so that Render just output the rendered template as is. This would work really well for testing, especially snapshot testing.

joerdav commented 7 months ago

@leaanthony I think the conversation here is mainly talking about the templ fmt command, rather than how the HTML comes out at the end. If you'd like to discuss more then it could be worth a new issue, though for snapshot testing I'd recommend just formatting the HTML after rendering, using something like https://github.com/a-h/htmlformat this is what we do in the templ tests!

leaanthony commented 7 months ago

Oh perfect! Thank you for replying πŸ™ Also, what a fantastic project! πŸš€

smithamax commented 6 months ago

I think the goal of a formatter is to be consistent, but by retaining whitespace, you end up with oddities.

I think following the same rules as gofmt would be more consistent, I can group lines in a func, why not in a templ. I guess a templ is a single expression so different, but I still want to break up my code

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.

This seems equivalent to arguing that instead of whitespace in go code you should just create a new block ({ ... }), but this would be noisy and change the code structure when all we want to do is make the code more pleasant to read

AlexanderArvidsson commented 6 months ago

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 gofmt also follows this rule, and prettier works for HTML which the standard go template package uses. It only makes sense the same comparison is valid with Templ.

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. If the formatter can respect newlines outside of the templates inside .templ files, it should respect newlines inside the templates as well.

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.

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. I can't think of a single time where I've purposefully tried to align my HTML code with the logical grouping (as in, with newlines), visually. It's actually impossible to do in many cases, because CSS controls the layouting (mostly). If you use Flex, you shouldn't try and make your HTML code be positioning the same way as Flex, right?

andradei commented 4 months ago

Pinging to see if we're close to a solution here. I'm leaning even more towards respecting the white-space and blank-lines the user might add (knowingly or not :smile: ). Editors nowadays remove trailing white-space and if someone wants to write some horrendous code with white-space and blank-lines alone, I think that should be preserved for everyone to see (including the programmer in the future :laughing:).

And perhaps add something similar to gofmt in the future.

joerdav commented 4 months ago

@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.

AlexanderArvidsson commented 1 month ago

Hello hello, I've done some work on this and would appreciate if @joerdav and/or @a-h takes a look at my PR. It will collapse multiple newlines into a double-newline (so you preserve 1 empty line between elements).

I'm sure I could add way more tests for this, but this shouldn't affect any existing codebase. It could be good to try running my fork on a big codebase and see what changes Git reports! Running it on my own (small) codebase showed no changes, but the idea is that this is backwards compatible (as in, it shouldn't modify people's existing code). This is however hard to guarantee just via unit tests.