Skip to content

Refac shell completion to a better command structure#12590

Merged
vitorsavian merged 4 commits into
k3s-io:masterfrom
vitorsavian:refac-completion
Jul 10, 2025
Merged

Refac shell completion to a better command structure#12590
vitorsavian merged 4 commits into
k3s-io:masterfrom
vitorsavian:refac-completion

Conversation

@vitorsavian

@vitorsavian vitorsavian commented Jul 7, 2025

Copy link
Copy Markdown
Member

Proposed Changes

  • better structure for shell completion command, this new structure for this command is to avoid unnecessary problems specially related to verify input from the user
k3s completion --help
k3s completion bash --help
k3s completion zsh --help

Types of Changes

  • Fix

Verification

  • you can see that the two of them are separated commands by
k3s completion --help
  • the verification to test if it's working as expected is
k3s completion bash -i
  • and then
source ~/.bashrc

Testing

Linked Issues

User-Facing Change

k3s completion shell command will now be separate to specific subcommands for bash and zsh

Further Comments

Signed-off-by: Vitor Savian <vitor.savian@suse.com>
Signed-off-by: Vitor Savian <vitor.savian@suse.com>
Signed-off-by: Vitor Savian <vitor.savian@suse.com>
Signed-off-by: Vitor Savian <vitor.savian@suse.com>
@vitorsavian
vitorsavian requested a review from a team as a code owner July 7, 2025 20:54
@codecov

codecov Bot commented Jul 7, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 0% with 53 lines in your changes missing coverage. Please review.

Project coverage is 19.96%. Comparing base (e6e3019) to head (05c025b).
Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
pkg/cli/completion/completion.go 0.00% 28 Missing ⚠️
pkg/cli/cmds/completion.go 0.00% 25 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (e6e3019) and HEAD (05c025b). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (e6e3019) HEAD (05c025b)
e2etests 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #12590       +/-   ##
===========================================
- Coverage   40.44%   19.96%   -20.48%     
===========================================
  Files         186      184        -2     
  Lines       19071    19254      +183     
===========================================
- Hits         7714     3845     -3869     
- Misses      10170    14975     +4805     
+ Partials     1187      434      -753     
Flag Coverage Δ
e2etests ?
unittests 19.96% <0.00%> (+0.64%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dereknola

Copy link
Copy Markdown
Member

I don't see how this resolved the RKE2 issue, is that expected future work for RKE2?

@dereknola

dereknola commented Jul 7, 2025

Copy link
Copy Markdown
Member

A much simpler fix would just be len(ARGS) < 1 check, which would solve the k3s completion bash --foo --bar from failing.

I'm not quite clear on what specifically the error this fixes looks like.

@vitorsavian

Copy link
Copy Markdown
Member Author

I don't see how this resolved the RKE2 issue, is that expected future work for RKE2?

here it's mostly a refac, my plan is to also change the behavior on rke2 to match k3s, but originally the problem was specific to rke2 when the user set the flags after the shell parameter

rke2 completion shell --kubectl --crictl

@vitorsavian
vitorsavian merged commit 66102c5 into k3s-io:master Jul 10, 2025
73 of 74 checks passed
@vitorsavian
vitorsavian deleted the refac-completion branch August 6, 2025 17:15
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.

4 participants