jprdonnelly/opentf
1
0
Fork 0
mirror of https://github.com/opentffoundation/opentf.git synced 2025-12-25 01:00:16 -05:00
Code Issues Packages Projects Releases Wiki Activity

command/format: improve list nested attribute rendering (#29827)

Browse Source 
* command/format: fix list nested attr diff rendering

Previously, diffs only rendered correctly if all changed elements
appeared before all unchanged elements. Once an unchanged element was
found, all remaining elements were skipped. This usually led to the
output being an empty list with a weird amount of space between the
brackets.

* command/format: improve list nested attr rendering

This makes several changes that make diffs for lists clearer and more
consistent:

  * Separate items with brackets instead of only new lines. This better
    matches the input syntax and avoids confusion from the first and
    last brackets implying there is a single item.
  * Render an action symbol for each element of the list
  * Use the correct action symbol for new and deleted items
  * Fix the alignment of opening and closing brackets

I also refactored the structure so it is similar to the set and map
cases to minimize duplication of the new prints.

* Fix re-use of blockBodyDiffResult struct
This commit is contained in:
Billy Keyes
2021-11-02 11:13:56 -04:00
committed by GitHub
parent 12651c0a81
commit 52ca30273f
2 changed files with 140 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

70
internal/command/format/diff.go
View File

@@ -473,9 +473,6 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
p.buf.WriteString(p.color.Color(forcesNewResourceCaption))
}
p.buf.WriteString("\n")
p.buf.WriteString(strings.Repeat(" ", indent+4))
p.writeActionSymbol(action)
p.buf.WriteString("{")
oldItems := ctyCollectionValues(old)
newItems := ctyCollectionValues(new)
@@ -489,48 +486,65 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
// commonLen is number of elements that exist in both lists, which
// will be presented as updates (~). Any additional items in one
// of the lists will be presented as either creates (+) or deletes (-)
// depending on which list they belong to.
// depending on which list they belong to. maxLen is the number of
// elements in that longer list.
var commonLen int
var maxLen int
// unchanged is the number of unchanged elements
var unchanged int
switch {
case len(oldItems) < len(newItems):
commonLen = len(oldItems)
maxLen = len(newItems)
default:
commonLen = len(newItems)
maxLen = len(oldItems)
}
for i := 0; i < commonLen; i++ {
for i := 0; i < maxLen; i++ {
path := append(path, cty.IndexStep{Key: cty.NumberIntVal(int64(i))})
oldItem := oldItems[i]
newItem := newItems[i]
if oldItem.RawEquals(newItem) {
var action plans.Action
var oldItem, newItem cty.Value
switch {
case i < commonLen:
oldItem = oldItems[i]
newItem = newItems[i]
if oldItem.RawEquals(newItem) {
action = plans.NoOp
unchanged++
} else {
action = plans.Update
}
case i < len(oldItems):
oldItem = oldItems[i]
newItem = cty.NullVal(oldItem.Type())
action = plans.Delete
case i < len(newItems):
newItem = newItems[i]
oldItem = cty.NullVal(newItem.Type())
action = plans.Create
default:
action = plans.NoOp
unchanged++
}
if action != plans.NoOp {
p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result)
p.writeSkippedAttr(result.skippedAttributes, indent+8)
p.buf.WriteString(strings.Repeat(" ", indent+4))
p.writeActionSymbol(action)
p.buf.WriteString("{")
result := &blockBodyDiffResult{}
p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+8, path, result)
if action == plans.Update {
p.writeSkippedAttr(result.skippedAttributes, indent+10)
}
p.buf.WriteString("\n")
p.buf.WriteString(strings.Repeat(" ", indent+6))
p.buf.WriteString("},\n")
}
}
for i := commonLen; i < len(oldItems); i++ {
path := append(path, cty.IndexStep{Key: cty.NumberIntVal(int64(i))})
oldItem := oldItems[i]
newItem := cty.NullVal(oldItem.Type())
p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result)
p.buf.WriteString("\n")
}
for i := commonLen; i < len(newItems); i++ {
path := append(path, cty.IndexStep{Key: cty.NumberIntVal(int64(i))})
newItem := newItems[i]
oldItem := cty.NullVal(newItem.Type())
p.writeAttrsDiff(objS.Attributes, oldItem, newItem, indent+6, path, result)
p.buf.WriteString("\n")
}
p.buf.WriteString(strings.Repeat(" ", indent+4))
p.buf.WriteString("},\n")
p.writeSkippedElems(unchanged, indent+4)
p.writeSkippedElems(unchanged, indent+6)
p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString("]")

