Skip to content

Fix completion with direct connections#378

Open
codeafridi wants to merge 2 commits into
psviderski:mainfrom
codeafridi:connect-flag
Open

Fix completion with direct connections#378
codeafridi wants to merge 2 commits into
psviderski:mainfrom
codeafridi:connect-flag

Conversation

@codeafridi

@codeafridi codeafridi commented May 26, 2026

Copy link
Copy Markdown

fixes #377

Summary

  • Initialize the CLI before Cobra completion runs.
  • Read global flags during completion so --connect works without a config file.
  • Disable connection progress output during completion.

Testing

  • GOCACHE=/tmp/go-build GOMODCACHE=/tmp/go-mod go test ./cmd/uncloud ./internal/cli/completion
  • GOCACHE=/tmp/go-build GOMODCACHE=/tmp/go-mod go build -o /tmp/uc ./cmd/uncloud
  • HOME=/tmp/uc-nohome /tmp/uc __complete --connect tcp://bad inspect ''

Full go test -p 1 ./... passed for non-e2e packages. test/e2e failed with existing Docker cluster runtime errors about invalid IPv6 addresses.

Comment thread cmd/uncloud/main.go
SSH: config.SSHDestination(dest),
}
}
if initErr != nil {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

might be nice to add a comment that initErr may be set in initCLI and we check for that here, is a bit non-obvious

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

addded a comment explaining this. initCLi can run from Cobras initializer before PersistentPreRunE

Comment thread cmd/uncloud/main.go
return opts
}

func setCLIContext(cmd *cobra.Command, uncli *cli.CLI) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is this changed from

cmd.SetContext(context.WithValue(cmd.Context(), "cli", uncli))

?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

completion callbacks can run on the leaf command context so setting it only on the root command is not enough. so added comment here too

Comment thread cmd/uncloud/main.go Outdated
return nil
}

func globalOptsFromArgs(opts globalOptions, args []string) globalOptions {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this looks odd and fragile... you have to do your own parsing for something that is a package that is supposed to do this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yeah i shortened it now and it reads --connect with pflag for the completion path

@miekg

miekg commented May 26, 2026

Copy link
Copy Markdown
Contributor
% ./uc --connect test@uncloud1.vm.science.ru.nl inspect  <TAB>                                                                               ~miekg/uncloud/cmd/uncloud pr-378
 completions
alertmanager   caddy          debug          grafana-data   ip             ip-php         prometheus     unprune
atomdns        caddy-metrics  grafana        helloworld     ip-data        ntfy           unprom         unredis

while I have moved my config.yaml out of the way. Works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] completion with only --connect is a noop

2 participants