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.
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
feat: allow to access output of default completion in custom completion #1878
Conversation
@jacobator I'd rather try to fix the specific bug, rather than growing yargs' API. The problem is we'd then have a new method to support forever. It seems like we should be able to take the disjunction between positionals and options when showing completion, and figure out how to not suggest them? |
@bcoe I agree that the disjunction of positionals/options should be done. I will try to look into that to see if I can assist with it. But the PR here is not only about that. Even when the positionals are excluded from the suggestion list, you would still want to modify the default suggestions. E.g. remove descriptions for all options, reorder options, etc. The way the default completion function access was added here #1855 does not make a lot of sense. You can either use default or fully custom completion for every call. This means you have to implement a full-fledged completion for some cases and would probably not require the default completion function for the others. Having access to default completion suggestions output serves as a great starting point, helping with the custom completion implementation w/o having to do it from the ground-up. |
lib/completion.ts
Outdated
@@ -278,7 +279,7 @@ interface FallbackCompletionFunction { | |||
( | |||
current: string, | |||
argv: Arguments, | |||
defaultCompletion: () => any, | |||
defaultCompletion: (onCompleted?: CompletionCallback) => any, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be tempted to call this completionFilter
.
@@ -193,7 +193,8 @@ export class Completion implements CompletionInstance { | |||
return (this.customCompletionFunction as FallbackCompletionFunction)( | |||
current, | |||
argv, | |||
() => this.defaultCompletion(args, argv, current, done), | |||
(onCompleted = done) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a test:
it('allows custom completion to be combined with default completion, using filter')
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, as a follow up PR, it would be good to add documentation. I'm fine with that being in a separate PR.
I will update the docs tomorrow |
Allow to access output of default completion in custom completion
Problem:
When using default completion both
positional
andoption
arguments are treated the same. This results in completion suggestingpositional
arguments in anoption
form, which is incorrect:Currently, the solution is to write your on competion funtion from the ground up. There is no way to apply custom completion function that would be able to utilize default completion function output. But this would help significantly in the case above.
Solution:
This PR adds ability to pass
onCompleted
callback todefaultCompletion
function to replace thedone
callback, allowing us to do this:The example above is naive and just filtering based on the
pos
text, but illustrates the point.Ideally, the default completion should ignore
positional
arguments out of the box, but this is a simple solution to fix completion right now.