Skip to content

Commit 654e97f

Browse files
Yassin Ezbakhetxtpbfmt-copybara-robot
authored andcommitted
txtpbfmt: Add support for formatting repeated primitive fields as lists.
When this option is used, repeated primitive fields are printed using a more conscice list syntax: ```textproto repeated_field: 1 repeated_field: 2 repeated_field: 3 ``` vs ```textproto repeated_field: [1, 2, 3] ``` This syntax is part of the go/textformat-spec (screen/4cDavcUGvdjyWwN), thus it's supported by all parsers. Furthermore, some proto implementations (like [C++](http://google3/third_party/protobuf/text_format.h;l=407;rcl=828248838) and [Java](http://google3/third_party/java_src/protobuf/current/java/com/google/protobuf/TextFormat.java;l=385;rcl=815831657)) allow using it when printing text-proto. This CL is a no-op if the config option isn't used. PiperOrigin-RevId: 834277184
1 parent 16587c7 commit 654e97f

File tree

6 files changed

+161
-10
lines changed

6 files changed

+161
-10
lines changed

config/config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ type Config struct {
8787
// Use single quotes around strings that contain double but not single quotes.
8888
SmartQuotes bool
8989

90+
// Use a short representation for repeated primitive fields (`x: 1 x: 2` vs `x: [1, 2]`). If this
91+
// field is true, all repeated primitive fields will use the short representation; otherwise, the
92+
// latter will be used only if it's being used in the input textproto.
93+
UseShortRepeatedPrimitiveFields bool
94+
9095
// Logger enables logging when it is non-nil.
9196
// If the log messages aren't going to be useful, it's best to leave Logger
9297
// set to nil, as otherwise log messages will be constructed.

impl/impl.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,8 @@ func addToConfig(metaComment string, c *config.Config) error {
254254
c.WrapStringsAfterNewlines = true
255255
case "wrap_strings_without_wordwrap":
256256
c.WrapStringsWithoutWordwrap = true
257+
case "use_short_repeated_primitive_fields":
258+
c.UseShortRepeatedPrimitiveFields = true
257259
case "on": // This doesn't change the overall config.
258260
case "off": // This doesn't change the overall config.
259261
default:

parser/parser_test.go

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1810,10 +1810,75 @@ a, b,
18101810
`}, {
18111811
name: "carriage return \\r is formatted away",
18121812
in: `foo: "bar"` + "\r" + `baz: "bat"` + "\r",
1813-
out: `foo: "bar"` + "\n" + `baz: "bat"` + "\n"}, {
1813+
out: `foo: "bar"` + "\n" + `baz: "bat"` + "\n",
1814+
}, {
18141815
name: "Windows-style newline \\r\\n is formatted away",
18151816
in: `foo: "bar"` + "\r\n" + `baz: "bat"` + "\r\n",
1816-
out: `foo: "bar"` + "\n" + `baz: "bat"` + "\n"}}
1817+
out: `foo: "bar"` + "\n" + `baz: "bat"` + "\n",
1818+
}, {
1819+
name: "use short repeated primitive fields",
1820+
in: `# txtpbfmt: use_short_repeated_primitive_fields
1821+
foo: 1
1822+
foo: 2
1823+
foo: 3
1824+
`,
1825+
out: `# txtpbfmt: use_short_repeated_primitive_fields
1826+
foo: [1, 2, 3]
1827+
`}, {
1828+
name: "use short repeated primitive fields with single element list",
1829+
in: `# txtpbfmt: use_short_repeated_primitive_fields
1830+
foo: 1
1831+
`,
1832+
out: `# txtpbfmt: use_short_repeated_primitive_fields
1833+
foo: 1
1834+
`}, {
1835+
name: "use short repeated primitive fields with interleaved list",
1836+
in: `# txtpbfmt: use_short_repeated_primitive_fields
1837+
foo: 1
1838+
foo: 2
1839+
bar: 3
1840+
foo: 4
1841+
foo: 5
1842+
bar: 6
1843+
bar: 7
1844+
foo: 8
1845+
`,
1846+
out: `# txtpbfmt: use_short_repeated_primitive_fields
1847+
foo: [1, 2]
1848+
bar: 3
1849+
foo: [4, 5]
1850+
bar: [6, 7]
1851+
foo: 8
1852+
`}, {
1853+
name: "use short repeated primitive fields with string field",
1854+
in: `# txtpbfmt: use_short_repeated_primitive_fields
1855+
foo: "a"
1856+
foo: "b"
1857+
`,
1858+
out: `# txtpbfmt: use_short_repeated_primitive_fields
1859+
foo: ["a", "b"]
1860+
`}, {
1861+
name: "use short repeated primitive fields with message field",
1862+
in: `# txtpbfmt: use_short_repeated_primitive_fields
1863+
foo { x: 1 }
1864+
foo { x: 2 }
1865+
`,
1866+
out: `# txtpbfmt: use_short_repeated_primitive_fields
1867+
foo { x: 1 }
1868+
foo { x: 2 }
1869+
`}, {
1870+
name: "use short repeated primitive fields inside message",
1871+
in: `# txtpbfmt: use_short_repeated_primitive_fields
1872+
foo { x: 1 x: 2 x: 3 }
1873+
foo { x: 4 }
1874+
foo { x: 5 x: 6 }
1875+
`,
1876+
out: `# txtpbfmt: use_short_repeated_primitive_fields
1877+
foo { x: [1, 2, 3] }
1878+
foo { x: 4 }
1879+
foo { x: [5, 6] }
1880+
`},
1881+
}
18171882
for _, input := range inputs {
18181883
out, err := Format([]byte(input.in))
18191884
if err != nil {

printer/printer.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func FormatWithConfig(in []byte, c config.Config) ([]byte, error) {
3232
if err != nil {
3333
return nil, err
3434
}
35-
return FormatNodes(nodes), nil
35+
return FormatNodesWithDepthAndConfig(nodes, 0, c), nil
3636
}
3737

3838
func removeDeleted(nodes []*ast.Node) []*ast.Node {
@@ -100,8 +100,13 @@ func FormatNodes(nodes []*ast.Node) []byte {
100100

101101
// FormatNodesWithDepth returns formatted nodes at the given indentation depth (0 = top-level) as bytes.
102102
func FormatNodesWithDepth(nodes []*ast.Node, depth int) []byte {
103+
return FormatNodesWithDepthAndConfig(nodes, depth, config.Config{})
104+
}
105+
106+
// FormatNodesWithDepthAndConfig returns formatted nodes at the given indentation depth (0 = top-level) as bytes.
107+
func FormatNodesWithDepthAndConfig(nodes []*ast.Node, depth int, c config.Config) []byte {
103108
var result bytes.Buffer
104-
formatter{&result}.writeNodes(removeDeleted(nodes), depth, false /* isSameLine */, false /* asListItems */)
109+
formatter{&result, c}.writeNodes(removeDeleted(nodes), depth, false /* isSameLine */, false /* asListItems */)
105110
return result.Bytes()
106111
}
107112

@@ -113,6 +118,7 @@ type stringWriter interface {
113118
// formatter accumulates pretty-printed textproto contents into a stringWriter.
114119
type formatter struct {
115120
stringWriter
121+
config.Config
116122
}
117123

118124
func (f formatter) writeNode(nd *ast.Node, depth int, isSameLine, asListItems bool, index, lastNonCommentIndex int) {
@@ -204,7 +210,8 @@ func (f formatter) writeNodeName(nd *ast.Node, indent string) {
204210
}
205211

206212
func (f formatter) writeNodeValues(nd *ast.Node, indent string) {
207-
if nd.ValuesAsList { // For ValuesAsList option we will preserve even empty list `field: []`
213+
if nd.ValuesAsList {
214+
// For ValuesAsList option we will preserve even empty list `field: []`
208215
f.writeValuesAsList(nd, nd.Values, indent+indentSpaces)
209216
} else if len(nd.Values) > 0 {
210217
f.writeValues(nd, nd.Values, indent+indentSpaces)

sort/sort.go

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,19 @@ type valuesSortFunction func(values []*ast.Value)
5656

5757
// Process sorts and filters the given nodes.
5858
func Process(parent *ast.Node, nodes []*ast.Node, c config.Config) error {
59-
return process(parent, nodes, nodeSortFunctionConfig(c), nodeFilterFunctionConfig(c), valuesSortFunctionConfig(c))
59+
return process(parent, nodes, nodeSortFunctionConfig(c), nodeFilterFunctionConfig(c), valuesSortFunctionConfig(c), c)
6060
}
6161

6262
// process sorts and filters the given nodes.
63-
func process(parent *ast.Node, nodes []*ast.Node, sortFunction nodeSortFunction, filterFunction nodeFilterFunction, valuesSortFunction valuesSortFunction) error {
63+
func process(parent *ast.Node, nodes []*ast.Node, sortFunction nodeSortFunction, filterFunction nodeFilterFunction, valuesSortFunction valuesSortFunction, c config.Config) error {
6464
if len(nodes) == 0 {
6565
return nil
6666
}
6767
if filterFunction != nil {
6868
filterFunction(nodes)
6969
}
7070
for _, nd := range nodes {
71-
err := process(nd, nd.Children, sortFunction, filterFunction, valuesSortFunction)
71+
err := process(nd, nd.Children, sortFunction, filterFunction, valuesSortFunction, c)
7272
if err != nil {
7373
return err
7474
}
@@ -77,11 +77,46 @@ func process(parent *ast.Node, nodes []*ast.Node, sortFunction nodeSortFunction,
7777
}
7878
}
7979
if sortFunction != nil {
80-
return sortFunction(parent, nodes)
80+
if err := sortFunction(parent, nodes); err != nil {
81+
return err
82+
}
83+
}
84+
if c.UseShortRepeatedPrimitiveFields {
85+
groupRepeatedPrimitiveFields(nodes)
8186
}
8287
return nil
8388
}
8489

90+
func isPrimitive(n *ast.Node) bool {
91+
return len(n.Children) == 0 && len(n.Values) == 1
92+
}
93+
94+
func groupRepeatedPrimitiveFields(nodes []*ast.Node) {
95+
for i := 0; i < len(nodes); {
96+
node := nodes[i]
97+
if node.Deleted || !isPrimitive(node) {
98+
i++
99+
continue
100+
}
101+
j := i + 1
102+
for ; j < len(nodes); j++ {
103+
if nodes[j].Deleted || !isPrimitive(nodes[j]) || nodes[j].Name != node.Name || len(nodes[j].PreComments) > 0 {
104+
break
105+
}
106+
}
107+
if j > i+1 {
108+
// Found group of repeated primitive fields: nodes[i...j-1]
109+
node.ValuesAsList = true
110+
node.ChildrenSameLine = true
111+
for k := i + 1; k < j; k++ {
112+
node.Values = append(node.Values, nodes[k].Values...)
113+
nodes[k].Deleted = true
114+
}
115+
}
116+
i = j
117+
}
118+
}
119+
85120
// removeDuplicates marks duplicate key:value pairs from nodes as Deleted.
86121
func removeDuplicates(nodes []*ast.Node) {
87122
type nameAndValue struct {

sort/sort_test.go

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,43 @@ func TestProcess(t *testing.T) {
150150
ValuesAsList: true,
151151
},
152152
},
153+
}, {
154+
name: "use short repeated primitive fields",
155+
nodes: []*ast.Node{
156+
{Name: "foo", Values: []*ast.Value{{Value: "1"}}},
157+
{Name: "foo", Values: []*ast.Value{{Value: "2"}}},
158+
{Name: "bar", Values: []*ast.Value{{Value: "3"}}},
159+
},
160+
c: config.Config{
161+
UseShortRepeatedPrimitiveFields: true,
162+
},
163+
want: []*ast.Node{
164+
{Name: "foo", Values: []*ast.Value{{Value: "1"}, {Value: "2"}}, ValuesAsList: true, ChildrenSameLine: true},
165+
{Name: "foo", Values: []*ast.Value{{Value: "2"}}, Deleted: true},
166+
{Name: "bar", Values: []*ast.Value{{Value: "3"}}},
167+
},
168+
}, {
169+
name: "use short repeated primitive fields with single element list (ValuesAsList: true)",
170+
nodes: []*ast.Node{
171+
{Name: "foo", Values: []*ast.Value{{Value: "1"}}, ValuesAsList: true},
172+
},
173+
c: config.Config{
174+
UseShortRepeatedPrimitiveFields: true,
175+
},
176+
want: []*ast.Node{
177+
{Name: "foo", Values: []*ast.Value{{Value: "1"}}, ValuesAsList: true},
178+
},
179+
}, {
180+
name: "use short repeated primitive fields with single element list (ValuesAsList: false)",
181+
nodes: []*ast.Node{
182+
{Name: "foo", Values: []*ast.Value{{Value: "1"}}},
183+
},
184+
c: config.Config{
185+
UseShortRepeatedPrimitiveFields: true,
186+
},
187+
want: []*ast.Node{
188+
{Name: "foo", Values: []*ast.Value{{Value: "1"}}},
189+
},
153190
}, {
154191
name: "error in sort function",
155192
nodes: []*ast.Node{
@@ -184,7 +221,7 @@ func TestProcess(t *testing.T) {
184221
if tc.skipValuesFunction {
185222
valuesFunction = nil
186223
}
187-
err := process(nil, tc.nodes, sortFunction, filterFunction, valuesFunction)
224+
err := process(nil, tc.nodes, sortFunction, filterFunction, valuesFunction, tc.c)
188225
if tc.wantErr != "" {
189226
if err == nil {
190227
t.Errorf("process(%v, %v, %v, %v) got nil error, want %q", tc.nodes, sortFunction, filterFunction, valuesFunction, tc.wantErr)

0 commit comments

Comments
 (0)