feat: allow to access output of default completion in custom completion

[jacobator]

Allow to access output of default completion in custom completion

Problem:

When using default completion both positional and option arguments are treated the same. This results in completion suggesting positional arguments in an option form, which is incorrect:

yargs
  .command(
    'exec <pos-a> <pos-b>',
    'Execute command.',
    (yargs) => {
      return yargs
        .positional('pos-a', { type: 'string', desc: 'Pos A' })
        .positional('pos-b', { type: 'string', desc: 'Pos B' })
        .option({
          'opt-a': { type: 'boolean', alias: 'oa', desc: 'Option A' },
          'opt-b': { type: 'string', requiresArg: true, desc: 'Option B' },
        });
    },
  )
  .completion('completion', 'Completion.').argv;

$ ./index.js exec --
--help     -- Show help
--opt-a    -- Option A
--opt-b    -- Option B
--pos-a    -- Pos A
--pos-b    -- Pos B
--version  -- Show version number

$ ./index.js exec --pos-a --
--help     -- Show help
--opt-a    -- Option A
--opt-b    -- Option B
--pos-b    -- Pos B
--version  -- Show version number

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 to defaultCompletion function to replace the done callback, allowing us to do this:

yargs
  .command(
    'exec <pos-a> <pos-b>',
    'Execute command.',
    (yargs) => {
      return yargs
        .positional('pos-a', { type: 'string', desc: 'Pos A' })
        .positional('pos-b', { type: 'string', desc: 'Pos B' })
        .option({
          'opt-a': { type: 'boolean', alias: 'oa', desc: 'Option A' },
          'opt-b': { type: 'string', requiresArg: true, desc: 'Option B' },
        });
    },
  )
  .completion(
    'completion',
    'Completion.',
    (current, argv, defaultCompletion, done) => {
      // Execute `defaultCompletion` to get default `completions`
      defaultCompletion((_err, completions) => {
        const filteredCompletions = completions.filter(
          (c) => !c.includes('pos'),
        );
        // Execute `done` with `filteredCompletions`
        done(filteredCompletions);
      });
    },
  ).argv;

The example above is naive and just filtering based on the pos text, but illustrates the point.

$ ./index.js exec --
--help     -- Show help
--opt-a    -- Option A
--opt-b    -- Option B
--version  -- Show version number

Ideally, the default completion should ignore positional arguments out of the box, but this is a simple solution to fix completion right now.

[bcoe]

Ideally, the default completion should ignore positional arguments out of the box, but this is a simple solution to fix completion right now.

@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?

[jacobator]

@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.

[bcoe]

I would be tempted to call this completionFilter.

[bcoe]

We should add a test:

it('allows custom completion to be combined with default completion, using filter').

[bcoe]

This LGTM, as a follow up PR, it would be good to add documentation. I'm fine with that being in a separate PR.

[jacobator]

I will update the docs tomorrow