Skip to content
This repository was archived by the owner on Oct 6, 2025. It is now read-only.

Comments

cli: fix autocompletion to work with context detection#58

Merged
xenoscopic merged 1 commit intomainfrom
fix-completion
May 14, 2025
Merged

cli: fix autocompletion to work with context detection#58
xenoscopic merged 1 commit intomainfrom
fix-completion

Conversation

@xenoscopic
Copy link
Contributor

The autocomplete didn't function after context detection because (1) it had a stale (nil) client reference and (2) CLI initialization wasn't occurring on the autocomplete path.

Signed-off-by: Jacob Howard <jacob.howard@docker.com>
@xenoscopic xenoscopic requested a review from doringeman May 14, 2025 15:56
return nil, cobra.ShellCompDirectiveNoFileComp
}
models, err := desktopClient.List()
models, err := desktopClient().List()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we just use desktopClient *desktop.Client if we're running cmd.Parent().PersistentPreRunE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose we could, but it would create a circular dependency, and passing the value through at command construction captures the nil value.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I see.

Copy link
Collaborator

@doringeman doringeman left a comment

Choose a reason for hiding this comment

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

Thanks!

@xenoscopic xenoscopic merged commit 083bf0f into main May 14, 2025
4 checks passed
@xenoscopic xenoscopic deleted the fix-completion branch May 14, 2025 16:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants