Skip to content

Commit

Permalink
cue/format: omit whitespace before the first clause
Browse files Browse the repository at this point in the history
When printing clauses, we often want to print a whitespace before the
token "let", "if", and "for". For example:

	for i, v in src if i > 1 {

As opposed to omitting spaces, which breaks the syntax:

	for i, v in srcif i > 1 {

However, we also printed a space before the first clause in the list.
This could result in a CUE file beginning with a space if the first
token is a clause:

	$ cue fmt .
	$ cat if.cue
	 if foo {
	}

First, we fix that bug for clauses by not having walkClauseList call
f.print(blank) before the first clause.

Second, we also fix it for "let" declarations by teaching mayCombine
about the previous token being ILLEGAL - meaning that we're printing the
first token in the entire file.

It is worth noting that this also changes how we print the first clause
inside another expression; for instance:

	cfgs: [ for crd in ["one", "two"] {
	}]

Now formats without the leading space, just like at the beginning of a file:

	cfgs: [for crd in ["one", "two"] {
	}]

Arguably that is more consistent, as there is no space after the "for"
clause, following the "}" token.

Fixes #1544.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I72e4618f2b1011005316c617a32e0c3a6ff3b706
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/538624
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@gmail.com>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
  • Loading branch information
mvdan committed Jun 12, 2024
1 parent 8a6194a commit 95035da
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 6 deletions.
9 changes: 7 additions & 2 deletions cue/format/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,12 @@ func (f *formatter) walkSpecList(list []*ast.ImportSpec) {

func (f *formatter) walkClauseList(list []ast.Clause, ws whiteSpace) {
f.before(nil)
for _, x := range list {
for i, x := range list {
f.before(x)
f.print(ws)
// Only print the whitespace between the clauses.
if i > 0 {
f.print(ws)
}
f.clause(x)
f.after(x)
}
Expand Down Expand Up @@ -738,6 +741,8 @@ func (f *formatter) clause(clause ast.Clause) {
f.markUnindentLine()

case *ast.LetClause:
// TODO(mvdan): LetClause is handled in both the clause and decl methods,
// because at the semantic level it is different in each case, but the code is repetitive.
f.print(n.Let, token.LET, blank, nooverride)
f.print(indent)
f.expr(n.Ident)
Expand Down
9 changes: 9 additions & 0 deletions cue/format/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,9 +425,18 @@ func (p *printer) writeByte(ch byte, n int) {
p.pos.Column += n
}

// TODO(mvdan): mayCombine as a name was carried over from Go,
// but it doesn't really make sense as a name for our logic here,
// since we return true when either side must use a blank space.

func mayCombine(prev, next token.Token) (before, after bool) {
s := next.String()
if 'a' <= s[0] && s[0] < 'z' {
if prev == token.ILLEGAL {
// If we're printing the first token,
// we don't need a blank space before it.
return false, true
}
return true, true
}
switch prev {
Expand Down
6 changes: 3 additions & 3 deletions cue/format/testdata/issue1544.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@ if true {
x: 5
}
-- if.golden --
if true {
if true {
x: 5
}
-- let.input --
let x = 5
y: x
-- let.golden --
let x = 5
let x = 5
y: x
-- for.input --
for v in ["a", "b"] {
(v): v
}
-- for.golden --
for v in ["a", "b"] {
for v in ["a", "b"] {
(v): v
}
2 changes: 1 addition & 1 deletion tools/trim/testdata/comprehensions.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ a_map: {
}
-- out/trim --
== in.cue
if true {
if true {
a_map: {
"first": {}
}
Expand Down

0 comments on commit 95035da

Please sign in to comment.