Skip to content

Commit

Permalink
cue/format: don't indent fields on same line as lbrace
Browse files Browse the repository at this point in the history
Vertical tabs are inserted after struct fields in order to correctly
vertically align values via text/tabwriter.

A recent CL https://cuelang.org/cl/1170966 made changes to this logic
in order to improve indentation of list elements.
As part of this change, a field whose value is a binary expression will
no longer have a vertical tab inserted.
For example, given the following code:

    x: 1
    foo: bar & {baz: 1}

The formatter will insert vertical tabs like so:

    x:<VT>1
    foo: bar & {baz:<VT>1}

Which means text/tabwriter will output the following:

    x:               1
    foo: bar & {baz: 1}

The logic is fixed to now ensure that no vtabs are inserted in a line
once an '{' has been written, so that vertical tabs in the above example
will now be inserted like this:

    x:<VT>1
    foo: bar & {baz: 1}

And the formatted code will now be:

    x: 1
    foo: bar & {baz: 1}

In addition, a few typos were fixed and dead code was removed.

Fixes #3015

Signed-off-by: Noam Dolovich <noam.tzvi.dolovich@gmail.com>
Change-Id: Ibee8ffc617324396523b50a4bb30b9cfc8beb28c
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1193515
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
  • Loading branch information
NoamTD authored and mvdan committed Apr 22, 2024
1 parent 9839112 commit 0177234
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 28 deletions.
7 changes: 0 additions & 7 deletions cue/format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,13 +262,6 @@ func (f *formatter) formfeed() whiteSpace {
return formfeed
}

func (f *formatter) wsOverride(def whiteSpace) whiteSpace {
if f.current.override == ignore {
return def
}
return f.current.override
}

func (f *formatter) onOneLine(node ast.Node) bool {
a := node.Pos()
b := node.End()
Expand Down
2 changes: 1 addition & 1 deletion cue/format/format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func init() {
}
}

