command/format: improve list nested attribute rendering

[bluekeyes]

This change contains two commits, the first addressing a bug and the second improving general rendering in the same area. I'm submitting them together, but would be happy to split them up or only submit the bug fix.

Bug: Lists that mix changed and unchanged elements did not render

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.

Before:

  # nested_fields.example will be updated in-place
  ~ resource "nested_fields" "example" {
        name   = "example"
      ~ values = [
          ~ {          },
          # (3 unchanged elements hidden)
        ]
    }

After:

  # nested_fields.example will be updated in-place
  ~ resource "nested_fields" "example" {
        name   = "example"
      ~ values = [
          ~ {
              name  = "b"
            ~ value = "2" -> "3"
          },
          # (3 unchanged elements hidden)
        ]
    }

Formatting Improvements

I made several changes to make list diffs 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

Before:

  # nested_fields.example will be updated in-place
  ~ resource "nested_fields" "example" {
        name   = "example"
      ~ values = [
          ~ {
              name  = "c"
            ~ value = "3" -> "c"

            + name  = "e"
            + value = "5"
          },
          # (3 unchanged elements hidden)
        ]
    }

After:

  # nested_fields.example will be updated in-place
  ~ resource "nested_fields" "example" {
        name   = "example"
      ~ values = [
          ~ {
                name  = "c"
              ~ value = "3" -> "c"
            },
          + {
              + name  = "e"
              + value = "5"
            },
            # (3 unchanged elements hidden)
        ]
    }

Context

I'm writing a provider using the new terraform-plugin-framework that uses the ListNestedAttributes type in several places. I extracted the relevant logic into a sample provider to experiment more after I observed strange plan output.

[alisdair]

Thanks very much for making these fixes and improvements! I spotted one struct reuse bug to fix before merge.

One problem that this PR is that we're not showing any context for changed nested list items, which we do when rendering sequence attributes. The code that handles that for lists and tuples is here:

terraform/internal/command/format/diff.go

Lines 1354 to 1436 in 834f61c

	// Maintain a stack of suppressed lines in the diff for later
	// display or elision
	var suppressedElements []*plans.Change
	var changeShown bool
	for i := 0; i < len(elemDiffs); i++ {
	if !p.verbose {
	for i < len(elemDiffs) && elemDiffs[i].Action == plans.NoOp {
	suppressedElements = append(suppressedElements, elemDiffs[i])
	i++
	}
	}
	// If we have some suppressed elements on the stack…
	if len(suppressedElements) > 0 {
	// If we've just rendered a change, display the first
	// element in the stack as context
	if changeShown {
	elemDiff := suppressedElements[0]
	p.buf.WriteString(strings.Repeat(" ", indent+4))
	p.writeValue(elemDiff.After, elemDiff.Action, indent+4)
	p.buf.WriteString(",\n")
	suppressedElements = suppressedElements[1:]
	}
	hidden := len(suppressedElements)
	// If we're not yet at the end of the list, capture the
	// last element on the stack as context for the upcoming
	// change to be rendered
	var nextContextDiff *plans.Change
	if hidden > 0 && i < len(elemDiffs) {
	hidden--
	nextContextDiff = suppressedElements[hidden]
	}
	// If there are still hidden elements, show an elision
	// statement counting them
	if hidden > 0 {
	p.writeActionSymbol(plans.NoOp)
	p.buf.WriteString(strings.Repeat(" ", indent+2))
	noun := "elements"
	if hidden == 1 {
	noun = "element"
	}
	p.buf.WriteString(fmt.Sprintf(p.color.Color("[dark_gray]# (%d unchanged %s hidden)[reset]"), hidden, noun))
	p.buf.WriteString("\n")
	}
	// Display the next context diff if it was captured above
	if nextContextDiff != nil {
	p.buf.WriteString(strings.Repeat(" ", indent+4))
	p.writeValue(nextContextDiff.After, nextContextDiff.Action, indent+4)
	p.buf.WriteString(",\n")
	}
	// Suppressed elements have now been handled so clear them again
	suppressedElements = nil
	}
	if i >= len(elemDiffs) {
	break
	}
	elemDiff := elemDiffs[i]
	p.buf.WriteString(strings.Repeat(" ", indent+2))
	p.writeActionSymbol(elemDiff.Action)
	switch elemDiff.Action {
	case plans.NoOp, plans.Delete:
	p.writeValue(elemDiff.Before, elemDiff.Action, indent+4)
	case plans.Update:
	p.writeValueDiff(elemDiff.Before, elemDiff.After, indent+4, path)
	case plans.Create:
	p.writeValue(elemDiff.After, elemDiff.Action, indent+4)
	default:
	// Should never happen since the above covers all
	// actions that ctySequenceDiff can return.
	p.writeValue(elemDiff.After, elemDiff.Action, indent+4)
	}
	p.buf.WriteString(",\n")
	changeShown = true
	}

If you happen to have ideas around how to bring that same functionality across here, it would be great to hear them. Doesn't block merging this PR, though!

[alisdair]

We're reusing the same blockBodyDiffResult value for multiple calls here, which means that the skippedAttributes field will continue to increment with every element. Screenshot example, where all of these values have one skipped attribute:

[bluekeyes]

Good catch, fixed and extended the tests to cover this.

[bluekeyes]

One problem that this PR is that we're not showing any context for changed nested list items, which we do when rendering sequence attributes.

I noticed this as well. An initial version of this change partially addressed it by inserting the (N unchanged elements hidden) message at each each place in the list where elements were suppressed, which at least lets the user infer the index that is changing. I removed it because it felt noisy and didn't seem to fit with the other suppression messages (which always appear at the end of the element), but it was pretty easy to implement if you'd like me to add it again.

Beyond that, I'll think about how to provide better context using the linked code as an example, but I don't know if I'll have time to implement it in the near future.

[alisdair]

Thanks, looks great! Merging. The additional sequence context we discussed is just a nice-to-have—maybe we'll find a way to add that in a future release.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.