Skip to content

Commit

Permalink
Structured Plan Renderer: another round of fixes after more equivalen…
Browse files Browse the repository at this point in the history
…ce testing (#32537)

* Structured Plan Renderer: another round of fixes after more equivalence testing

* fix sort function

* use new constants
  • Loading branch information
liamcervante committed Jan 20, 2023
1 parent 125aaef commit 7d1ea52
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 12 deletions.
12 changes: 9 additions & 3 deletions internal/command/jsonformat/computed/renderers/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ var _ computed.DiffRenderer = (*mapRenderer)(nil)

func Map(elements map[string]computed.Diff) computed.DiffRenderer {
return &mapRenderer{
elements: elements,
elements: elements,
alignKeys: true,
}
}

Expand All @@ -34,6 +35,7 @@ type mapRenderer struct {

overrideNullSuffix bool
overrideForcesReplacement bool
alignKeys bool
}

func (renderer mapRenderer) RenderHuman(diff computed.Diff, indent int, opts computed.RenderHumanOpts) string {
Expand Down Expand Up @@ -83,14 +85,18 @@ func (renderer mapRenderer) RenderHuman(diff computed.Diff, indent int, opts com
for _, warning := range element.WarningsHuman(indent+1, opts) {
buf.WriteString(fmt.Sprintf("%s%s\n", formatIndent(indent+1), warning))
}

// Only show commas between elements for objects.
comma := ""
if _, ok := element.Renderer.(*objectRenderer); ok {
comma = ","
}

buf.WriteString(fmt.Sprintf("%s%s %-*s = %s%s\n", formatIndent(indent+1), colorizeDiffAction(element.Action, opts), maximumKeyLen, escapedKeys[key], element.RenderHuman(indent+1, elementOpts), comma))
if renderer.alignKeys {
buf.WriteString(fmt.Sprintf("%s%s %-*s = %s%s\n", formatIndent(indent+1), colorizeDiffAction(element.Action, opts), maximumKeyLen, escapedKeys[key], element.RenderHuman(indent+1, elementOpts), comma))
} else {
buf.WriteString(fmt.Sprintf("%s%s %s = %s%s\n", formatIndent(indent+1), colorizeDiffAction(element.Action, opts), escapedKeys[key], element.RenderHuman(indent+1, elementOpts), comma))
}

}

if unchangedElements > 0 {
Expand Down
4 changes: 3 additions & 1 deletion internal/command/jsonformat/computed/renderers/primitive.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package renderers

import (
"fmt"
"math/big"
"strings"

"github.com/zclconf/go-cty/cty"
Expand Down Expand Up @@ -63,7 +64,8 @@ func renderPrimitiveValue(value interface{}, t cty.Type, opts computed.RenderHum
}
return "false"
case t == cty.Number:
return fmt.Sprintf("%g", value)
bf := big.NewFloat(value.(float64))
return bf.Text('f', -1)
default:
panic("unrecognized primitive type: " + t.FriendlyName())
}
Expand Down
59 changes: 59 additions & 0 deletions internal/command/jsonformat/computed/renderers/renderer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ func TestRenderers_Human(t *testing.T) {
},
expected: "0 -> 1",
},
"primitive_update_long_float": {
diff: computed.Diff{
Renderer: Primitive(123456789.0, 987654321.0, cty.Number),
Action: plans.Update,
},
expected: "123456789 -> 987654321",
},
"primitive_update_replace": {
diff: computed.Diff{
Renderer: Primitive(0.0, 1.0, cty.Number),
Expand Down Expand Up @@ -830,6 +837,58 @@ jsonencode(
{
~ "element_one" = 1 -> (known after apply)
}
`,
},
"map_aligns_key": {
diff: computed.Diff{
Renderer: Map(map[string]computed.Diff{
"element_one": {
Renderer: Primitive(1.0, 2.0, cty.Number),
Action: plans.Update,
},
"element_two": {
Renderer: Primitive(1.0, 3.0, cty.Number),
Action: plans.Update,
},
"element_three": {
Renderer: Primitive(1.0, 4.0, cty.Number),
Action: plans.Update,
},
}),
Action: plans.Update,
},
expected: `
{
~ "element_one" = 1 -> 2
~ "element_three" = 1 -> 4
~ "element_two" = 1 -> 3
}
`,
},
"nested_map_does_not_align_keys": {
diff: computed.Diff{
Renderer: NestedMap(map[string]computed.Diff{
"element_one": {
Renderer: Primitive(1.0, 2.0, cty.Number),
Action: plans.Update,
},
"element_two": {
Renderer: Primitive(1.0, 3.0, cty.Number),
Action: plans.Update,
},
"element_three": {
Renderer: Primitive(1.0, 4.0, cty.Number),
Action: plans.Update,
},
}),
Action: plans.Update,
},
expected: `
{
~ "element_one" = 1 -> 2
~ "element_three" = 1 -> 4
~ "element_two" = 1 -> 3
}
`,
},
"list_create_empty": {
Expand Down
21 changes: 16 additions & 5 deletions internal/command/jsonformat/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,23 @@ func precomputeDiffs(plan Plan, mode plans.Mode) diffs {

less := func(drs []diff) func(i, j int) bool {
return func(i, j int) bool {
iA := drs[i].change.Address
jA := drs[j].change.Address
if iA == jA {
return drs[i].change.Deposed < drs[j].change.Deposed
left := drs[i].change
right := drs[j].change

if left.ModuleAddress != right.ModuleAddress {
return left.ModuleAddress < right.ModuleAddress
}

if left.Mode != right.Mode {
return left.Mode == jsonplan.DataResourceMode
}
return iA < jA

if left.Address != right.Address {
return left.Address < right.Address
}

// Everything else being equal, we'll sort by deposed.
return left.Deposed < right.Deposed
}
}

Expand Down
6 changes: 3 additions & 3 deletions internal/command/jsonformat/renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ func (r Renderer) RenderHumanPlan(plan Plan, mode plans.Mode, opts ...RendererOp
}

changes = append(changes, diff)
willPrintResourceChanges = true

// Don't count move-only changes
if action != plans.NoOp {
willPrintResourceChanges = true
counts[action]++
}
}
Expand Down Expand Up @@ -325,12 +325,12 @@ func (r Renderer) renderHumanDiffDrift(diffs diffs, mode plans.Mode) bool {
switch mode {
case plans.RefreshOnlyMode:
r.Streams.Println(format.WordWrap(
"\nThis is a refresh-only plan, so Terraform will not take any actions to undo these. If you were expecting these changes then you can apply this plan to record the updated values in the Terraform state without changing any remote objects.",
"\n\nThis is a refresh-only plan, so Terraform will not take any actions to undo these. If you were expecting these changes then you can apply this plan to record the updated values in the Terraform state without changing any remote objects.",
r.Streams.Stdout.Columns(),
))
default:
r.Streams.Println(format.WordWrap(
"\nUnless you have made equivalent changes to your configuration, or ignored the relevant attributes using ignore_changes, the following plan may include actions to undo or respond to these changes.",
"\n\nUnless you have made equivalent changes to your configuration, or ignored the relevant attributes using ignore_changes, the following plan may include actions to undo or respond to these changes.",
r.Streams.Stdout.Columns(),
))
}
Expand Down

0 comments on commit 7d1ea52

Please sign in to comment.