Skip to content

Commit b05e2e0

Browse files
ecPabloecPablo
authored
fix: make newlines consistent and fix default value for format flag (#548)
Some small fixes as a followup from the proposal analyzer refactor: * Standarize newlines in template files: Tried using: ``` {{.Value -}} ``` But If .tpml ends with a newline (added by IDE), this still won’t trim it because Go parses the template before evaluating it, and the newline is part of the literal text. * Throw error when invalid formater flag value is passed in.
1 parent f700dc9 commit b05e2e0

23 files changed

+67
-40
lines changed

‎engine/cld/legacy/cli/mcmsv2/mcms_v2.go‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,10 @@ func buildMCMSv2AnalyzeProposalCmd(
733733
}
734734

735735
// Set renderer based on format flag
736-
renderer := createRendererFromFormat(format)
736+
renderer, err := createRendererFromFormat(format)
737+
if err != nil {
738+
return fmt.Errorf("failed to create renderer: %w", err)
739+
}
737740
cfgv2.proposalCtx.SetRenderer(renderer)
738741

739742
var analyzedProposal string
@@ -1611,14 +1614,16 @@ func addCallProxyOption(
16111614

16121615
// createRendererFromFormat creates an appropriate renderer based on the format string.
16131616
// Defaults to markdown renderer for unknown formats.
1614-
func createRendererFromFormat(format string) analyzer.Renderer {
1617+
func createRendererFromFormat(format string) (analyzer.Renderer, error) {
16151618
switch format {
16161619
case "text", "txt":
1617-
return analyzer.NewTextRenderer()
1620+
return analyzer.NewTextRenderer(), nil
16181621
case "markdown", "md":
1619-
return analyzer.NewMarkdownRenderer()
1622+
return analyzer.NewMarkdownRenderer(), nil
1623+
case "":
1624+
return analyzer.NewMarkdownRenderer(), nil
16201625
default:
1621-
// Default to markdown if format is not specified or invalid
1622-
return analyzer.NewMarkdownRenderer()
1626+
// error if format is not specified or invalid
1627+
return nil, fmt.Errorf("unknown format '%s'", format)
16231628
}
16241629
}

‎experimental/analyzer/decoded_call_test.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ Method: complexCall
114114
Inputs:
115115
address: 0x0000000000000000000000000000000000000001
116116
chain: ` + "`" + `1 (<chain unknown>)` + "`" + `
117-
118117
data: 0x010203
119118
120119
Outputs:

‎experimental/analyzer/describe_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestDescribeProposal(t *testing.T) {
3030
name: "Empty proposal",
3131
operations: []types.Operation{},
3232
expectError: false,
33-
outputContains: []string{""},
33+
outputContains: []string{"\n"},
3434
},
3535
{
3636
name: "Single operation - unsupported chain",
@@ -128,7 +128,7 @@ func TestDescribeTimelockProposal(t *testing.T) {
128128
name: "Empty proposal",
129129
operations: []types.BatchOperation{},
130130
expectError: false,
131-
outputContains: []string{""},
131+
outputContains: []string{"\n"},
132132
},
133133
{
134134
name: "Single batch - unsupported chain",

‎experimental/analyzer/renderer_markdown.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,12 @@ func (r *MarkdownRenderer) renderTemplate(tmpl *template.Template, data interfac
608608
return fmt.Sprintf("Error rendering template: %v", err)
609609
}
610610

611-
return buf.String()
611+
out := buf.String()
612+
if !strings.HasSuffix(out, "\n\n") { // assuming double newline means intentional spacing
613+
out = strings.TrimSuffix(out, "\n")
614+
}
615+
616+
return out
612617
}
613618

614619
// isSimpleValue determines if a string represents a simple value that should be displayed without backticks.

‎experimental/analyzer/renderer_text.go‎

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"embed"
66
"fmt"
7+
"strings"
78
"text/template"
89

910
"github.com/ethereum/go-ethereum/common/hexutil"
@@ -132,8 +133,10 @@ func (r *TextRenderer) RenderDecodedCall(d *DecodedCall, ctx *FieldContext) stri
132133
if err := r.decodedCallTmpl.Execute(&buf, data); err != nil {
133134
return fmt.Sprintf("Error rendering decoded call: %v", err)
134135
}
136+
out := buf.String()
137+
out = strings.TrimSuffix(out, "\n")
135138

136-
return buf.String()
139+
return out
137140
}
138141

139142
// RenderProposal renders a ProposalReport as plain text using templates
@@ -156,7 +159,12 @@ func (r *TextRenderer) RenderProposal(rep *ProposalReport, ctx *FieldContext) st
156159
return fmt.Sprintf("Error rendering proposal: %v", err)
157160
}
158161

159-
return buf.String()
162+
out := buf.String()
163+
if !strings.HasSuffix(out, "\n\n") { // assuming double newline means intentional spacing
164+
out = strings.TrimSuffix(out, "\n")
165+
}
166+
167+
return out
160168
}
161169

162170
// RenderTimelockProposal renders a Timelock ProposalReport as plain text using templates
@@ -188,7 +196,12 @@ func (r *TextRenderer) RenderTimelockProposal(rep *ProposalReport, ctx *FieldCon
188196
return fmt.Sprintf("Error rendering timelock: %v", err)
189197
}
190198

191-
return buf.String()
199+
out := buf.String()
200+
if !strings.HasSuffix(out, "\n\n") { // assuming double newline means intentional spacing
201+
out = strings.TrimSuffix(out, "\n")
202+
}
203+
204+
return out
192205
}
193206

