Skip to content

feat: enhance progress bar status handling and alignment #845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ilopezluna merged 1 commit into from
Apr 8, 2026
Merged

feat: enhance progress bar status handling and alignment #845

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 33 additions & 11 deletions cmd/cli/desktop/progress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,37 +176,57 @@ func displayProgressSimple(body io.Reader, printer standalone.StatusPrinter) (st
return finalMessage, progressShown, nil
}

// Status strings used in progress display. All are padded to
// progressStatusWidth so that progress bars line up at the same column.
const (
progressStatusWaiting = "Waiting"
progressStatusDownloading = "Downloading"
progressStatusPullComplete = "Pull complete"
progressStatusUploading = "Uploading"
progressStatusPushComplete = "Push complete"

// progressStatusWidth is the column width to which all status strings
// are left-padded, keeping progress bars horizontally aligned.
progressStatusWidth = max(
len(progressStatusWaiting),
len(progressStatusDownloading),
len(progressStatusPullComplete),
len(progressStatusUploading),
len(progressStatusPushComplete),
)
Comment on lines +190 to +196
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Critical

  • Compatibility Issue: The max built-in function was introduced in Go 1.21. Using it within a const block requires the project to be built with Go 1.21 or later. If this CLI tool needs to support older Go versions (which is common for distribution), this will cause a build failure.

Suggested fix: Use a literal constant or a var block with an init() function if compatibility with Go < 1.21 is required.

	// progressStatusWidth is the column width to which all status strings
	// are left-padded, keeping progress bars horizontally aligned.
	progressStatusWidth = 13
References
  1. Pragmatism: Use the simplest viable approach that maintains compatibility with the project's environment. (link)

)

// writeDockerProgress writes a progress update in Docker's JSONMessage format
func writeDockerProgress(w io.Writer, msg *oci.ProgressMessage) error {
layerID := msg.Layer.ID
if layerID == "" {
return nil
}

// Detect if this is a push operation based on the sentinel layer ID
isPush := msg.Mode == "push"
// Detect if this is a push operation.
isPush := msg.Mode == oci.ModePush

// Determine status based on progress
// Determine status based on progress.
var status string
var progressDetail *jsonstream.Progress

if msg.Layer.Current == 0 {
status = "Waiting"
status = progressStatusWaiting
} else if msg.Layer.Current < msg.Layer.Size {
if isPush {
status = "Uploading"
status = progressStatusUploading
} else {
status = "Downloading"
status = progressStatusDownloading
}
progressDetail = &jsonstream.Progress{
Current: int64(msg.Layer.Current),
Total: int64(msg.Layer.Size),
}
} else if msg.Layer.Current >= msg.Layer.Size && msg.Layer.Size > 0 {
if isPush {
status = "Push complete"
status = progressStatusPushComplete
} else {
status = "Pull complete"
status = progressStatusPullComplete
}
progressDetail = &jsonstream.Progress{
Current: int64(msg.Layer.Current),
Expand All @@ -218,15 +238,17 @@ func writeDockerProgress(w io.Writer, msg *oci.ProgressMessage) error {
return nil
}

// Shorten layer ID for display (similar to Docker)
// Shorten layer ID for display (similar to Docker).
displayID := strings.TrimPrefix(layerID, "sha256:")
if len(displayID) > 12 {
displayID = displayID[:12]
}

dockerMsg := jsonstream.Message{
ID: displayID,
Status: status,
ID: displayID,
// Pad status to a fixed width so all progress bars start at the
Comment on lines 247 to +249
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question (bug_risk): Padding the Status field may affect downstream consumers expecting exact status strings.

Previously Status contained the raw value (e.g., "Downloading"), but now it includes trailing spaces for alignment. If any callers or external tools do string equality checks or log Status as-is, this change could break them. Please confirm Status is only used for display, or consider doing the padding only in the presentation layer (or via a separate display field).

// same column regardless of status string length.
Status: fmt.Sprintf("%-*s", progressStatusWidth, status),
Progress: progressDetail,
}

Expand Down
Loading