Skip to content

Commit

Permalink
cue/format: avoid indenting a lone leading comment
Browse files Browse the repository at this point in the history
We would often precede a comment with a vertical tab for alignment,
even when a comment began at the start of the line,
where vertical alignment is completely pointless.

This was largely harmless, except in a few edge cases such as
a single comment followed by a declaration, where we would indent.

Instead, only add the vertical tab if the comment does not begin
at the start of the line, resolving these issues.

Note that this change requires no longer using internal.NewComment
for top-level comments in a file, as the API always sets either of
the Doc or Line booleans in ast.Comment, causing extra and unwanted
blank spaces or newlines. A lone comment at the start of a file
is neither a doc comment nor a line (inline) comment.

Fixes #722.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: If9f1afb3f3e6dbdf65d5ee28c76a0f279c97aa49
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1196134
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Aram Hăvărneanu <aram@cue.works>
  • Loading branch information
mvdan committed Jun 14, 2024
1 parent 0cf2060 commit d70f9d7
Show file tree
Hide file tree
Showing 9 changed files with 14 additions and 13 deletions.
4 changes: 3 additions & 1 deletion cmd/cue/cmd/get_go.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,9 @@ func (e *extractor) extractPkg(root string, p *packages.Package) error {
addDoc(f.Doc, pkg)

f := &cueast.File{Decls: []cueast.Decl{
internal.NewComment(false, "Code generated by cue get go. DO NOT EDIT."),
&cueast.CommentGroup{List: []*cueast.Comment{
{Text: "// Code generated by cue get go. DO NOT EDIT."},
}},
&cueast.CommentGroup{List: []*cueast.Comment{
{Text: "//cue:generate cue get go " + args},
}},
Expand Down
2 changes: 1 addition & 1 deletion cue/ast/astutil/sanitize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ a: bar.Min()
spec2 := ast.NewImport(nil, "foo/bar")
spec3 := ast.NewImport(ast.NewIdent("bar"), "foo")
return &ast.File{Decls: []ast.Decl{
internal.NewComment(false, "File comment"),
&ast.CommentGroup{List: []*ast.Comment{{Text: "// File comment"}}},
&ast.Package{Name: ast.NewIdent("pkg")},
&ast.Field{
Label: ast.NewIdent("a"),
Expand Down
5 changes: 2 additions & 3 deletions cue/format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,13 +313,12 @@ func (f *formatter) visitComments(until int8) {
func (f *formatter) printComment(cg *ast.CommentGroup) {
f.Print(cg)

printBlank := false
if cg.Doc && len(f.output) > 0 {
f.Print(newline)
printBlank = true
}
for _, c := range cg.List {
if !printBlank {
if f.pos.Column > 1 {
// Vertically align inline comments.
f.Print(vtab)
}
f.Print(c.Slash)
Expand Down
4 changes: 2 additions & 2 deletions cue/format/testdata/comments.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -334,10 +334,10 @@ foo: [
-- comment_alone.golden --
// Just one comment on its own.
-- comment_field.input --
// Just one comment on a field. TODO(mvdan): do not indent.
// Just one comment on a field.
foo: string
-- comment_field.golden --
// Just one comment on a field. TODO(mvdan): do not indent.
// Just one comment on a field.
foo: string
-- comments_alone.input --
// Just a few comments
Expand Down
2 changes: 1 addition & 1 deletion cue/format/testdata/expressions.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ _foo: {
// skip_create_image: true
}
-- issue2496.golden --
// TODO(mvdan): we indent the list elements too much.
// TODO(mvdan): we indent the list elements too much.
machine_type: [
if arch == "amd" {"n2-standard-2"},
if arch == "arm" {"t2a-standard-2"},
Expand Down
4 changes: 2 additions & 2 deletions encoding/protobuf/jsonpb/testdata/decoder/null.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ a9: null
// comment a10
a10: null
-- out/jsonpb/data.yaml --
// comment a0
// comment a0
a0: 0

// comment a1
Expand Down Expand Up @@ -121,7 +121,7 @@ a9: true
// comment a10
a10: null
-- out/jsonpb/data.cue --
// comment a0
// comment a0
a0: 0

// comment a1
Expand Down
2 changes: 1 addition & 1 deletion internal/core/export/testdata/main/disjunction.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ a: *1 | int
b: *2 | int
c: *(a & b) | 3
-- out/definition --
// Issue #950
// Issue #950
a: *1 | int
b: *2 | int
c: *(a & b) | 3
Expand Down
2 changes: 1 addition & 1 deletion internal/core/export/testdata/selfcontained/import.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ diff old new
+++ new
@@ -1,10 +1,7 @@
import "tool/exec"

// Can be inlined.
-v: {
- x: 3
Expand Down
2 changes: 1 addition & 1 deletion pkg/math/testdata/mult.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ f1: math.MultipleOf(5555555, 2)
f2: math.MultipleOf(100.01, 4)

-- out/math --
// should be true
// should be true
t1: true
t2: true
t3: true
Expand Down

0 comments on commit d70f9d7

Please sign in to comment.