From 1518ae2d362d72ac740fa07ea5eecae426cbbedb Mon Sep 17 00:00:00 2001 From: christophe-duc Date: Wed, 7 Jan 2026 18:26:39 -0400 Subject: [PATCH] fix: address Copilot PR review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add nil checks to ContainerListItem methods to prevent panics - Add nil checks in presentation/containers.go for pod and container - Sort pods by name for deterministic ordering in the UI - Make pod operation error messages consistent ("X not yet supported for pods") - Remove --color flag from pod logs for consistency with container logs 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- pkg/commands/container_list_item.go | 42 ++++++++++++++++++++--------- pkg/commands/podman.go | 13 ++++++++- pkg/gui/container_logs.go | 3 ++- pkg/gui/containers_panel.go | 16 +++++------ pkg/gui/presentation/containers.go | 7 ++++- 5 files changed, 58 insertions(+), 23 deletions(-) diff --git a/pkg/commands/container_list_item.go b/pkg/commands/container_list_item.go index 296b7b8e..54786bee 100644 --- a/pkg/commands/container_list_item.go +++ b/pkg/commands/container_list_item.go @@ -11,31 +11,40 @@ type ContainerListItem struct { // ID returns the unique ID for the item. func (c *ContainerListItem) ID() string { - if c.IsPod { + if c.IsPod && c.Pod != nil { return c.Pod.ID } - return c.Container.ID + if c.Container != nil { + return c.Container.ID + } + return "" } // Name returns the display name for the item. func (c *ContainerListItem) Name() string { - if c.IsPod { + if c.IsPod && c.Pod != nil { return c.Pod.Name } - return c.Container.Name + if c.Container != nil { + return c.Container.Name + } + return "" } // State returns the state for the item. func (c *ContainerListItem) State() string { - if c.IsPod { + if c.IsPod && c.Pod != nil { return c.Pod.State() } - return c.Container.Summary.State + if c.Container != nil { + return c.Container.Summary.State + } + return "" } // GetContainers returns the containers if this is a pod, nil otherwise. func (c *ContainerListItem) GetContainers() []*Container { - if c.IsPod { + if c.IsPod && c.Pod != nil { return c.Pod.Containers } return nil @@ -46,21 +55,30 @@ func (c *ContainerListItem) IsInPod() bool { if c.IsPod { return false } - return c.Container.Summary.Pod != "" + if c.Container != nil { + return c.Container.Summary.Pod != "" + } + return false } // PodID returns the pod ID if this container is in a pod, empty string otherwise. func (c *ContainerListItem) PodID() string { - if c.IsPod { + if c.IsPod && c.Pod != nil { return c.Pod.ID } - return c.Container.Summary.Pod + if c.Container != nil { + return c.Container.Summary.Pod + } + return "" } // PodName returns the pod name if this container is in a pod, empty string otherwise. func (c *ContainerListItem) PodName() string { - if c.IsPod { + if c.IsPod && c.Pod != nil { return c.Pod.Name } - return c.Container.Summary.PodName + if c.Container != nil { + return c.Container.Summary.PodName + } + return "" } diff --git a/pkg/commands/podman.go b/pkg/commands/podman.go index 9ccf6055..6a607fa3 100644 --- a/pkg/commands/podman.go +++ b/pkg/commands/podman.go @@ -7,6 +7,7 @@ import ( ogLog "log" "os" "os/exec" + "sort" "strings" "sync" "time" @@ -426,8 +427,18 @@ func (c *PodmanCommand) buildContainerListItems(containers []*Container, podSumm } } + // Sort pod IDs for deterministic ordering + podIDs := make([]string, 0, len(podMap)) + for podID := range podMap { + podIDs = append(podIDs, podID) + } + sort.Slice(podIDs, func(i, j int) bool { + return podMap[podIDs[i]].Name < podMap[podIDs[j]].Name + }) + // Add pods and their containers - for podID, ps := range podMap { + for _, podID := range podIDs { + ps := podMap[podID] // Create pod object pod := &Pod{ ID: ps.ID, diff --git a/pkg/gui/container_logs.go b/pkg/gui/container_logs.go index 931f72f1..9115d436 100644 --- a/pkg/gui/container_logs.go +++ b/pkg/gui/container_logs.go @@ -175,7 +175,8 @@ func (gui *Gui) renderPodLogsToMainAux(pod *commands.Pod, ctx context.Context, n func (gui *Gui) writePodLogs(pod *commands.Pod, ctx context.Context, writer io.Writer) error { // Build podman pod logs command - args := []string{"pod", "logs", "--follow", "--color"} + // Note: --color is used to distinguish output from different containers in the pod + args := []string{"pod", "logs", "--follow"} if gui.Config.UserConfig.Logs.Timestamps { args = append(args, "--timestamps") diff --git a/pkg/gui/containers_panel.go b/pkg/gui/containers_panel.go index be0e0795..9a42fc82 100644 --- a/pkg/gui/containers_panel.go +++ b/pkg/gui/containers_panel.go @@ -441,7 +441,7 @@ func (gui *Gui) handleContainersRemoveMenu(g *gocui.Gui, v *gocui.View) error { } if item.IsPod { - return gui.createErrorPanel("Pod operations not yet supported") + return gui.createErrorPanel("Remove not yet supported for pods") } ctr := item.Container @@ -501,7 +501,7 @@ func (gui *Gui) handleContainerPause(g *gocui.Gui, v *gocui.View) error { } if item.IsPod { - return gui.createErrorPanel("Pod operations not yet supported") + return gui.createErrorPanel("Pause not yet supported for pods") } return gui.PauseContainer(item.Container) @@ -514,7 +514,7 @@ func (gui *Gui) handleContainerStop(g *gocui.Gui, v *gocui.View) error { } if item.IsPod { - return gui.createErrorPanel("Pod operations not yet supported") + return gui.createErrorPanel("Stop not yet supported for pods") } ctr := item.Container @@ -536,7 +536,7 @@ func (gui *Gui) handleContainerRestart(g *gocui.Gui, v *gocui.View) error { } if item.IsPod { - return gui.createErrorPanel("Pod operations not yet supported") + return gui.createErrorPanel("Restart not yet supported for pods") } ctr := item.Container @@ -556,7 +556,7 @@ func (gui *Gui) handleContainerAttach(g *gocui.Gui, v *gocui.View) error { } if item.IsPod { - return gui.createErrorPanel("Pod operations not yet supported") + return gui.createErrorPanel("Attach not yet supported for pods") } ctr := item.Container @@ -588,7 +588,7 @@ func (gui *Gui) handleContainerViewLogs(g *gocui.Gui, v *gocui.View) error { if item.IsPod { // TODO: implement pod logs to stdout - return gui.createErrorPanel("Pod logs to stdout not yet supported") + return gui.createErrorPanel("View logs (stdout) not yet supported for pods") } gui.renderLogsToStdout(item.Container) @@ -603,7 +603,7 @@ func (gui *Gui) handleContainersExecShell(g *gocui.Gui, v *gocui.View) error { } if item.IsPod { - return gui.createErrorPanel("Cannot exec into a pod. Select a container instead.") + return gui.createErrorPanel("Exec shell not yet supported for pods. Select a container instead.") } return gui.containerExecShell(item.Container) @@ -628,7 +628,7 @@ func (gui *Gui) handleContainersCustomCommand(g *gocui.Gui, v *gocui.View) error } if item.IsPod { - return gui.createErrorPanel("Custom commands not yet supported for pods") + return gui.createErrorPanel("Custom commands not yet supported for pods. Select a container instead.") } commandObject := gui.PodmanCommand.NewCommandObject(commands.CommandObject{ diff --git a/pkg/gui/presentation/containers.go b/pkg/gui/presentation/containers.go index fe7a5a74..2d74ccdc 100644 --- a/pkg/gui/presentation/containers.go +++ b/pkg/gui/presentation/containers.go @@ -26,10 +26,15 @@ func GetContainerDisplayStrings(guiConfig *config.GuiConfig, container *commands // GetContainerListItemDisplayStrings returns display strings for a ContainerListItem (pod or container) func GetContainerListItemDisplayStrings(guiConfig *config.GuiConfig, item *commands.ContainerListItem) []string { - if item.IsPod { + if item.IsPod && item.Pod != nil { return GetPodDisplayStrings(guiConfig, item.Pod) } + if item.Container == nil { + // Return an empty row with the expected number of columns when container data is missing. + return []string{"", "", "", "", "", ""} + } + // Add indentation for containers in pods strings := GetContainerDisplayStrings(guiConfig, item.Container) if item.Indent > 0 {