Skip to content

Add better completion - #233

Open
gnufied wants to merge 5 commits into
gmeghnag:mainfrom
gnufied:add-better-completion
Open

Add better completion#233
gnufied wants to merge 5 commits into
gmeghnag:mainfrom
gnufied:add-better-completion

Conversation

@gnufied

@gnufied gnufied commented Jan 12, 2026

Copy link
Copy Markdown

So omc does not currently offer completion for positional arguments such as:

omc get <pod> <pod_name>

This PR implements that. A lot of changes related to whitespace noise is because, we didn't seem to be using gofmt standards previously and bunch of files were using four spaces, instead of tab. gofmt uses tabs. Let me know if that is an issue.

Disclaimer: Most of the completion code however was generated by Claude.

@gnufied
gnufied force-pushed the add-better-completion branch from 4cb1266 to d28c3b7 Compare January 12, 2026 20:12
@gmeghnag

Copy link
Copy Markdown
Owner

Thanks for this — positional-argument completion is a genuinely useful addition, and the core mechanism (reading resource/pod/namespace names out of the must-gather) works well. I built the branch and tests pass. Before this can be merged, though, I'd like to ask for a few changes:

1. Fix the describe completion — it advertises resources omc can't describe

cmd/describe/completion.go offers ~33 resource types (service, deployment, replicaset, statefulset, daemonset, job, cronjob, pvc, configmap, secret, ingress, route, hpa, …), but omc describe only registers pod and node (see cmd/describe/describe.go — everything else falls through to Invalid object type). The completion should offer only what's actually implemented (pod/node and their aliases), ideally derived from the registered subcommands rather than a hardcoded list so it can't drift again.

2. Please remove Makefile and COMPLETION.md

  • The 325-line Makefile (release/cross-compile/coverage targets) is unrelated to shell completion and expands the scope of this PR considerably. If it's valuable, it should be its own PR discussed separately.
  • COMPLETION.md contains inaccurate claims — e.g. that describe "supports all Kubernetes and OpenShift resource types" — which isn't true (see point 1). I'd rather not ship documentation that overstates support. Please drop it (or, if you want docs, reduce it to a short, accurate note).

3. Split the gofmt/whitespace reformatting into a separate PR

A large part of the diff is tabs-vs-spaces reformatting across files unrelated to completion (etcd tests rewritten wholesale, license headers, admin, prometheus, etc.). This makes the actual change hard to review and pollutes git blame. A dedicated "run gofmt across the tree" PR would be easy to review and easy to accept.

4. Minor code-consistency cleanups

  • Mixed file-IO APIs: cmd/logs/completion.go and cmd/project.go use the deprecated io/ioutil, while cmd/completion/resources.go uses os. Please standardize on os.
  • Two YAML libraries are used (gopkg.in/yaml.v2 in logs vs sigs.k8s.io/yaml in completion). Please use only sigs.k8s.io/yaml (the official Kubernetes YAML library) and drop the gopkg.in/yaml.v2 usage.
  • The commonShortForms list in cmd/get/completion.go is hand-maintained and has at least one duplicate (deploy/deployment/deployments). If these aliases can be sourced from KindGroupNamespaced/KnownResources instead, that would avoid drift.

Happy to help review once the describe list is corrected and the Makefile/COMPLETION.md/reformatting are split out.

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.

2 participants