// TestNodes tests nodes that are that are invalid CUE, but are accepted by
// TestNodes tests nodes that are invalid CUE, but are accepted by
// format.
func TestNodes(t *testing.T) {
testCases := []struct {
Expand Down
7 changes: 4 additions & 3 deletions cue/format/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ func (f *formatter) inlineField(n *ast.Field) *ast.Field {
regular := internal.IsRegularField(n)
// shortcut single-element structs.
// If the label has a valid position, we assume that an unspecified
// Lbrace signals the intend to collapse fields.
// Lbrace signals the intent to collapse fields.
if !n.Label.Pos().IsValid() && !(f.printer.cfg.simplify && regular) {
return nil
}
Expand Down Expand Up @@ -304,7 +304,7 @@ func (f *formatter) decl(decl ast.Decl) {

nextFF := f.nextNeedsFormfeed(n.Value)
tab := vtab
if nextFF {
if nextFF || f.prevLbraceOnLine {
tab = blank
}

Expand Down Expand Up @@ -645,6 +645,7 @@ func (f *formatter) exprRaw(expr ast.Expr, prec1, depth int) {
ws |= newline | nooverride
}
f.print(x.Lbrace, token.LBRACE, &l, ws, ff, indent)
f.prevLbraceOnLine = l == f.lineout

f.walkDeclList(x.Elts)
f.matchUnindent()
Expand Down Expand Up @@ -849,7 +850,7 @@ func (f *formatter) binaryExpr(x *ast.BinaryExpr, prec1, cutoff, depth int) {
prec := x.Op.Precedence()
if prec < prec1 {
// parenthesis needed
// Note: The parser inserts an syntax.ParenExpr node; thus this case
// Note: The parser inserts a syntax.ParenExpr node; thus this case
// can only occur if the AST is created in a different way.
// defer p.pushComment(nil).pop()
f.print(token.LPAREN, nooverride)
Expand Down
20 changes: 14 additions & 6 deletions cue/format/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ type printer struct {

lastTok token.Token // last token printed (syntax.ILLEGAL if it's whitespace)

output []byte
indent int
spaceBefore bool
output []byte
indent int
spaceBefore bool
prevLbraceOnLine bool // true if a '{' has been written on the current line

errs errors.Error
}
Expand Down Expand Up @@ -269,17 +270,17 @@ func (p *printer) writeWhitespace(ws whiteSpace) {
case ws&newsection != 0:
p.maybeIndentLine(ws)
p.writeByte('\f', 2)
p.lineout += 2
p.incrementLine(2)
p.spaceBefore = true
case ws&formfeed != 0:
p.maybeIndentLine(ws)
p.writeByte('\f', 1)
p.lineout++
p.incrementLine(1)
p.spaceBefore = true
case ws&newline != 0:
p.maybeIndentLine(ws)
p.writeByte('\n', 1)
p.lineout++
p.incrementLine(1)
p.spaceBefore = true
case ws&declcomma != 0:
p.writeByte(',', 1)
Expand All @@ -295,6 +296,13 @@ func (p *printer) writeWhitespace(ws whiteSpace) {
}
}

func (p *printer) incrementLine(n int) {
if n != 0 {
p.prevLbraceOnLine = false
}
p.lineout += line(n)
}

func (p *printer) markLineIndent(ws whiteSpace) {
p.indentStack = append(p.indentStack, ws)
}
Expand Down
8 changes: 4 additions & 4 deletions cue/format/testdata/expressions.golden
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,12 @@ import "list"
o: [{}]
o: [{}]

p: 1
p: 1
p: p & {p: 2}
q: 1
q: 1
q: q | {q: 2}
r: 1
r: 1
r: b & [1, 2, {a: 4}]
s: [string]: [string]: a
s: [string]: {s: string}
s: [string]: {s: string}
}
2 changes: 1 addition & 1 deletion cue/token/position.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (p RelPos) Pos() Pos {
return Pos{nil, int(p)}
}

// HasRelPos repors whether p has a relative position.
// HasRelPos reports whether p has a relative position.
func (p Pos) HasRelPos() bool {
return p.offset&relMask != 0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,17 +153,17 @@ package descriptor
// 0 is reserved for errors.
// Order is weird for historical reasons.
"TYPE_DOUBLE"
#enumValue: 1
#enumValue: 1
} | {"TYPE_FLOAT", #enumValue: 2} | {
// Not ZigZag encoded. Negative numbers take 10 bytes. Use TYPE_SINT64 if
// negative values are likely.
"TYPE_INT64"
#enumValue: 3
#enumValue: 3
} | {"TYPE_UINT64", #enumValue: 4} | {
// Not ZigZag encoded. Negative numbers take 10 bytes. Use TYPE_SINT32 if
// negative values are likely.
"TYPE_INT32"
#enumValue: 5
#enumValue: 5
} | {"TYPE_FIXED64", #enumValue: 6} |
{"TYPE_FIXED32", #enumValue: 7} |
{"TYPE_BOOL", #enumValue: 8} |
Expand All @@ -180,7 +180,7 @@ package descriptor
} | {
// New in version 2.
"TYPE_BYTES"
#enumValue: 12
#enumValue: 12
} | {"TYPE_UINT32", #enumValue: 13} |
{"TYPE_ENUM", #enumValue: 14} |
{"TYPE_SFIXED32", #enumValue: 15} |
Expand Down Expand Up @@ -216,7 +216,7 @@ package descriptor
#Label: {
// 0 is reserved for errors
"LABEL_OPTIONAL"
#enumValue: 1
#enumValue: 1
} | {"LABEL_REQUIRED", #enumValue: 2} |
{"LABEL_REPEATED", #enumValue: 3}

Expand Down Expand Up @@ -518,7 +518,7 @@ package descriptor
#CType: {
// Default mode.
"STRING"
#enumValue: 0
#enumValue: 0
} | {"CORD", #enumValue: 1} |
{"STRING_PIECE", #enumValue: 2}

Expand Down

0 comments on commit 0177234

Please sign in to comment.