194207
// RenderField renders a NamedField as plain text
@@ -225,66 +238,72 @@ func (r *TextRenderer) getChainNameOrEmpty(selector uint64) string {
225238
// renderFieldValue renders any field value as plain text using templates
226239
func (r *TextRenderer) renderFieldValue(field FieldValue) string {
227240
var buf bytes.Buffer
228-
241+
var out string
229242
switch f := field.(type) {
230243
case AddressField:
231244
data := AddressFieldData{Value: f.GetValue()}
232245
if err := r.addressFieldTmpl.Execute(&buf, data); err != nil {
233246
return fmt.Sprintf("Error rendering address field: %v", err)
234247
}
235248

236-
return buf.String()
249+
out = buf.String()
237250

238251
case ChainSelectorField:
239252
if err := r.chainSelectorFieldTmpl.Execute(&buf, f); err != nil {
240253
return fmt.Sprintf("Error rendering chain selector field: %v", err)
241254
}
242255

243-
return buf.String()
256+
out = buf.String()
244257

245258
case BytesField:
246259
if err := r.bytesFieldTmpl.Execute(&buf, f); err != nil {
247260
return fmt.Sprintf("Error rendering bytes field: %v", err)
248261
}
249262

250-
return buf.String()
263+
out = buf.String()
251264

252265
case ArrayField:
253266
if err := r.arrayFieldTmpl.Execute(&buf, f); err != nil {
254267
return fmt.Sprintf("Error rendering array field: %v", err)
255268
}
256269

257-
return buf.String()
270+
out = buf.String()
258271

259272
case StructField:
260273
if err := r.structFieldTmpl.Execute(&buf, f); err != nil {
261274
return fmt.Sprintf("Error rendering struct field: %v", err)
262275
}
263276

264-
return buf.String()
277+
out = buf.String()
265278

266279
case SimpleField:
267280
if err := r.simpleFieldTmpl.Execute(&buf, f); err != nil {
268281
return fmt.Sprintf("Error rendering simple field: %v", err)
269282
}
270283

271-
return buf.String()
284+
out = buf.String()
272285

273286
case YamlField:
274287
if err := r.yamlFieldTmpl.Execute(&buf, f); err != nil {
275288
return fmt.Sprintf("Error rendering yaml field: %v", err)
276289
}
277290

278-
return buf.String()
291+
out = buf.String()
279292

280293
case NamedField:
281294
if err := r.namedFieldTmpl.Execute(&buf, f); err != nil {
282295
return fmt.Sprintf("Error rendering named field: %v", err)
283296
}
284297

285-
return buf.String()
298+
out = buf.String()
286299

287300
default:
288301
return fmt.Sprintf("<unknown field type: %s>", field.GetType())
289302
}
303+
304+
if !strings.HasSuffix(out, "\n\n") { // assuming double newline means intentional spacing
305+
out = strings.TrimSuffix(out, "\n")
306+
}
307+
308+
return out
290309
}

‎experimental/analyzer/renderer_text_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func TestTextRenderer_RenderField_ChainSelectorField(t *testing.T) {
142142
field := ChainSelectorField{Value: 999999}
143143
result := renderer.RenderField(NamedField{Name: "chain", Value: field}, ctx)
144144

145-
assert.Equal(t, "chain: `999999 (<chain unknown>)`\n", result)
145+
assert.Equal(t, "chain: `999999 (<chain unknown>)`", result)
146146
})
147147
}
148148

‎experimental/analyzer/templates/markdown/address_field.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
`{{.Value}}`
1+
`{{.Value}}`

‎experimental/analyzer/templates/markdown/array_field.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
array[{{.Length}}]: {{if eq .Length 0}}[]{{else}}[{{range $i, $elem := .Elements}}{{if $i}}, {{end}}{{compactValue $elem $.Context}}{{end}}{{if gt .Length 3}}, … (+{{sub .Length 3}}){{end}}]{{end}}
1+
array[{{.Length}}]: {{if eq .Length 0}}[]{{else}}[{{range $i, $elem := .Elements}}{{if $i}}, {{end}}{{compactValue $elem $.Context}}{{end}}{{if gt .Length 3}}, … (+{{sub .Length 3}}){{end}}]{{end}}

‎experimental/analyzer/templates/markdown/bytes_field.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
bytes(len={{.Length}}): {{hexPreview .Value 16}}
1+
bytes(len={{.Length}}): {{hexPreview .Value 16}}

‎experimental/analyzer/templates/markdown/proposal.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
{{range $i, $op := .Operations}}Operation #{{$i}}
22
Chain selector: {{$op.ChainSelector}} ({{$op.ChainName}})
33
{{range $call := $op.Calls}}{{indent (renderCall $call $.Context)}}{{end}}
4-
{{end}}
4+
{{end}}

0 commit comments

Comments
 (0)