Skip to content
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

Jump to bottom

feat: add CompletionFunc type to help with completions #2220

Merged
merged 2 commits into from
Jan 22, 2025
Merged

feat: add CompletionFunc type to help with completions #2220

merged 2 commits into from
Jan 22, 2025

Conversation

ccoVeille
Copy link
Contributor

Also document that NoFilesCompletion and FixedCompletion can be used with RegisterFlagCompletionFunc.

@ccoVeille
feat: add CompletionFunc type to help with completions
72b0553 
Also document that NoFilesCompletion and FixedCompletion can be used with RegisterFlagCompletionFunc.
@CLAassistant
Copy link

CLAassistant commented Jan 19, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@ccoVeille
Copy link
Contributor Author

ccoVeille commented Jan 19, 2025 •
edited
Loading

I worked on this because:

marckhouzam
marckhouzam reviewed Jan 21, 2025
View reviewed changes
Copy link
Collaborator

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

Nice improvement. Thanks!
I would just like to adjust the comments if you don't mind. Please see comments in-line

completions.go Outdated Show resolved Hide resolved
completions.go Outdated Show resolved Hide resolved
completions.go Outdated Show resolved Hide resolved
@ccoVeille @marckhouzam
Apply code review feedback
9a528ef 
Co-Authored-By: Marc Khouzam <[email protected]>
@ccoVeille
Copy link
Contributor Author

@marckhouzam Please review

marckhouzam
marckhouzam approved these changes Jan 22, 2025
View reviewed changes
Copy link
Collaborator

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

Thanks!

@marckhouzam marckhouzam added this to the 1.9.0 milestone Jan 22, 2025
@marckhouzam marckhouzam merged commit 6c3c116 into spf13:main Jan 22, 2025
20 checks passed
@ccoVeille ccoVeille deleted the completions branch January 22, 2025 02:57
@ccoVeille ccoVeille mentioned this pull request Jan 22, 2025
Open
@ccoVeille
Copy link
Contributor Author

For anyone interested, I want to continue the discussion with #2222

@ccoVeille ccoVeille mentioned this pull request Jan 22, 2025
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marckhouzam marckhouzam marckhouzam approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.9.0
Development

Successfully merging this pull request may close these issues.
3 participants
@ccoVeille @CLAassistant @marckhouzam