-
-
Notifications
You must be signed in to change notification settings - Fork 277
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
fix(fmt): Preserve multiple newlines between elements (#374) #919
base: main
Are you sure you want to change the base?
Changes from 1 commit
e398947
2f6aa04
dc3c684
aa31f93
24ba4f1
af46dd5
5e7bd2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
-- in -- | ||
package main | ||
|
||
templ x() { | ||
<div> | ||
<span>Hello | ||
|
||
World | ||
</span> | ||
|
||
|
||
|
||
<span>Foo Bar </span> | ||
</div> | ||
} | ||
-- out -- | ||
package main | ||
|
||
templ x() { | ||
<div> | ||
<span> | ||
Hello | ||
World | ||
</span> | ||
|
||
<span>Foo Bar </span> | ||
</div> | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,7 +80,7 @@ func TestTextParser(t *testing.T) { | |
}, | ||
}, | ||
{ | ||
name: "Multiline text is colected line by line", | ||
name: "Multiline text is collected line by line", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for fixing the typos! |
||
input: "Line 1\nLine 2", | ||
expected: Text{ | ||
Value: "Line 1", | ||
|
@@ -92,7 +92,7 @@ func TestTextParser(t *testing.T) { | |
}, | ||
}, | ||
{ | ||
name: "Multiline text is colected line by line (Windows)", | ||
name: "Multiline text is collected line by line (Windows)", | ||
input: "Line 1\r\nLine 2", | ||
expected: Text{ | ||
Value: "Line 1", | ||
|
@@ -103,6 +103,18 @@ func TestTextParser(t *testing.T) { | |
TrailingSpace: "\n", | ||
}, | ||
}, | ||
{ | ||
name: "Multiline text with multiple newlines is collected line by line", | ||
input: "Line 1\n\n\n\nLine 2", | ||
expected: Text{ | ||
Value: "Line 1", | ||
Range: Range{ | ||
From: Position{Index: 0, Line: 0, Col: 0}, | ||
To: Position{Index: 6, Line: 0, Col: 6}, | ||
}, | ||
TrailingSpace: "\n", | ||
}, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
tt := tt | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -375,17 +375,27 @@ func (t HTMLTemplate) Write(w io.Writer, indent int) error { | |
type TrailingSpace string | ||
|
||
const ( | ||
SpaceNone TrailingSpace = "" | ||
SpaceHorizontal TrailingSpace = " " | ||
SpaceVertical TrailingSpace = "\n" | ||
SpaceNone TrailingSpace = "" | ||
SpaceHorizontal TrailingSpace = " " | ||
SpaceVertical TrailingSpace = "\n" | ||
SpaceVerticalDouble TrailingSpace = "\n\n" | ||
) | ||
|
||
var ErrNonSpaceCharacter = errors.New("non space character found") | ||
|
||
func NewTrailingSpace(s string) (ts TrailingSpace, err error) { | ||
func NewTrailingSpace(s string, allowMulti bool) (ts TrailingSpace, err error) { | ||
joerdav marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the other conversation, the bool flag makes it harder to understand what's going on without referring to the function definition. Would prefer an alternative solution, e.g. an enum style for allow multi, or functional params. |
||
var hasHorizontalSpace bool | ||
for _, r := range s { | ||
|
||
runes := []rune(s) | ||
|
||
for i, r := range s { | ||
if r == '\n' { | ||
if allowMulti && i < len(runes)-1 { | ||
next := runes[i+1] | ||
if next == '\n' { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect Windows users would like a check for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had assumed this was normalized before entering this part of the code. Seems dangerous to have 2 code-paths based on OS everywhere in the code. I'd check, but it'd be difficult for me to test it since I don't have Windows 😅 |
||
return SpaceVerticalDouble, nil | ||
} | ||
} | ||
return SpaceVertical, nil | ||
} | ||
if unicode.IsSpace(r) { | ||
|
@@ -621,7 +631,9 @@ func writeNodes(w io.Writer, level int, nodes []Node, indent bool) error { | |
} | ||
// Put a newline after the last node in indentation mode. | ||
if indent && ((nextNodeIsBlock(nodes, i) || i == len(nodes)-1) || shouldAlwaysBreakAfter(nodes[i])) { | ||
trailing = SpaceVertical | ||
if !strings.Contains(string(trailing), string(SpaceVertical)) { | ||
trailing = SpaceVertical | ||
} | ||
} | ||
switch trailing { | ||
case SpaceNone: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.