templ icon indicating copy to clipboard operation
templ copied to clipboard

Inconsistent spacing of elements when next to an if statement

Open joeyak opened this issue 1 year ago • 4 comments

templ version: 0.2.476 go version: go version go1.21.5 windows/amd64 go playground example: https://go.dev/play/p/Ou5v3iN86J9

I've run into a case where having an if statement on an element will result in whitespace being written to only one side. This messes with the spacing of pages.

With this templ file

package main

templ page(doIf bool) {
	<button>Left</button>
	if doIf {
		<button>Middle</button>
	}
	<button>Right</button>
}

the output is

<button>Left</button> <button>Middle</button><button>Right</button>

when one of these two is expected

<button>Left</button><button>Middle</button><button>Right</button>
<button>Left</button> <button>Middle</button> <button>Right</button>

image

joeyak avatar Dec 22 '23 17:12 joeyak

Thanks for the truly excellent bug report.

I haven't had chance to test this on the new version I released today, but I think it might be fixed in https://github.com/a-h/templ/commit/83497bcd43fc5bfa814146f88eecd20004e79510

The 'fmt' process was inserting an extra space.

If you have time to re-test before I do, great. Or, I will investigate soon. 😀

a-h avatar Dec 22 '23 21:12 a-h

I've tested it with a new version, and it still persists.

joeyak avatar Dec 22 '23 21:12 joeyak

I think that bug fix should really only effect the formatting of attributes on multiple lines. It seems the generator isn't spacing the elements correctly. Gonna take a swing at seeing if I can find the issue and fix

JonnyLoughlin avatar Dec 23 '23 16:12 JonnyLoughlin

I forgot to update my fork before throwing together this rough demo commit of what a fix would look like but I was testing on the most recent commit and the new test passes correctly. In generator.go, writeNodes() can be called recursively by certain nodes like if nodes. Eventually in this part

// Write trailing whitespace, if there is a next node that might need the space.
// If the next node is inline or text, we might need it.
// If the current node is a block element, we don't need it.
needed := (isInlineOrText(current) && isInlineOrText(next))
if ws, ok := current.(parser.WhitespaceTrailer); ok && needed {
if err := g.writeWhitespaceTrailer(indentLevel, ws.Trailing()); err != nil {
	return err
}
}
return

next currently gets passed nil since writeNodes() is writing a subtree of nodes. If we pass the next element to writeNodes(), we use it as the value of next when writing the last node with writeNode

It's kind of hard for me to explain so that's why I threw the branch together as an example. I'm also not sure if these lines I added

var nextNode parser.Node
if len(n.ElseIfs) > 0 {
	nextNode = n.ElseIfs[0].Then[0]
}
if nextNode == nil {
	nextNode = next
}
if err = g.writeNodes(indentLevel, stripLeadingAndTrailingWhitespace(n.Then), nextNode); err != nil {

are needed.

Not sure how much I'll get to work on this with this holidays but I just wanted to update with a cause.

JonnyLoughlin avatar Dec 24 '23 01:12 JonnyLoughlin

Fixed, thanks @JonnyLoughlin

joerdav avatar Jan 30 '24 16:01 joerdav