123
internal/command/format/diff_test.go
View File

@@ -2263,10 +2263,10 @@ func TestResourceChange_nestedList(t *testing.T) {
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
~ {
+ mount_point = "/var/diska"
+ size = "50GB"
},
+ {
+ mount_point = "/var/diska"
+ size = "50GB"
},
]
id = "i-02ae66f368e8518a9"
@@ -2309,9 +2309,9 @@ func TestResourceChange_nestedList(t *testing.T) {
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
~ {
+ mount_point = "/var/diska"
},
+ {
+ mount_point = "/var/diska"
},
]
id = "i-02ae66f368e8518a9"
@@ -2371,10 +2371,10 @@ func TestResourceChange_nestedList(t *testing.T) {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
~ {
+ size = "50GB"
# (1 unchanged attribute hidden)
},
# (1 unchanged element hidden)
+ size = "50GB"
# (1 unchanged attribute hidden)
},
# (1 unchanged element hidden)
]
id = "i-02ae66f368e8518a9"
@@ -2437,9 +2437,9 @@ func TestResourceChange_nestedList(t *testing.T) {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
~ {
~ mount_point = "/var/diska" -> "/var/diskb" # forces replacement
# (1 unchanged attribute hidden)
},
~ mount_point = "/var/diska" -> "/var/diskb" # forces replacement
# (1 unchanged attribute hidden)
},
]
id = "i-02ae66f368e8518a9"
@@ -2493,9 +2493,9 @@ func TestResourceChange_nestedList(t *testing.T) {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [ # forces replacement
~ {
~ mount_point = "/var/diska" -> "/var/diskb"
# (1 unchanged attribute hidden)
},
~ mount_point = "/var/diska" -> "/var/diskb"
# (1 unchanged attribute hidden)
},
]
id = "i-02ae66f368e8518a9"
@@ -2540,10 +2540,10 @@ func TestResourceChange_nestedList(t *testing.T) {
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
~ {
- mount_point = "/var/diska" -> null
- size = "50GB" -> null
},
- {
- mount_point = "/var/diska" -> null
- size = "50GB" -> null
},
]
id = "i-02ae66f368e8518a9"
@@ -2674,13 +2674,86 @@ func TestResourceChange_nestedList(t *testing.T) {
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
~ {
- mount_point = "/var/diska" -> null
- size = "50GB" -> null
},
- {
- mount_point = "/var/diska" -> null
- size = "50GB" -> null
},
] -> (known after apply)
id = "i-02ae66f368e8518a9"
# (1 unchanged block hidden)
}
`,
},
"in-place update - modification": {
Action: plans.Update,
Mode: addrs.ManagedResourceMode,
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-BEFORE"),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diskb"),
"size": cty.StringVal("50GB"),
}),
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diskc"),
"size": cty.StringVal("50GB"),
}),
}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
"new_field": cty.StringVal("new_value"),
}),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-AFTER"),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diskb"),
"size": cty.StringVal("75GB"),
}),
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diskc"),
"size": cty.StringVal("25GB"),
}),
}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
"new_field": cty.StringVal("new_value"),
}),
}),
}),
RequiredReplace: cty.NewPathSet(),
Schema: testSchemaPlus(configschema.NestingList),
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
~ {
~ size = "50GB" -> "75GB"
# (1 unchanged attribute hidden)
},
~ {
~ size = "50GB" -> "25GB"
# (1 unchanged attribute hidden)
},
# (1 unchanged element hidden)
]
id = "i-02ae66f368e8518a9"
# (1 unchanged block hidden)
}
`,