From 8219c922a2e1fce68800924f9f9dd47198badf6b Mon Sep 17 00:00:00 2001
From: Thomas Gummerer <t.gummerer@gmail.com>
Date: Fri, 26 Apr 2024 11:54:21 -0300
Subject: [PATCH] display: only hide replacement steps in diff (#16065)

When displaying diff events, we currently try to hide non-logical
replacement steps unless we specifically enable showing them. However we
currently do that for all non-logical operations, regardless whether
they are replacement steps or not.

In particular a RefreshStep is non-logical, but it's also not a
replacement step. We still want to show them during the diff because
their output can be important. Especially if the user just requested a
diff it doesn't make sense to hide the diff from them at the same time.

The intention here is to only hide replacement steps, so do that.

The full diff with the display tests is here:
https://gist.github.com/tgummerer/fcd012f13669a9cdc39530cde7770260 It's
unedited, so it includes some flakyness which isn't interesting.

I looked it over, and I think it looks like what we want, but I'm
curious to hear what others think. E.g.
https://gist.github.com/tgummerer/fcd012f13669a9cdc39530cde7770260#file-testdata-diff-L558
looks more correct now, as it shows the two delete operation that
actually happened, that it didn't show before, and it still shows the
same operation (Calling this one out in particular, since it took me a
bit to understand that we still have the same operation in the diff)

Fixes https://github.com/pulumi/pulumi/issues/7665
---
 ...isplay-making-sure-it-shows-diffs-from-refreshes.yaml | 4 ++++
 pkg/backend/display/display.go                           | 8 +++++---
 pkg/resource/deploy/step.go                              | 9 +++++++++
 3 files changed, 18 insertions(+), 3 deletions(-)
 create mode 100644 changelog/pending/20240426--cli-display--fix-output-of-the-diff-display-making-sure-it-shows-diffs-from-refreshes.yaml

diff --git a/changelog/pending/20240426--cli-display--fix-output-of-the-diff-display-making-sure-it-shows-diffs-from-refreshes.yaml b/changelog/pending/20240426--cli-display--fix-output-of-the-diff-display-making-sure-it-shows-diffs-from-refreshes.yaml
new file mode 100644
index 000000000000..44b64a4184d1
--- /dev/null
+++ b/changelog/pending/20240426--cli-display--fix-output-of-the-diff-display-making-sure-it-shows-diffs-from-refreshes.yaml
@@ -0,0 +1,4 @@
+changes:
+- type: fix
+  scope: cli/display
+  description: Fix output of the diff display, making sure it shows diffs from refreshes
diff --git a/pkg/backend/display/display.go b/pkg/backend/display/display.go
index be99af23de81..ef4125852ce7 100644
--- a/pkg/backend/display/display.go
+++ b/pkg/backend/display/display.go
@@ -199,10 +199,12 @@ func shouldShow(step engine.StepEventMetadata, opts Options) bool {
 		return opts.ShowSameResources
 	}
 
-	// For logical replacement operations, only show them during progress-style updates (since this is integrated
+	// For non-logical replacement operations, only show them during progress-style updates (since this is integrated
 	// into the resource status update), or if it is requested explicitly (for diffs and JSON outputs).
-	if (opts.Type == DisplayDiff || opts.JSONDisplay) && !step.Logical && !opts.ShowReplacementSteps {
-		return false
+	if !opts.ShowReplacementSteps {
+		if (opts.Type == DisplayDiff || opts.JSONDisplay) && !step.Logical && deploy.IsReplacementStep(step.Op) {
+			return false
+		}
 	}
 
 	// Otherwise, default to showing the operation.
diff --git a/pkg/resource/deploy/step.go b/pkg/resource/deploy/step.go
index bd0872d4271a..ec76492577ec 100644
--- a/pkg/resource/deploy/step.go
+++ b/pkg/resource/deploy/step.go
@@ -1300,6 +1300,15 @@ var StepOps = []display.StepOp{
 	OpImportReplacement,
 }
 
+func IsReplacementStep(op display.StepOp) bool {
+	if op == OpReplace || op == OpCreateReplacement || op == OpDeleteReplaced ||
+		op == OpReadReplacement || op == OpDiscardReplaced || op == OpRemovePendingReplace ||
+		op == OpImportReplacement {
+		return true
+	}
+	return false
+}
+
 // Color returns a suggested color for lines of this op type.
 func Color(op display.StepOp) string {
 	switch op {