Skip to content

Commit fcb97cc

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: 836120469
1 parent 16587c7 commit fcb97cc

File tree

5 files changed

+198
-7
lines changed

5 files changed

+198
-7
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: 114 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1810,10 +1810,122 @@ 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+
name: "use short repeated primitive fields with inline comments",
1882+
in: `# txtpbfmt: use_short_repeated_primitive_fields
1883+
foo: 1 # inline comment a
1884+
foo: 2
1885+
foo: 3 # inline comment b
1886+
foo: 4
1887+
`,
1888+
out: `# txtpbfmt: use_short_repeated_primitive_fields
1889+
foo: [
1890+
1, # inline comment a
1891+
2,
1892+
3, # inline comment b
1893+
4
1894+
]
1895+
`}, {
1896+
name: "use short repeated primitive fields with pre comments",
1897+
in: `# txtpbfmt: use_short_repeated_primitive_fields
1898+
# pre comment a
1899+
foo: 1
1900+
foo: 2
1901+
foo: 3
1902+
# pre comment b
1903+
foo: 4
1904+
`,
1905+
out: `# txtpbfmt: use_short_repeated_primitive_fields
1906+
# pre comment a
1907+
foo: [1, 2, 3]
1908+
# pre comment b
1909+
foo: 4
1910+
`}, {
1911+
name: "use short repeated primitive fields with mixed comments",
1912+
in: `# txtpbfmt: use_short_repeated_primitive_fields
1913+
foo: 1
1914+
foo: 2
1915+
foo: 3 # inline comment
1916+
# pre comment
1917+
foo: 4
1918+
`,
1919+
out: `# txtpbfmt: use_short_repeated_primitive_fields
1920+
foo: [
1921+
1,
1922+
2,
1923+
3 # inline comment
1924+
]
1925+
# pre comment
1926+
foo: 4
1927+
`},
1928+
}
18171929
for _, input := range inputs {
18181930
out, err := Format([]byte(input.in))
18191931
if err != nil {

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 || len(nodes[j].PostValuesComments) > 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)