fix: make newlines consistent and fix default value for format flag#548
Conversation
|
There was a problem hiding this comment.
Pull Request Overview
This PR standardizes newline handling in template rendering and improves format flag validation. The changes ensure consistent output formatting by removing trailing newlines from template outputs (except when double newlines indicate intentional spacing) and add proper error handling for invalid format flag values.
Key changes:
- Removes trailing newlines from template files and updates corresponding test expectations
- Adds trimming logic to renderer methods to handle newlines consistently
- Changes
createRendererFromFormatto return an error for invalid format values instead of silently defaulting
Reviewed Changes
Copilot reviewed 7 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| experimental/analyzer/templates/text/named_field.tmpl | Removes trailing newline from template file |
| experimental/analyzer/renderer_text_test.go | Updates test assertion to match new output without trailing newline |
| experimental/analyzer/renderer_text.go | Adds newline trimming logic to rendering methods and imports strings package |
| experimental/analyzer/renderer_markdown.go | Adds newline trimming logic to template rendering helper |
| experimental/analyzer/describe_test.go | Updates test expectations to reflect newline handling changes |
| experimental/analyzer/decoded_call_test.go | Removes blank line from expected output in test |
| engine/cld/legacy/cli/mcmsv2/mcms_v2.go | Updates format flag handling to return errors for invalid values and handle empty string case |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Default to markdown if format is not specified or invalid | ||
| return analyzer.NewMarkdownRenderer() | ||
| return nil, fmt.Errorf("unknown format '%s'", format) |
There was a problem hiding this comment.
The comment is now inaccurate. Since the function returns an error for unknown formats instead of defaulting to markdown, the comment should be updated to reflect this behavior. Consider: '// Return error for unknown formats'.
|
| } | ||
|
|
||
| return buf.String() | ||
| out := buf.String() |
There was a problem hiding this comment.
nit/noaction: I still think you could avoid the undesired new line characters by inserting the appropriate dashes to the templates.
There was a problem hiding this comment.
Let me just use one example. I find the suggested patch more readable and more maintainable:
index 32037d2..e9fafa9 100644
--- i/experimental/analyzer/templates/text/decoded_call.tmpl
+++ w/experimental/analyzer/templates/text/decoded_call.tmpl
@@ -1,9 +1,14 @@
Address: {{.Address}}
Method: {{.Method}}
-{{if .Inputs}}
+{{- if .Inputs}}
Inputs:
-{{range .Inputs}} {{.Name}}: {{.Summary}}{{if .Details}} {{.Details}}{{end}}
-{{end}}{{end}}{{if .Outputs}}{{if .Inputs}}
-{{end}}Outputs:
-{{range .Outputs}} {{.Name}}: {{.Summary}}{{if .Details}} {{.Details}}{{end}}
-{{end}}{{end}}
+{{- range .Inputs}}
+ {{.Name}}: {{.Summary}}{{if .Details}} {{.Details}}{{end}}
+{{- end}}
+{{- end}}
+{{- if .Outputs}}
+ Outputs:
+{{- range .Outputs}}
+ {{.Name}}: {{.Summary}}{{if .Details}} {{.Details}}{{end}}
+{{- end}}
+{{- end}}
Some small fixes as a followup from the proposal analyzer refactor:
Tried using:
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.