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.

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

Option completion doesn't work for commands #180

Closed
tschaub opened this issue Jun 18, 2015 · 7 comments
Closed

Option completion doesn't work for commands #180

tschaub opened this issue Jun 18, 2015 · 7 comments

Comments

@tschaub
Copy link
Contributor

tschaub commented Jun 18, 2015

Given this test.js script:

#!/usr/bin/env node
var argv = require('./index')
  .completion('completion')
  .command('bar', 'bar command', function(yargs) {
    var subArgv = yargs
      .option('foo', {
        describe: 'foo option'
      })
      .argv;

    console.log('subArgv: %j', subArgv);
  })
  .argv;

console.log('argv: %j', argv);

Enabling completion:

$ eval "$( ./test.js completion )"

And typing ./test.js bar --f doesn't result in the foo option being completed.

@bcoe
Copy link
Member

bcoe commented Jun 18, 2015

The parsing inside the command's handler is facilitated by a fresh instance of yargs. It would take a bit of work for to make completion work contextually, based on the command entered prior to the tab completion being hit.

One could make the argument that this might be specific enough that we should just endorse that you implement it via a custom completion handler (the optional closure you can provide to completion).

To argue the other side though, it would be a slick feature to have command-specific completion -- is this something you'd be interested in taking a stab at?

@tschaub
Copy link
Contributor Author

tschaub commented Jun 18, 2015

@bcoe yeah, I'd be interested in tackling it. I may not have time for it until early next week. So if anybody else is game, please have at it. I'll leave word here when I start on it in earnest.

@tschaub
Copy link
Contributor Author

tschaub commented Jun 20, 2015

Had a bit of time during a layover to look into this. It may end up being pretty straightforward: master...tschaub:command-completion

This only works for commands with a handler that calls completion on the yargs object that it is passed. For example:

#!/usr/bin/env node
var argv = require('./index')
  .help('help')
  .completion()
  .command('foo', 'foo command', function(yargs) {
    var sub = yargs
      .options({
        bar: {
          describe: 'bar option'
        }
      })
      .completion()
      .help('help')
      .argv;

    process.exit(0);
  })
  .command('bam', 'bam command', function(yargs) {
    var sub = yargs
      .options({
        baz: {
          describe: 'baz option'
        }
      })
      .completion()
      .help('help')
      .argv;

    process.exit(0);
  })
  .argv

Needs testing and potentially more work. I'll try to pick it up later on.

@bcoe
Copy link
Member

bcoe commented Jun 20, 2015

cool \o/ I was worried that would be much more of a pain.

I'm also flying today, and am going to see about squashing a few more bugs on yargs.

Question, how do you feel about the default completion handler also completing file names in the current working directory? I feel like this is something you usually want, but lose when you enable yargs' completion. Should we default to supporting this, or should the onus be on the developer to add this if they need it?

@tschaub
Copy link
Contributor Author

tschaub commented Jun 22, 2015

@bcoe - I've opened #185 for command option completion (as noted there, this only works for commands one level deep). This is working well for the use cases I've tried. I'm sure it could be enhanced if there are more complex use cases.

how do you feel about the default completion handler also completing file names in the current working directory

That could be nice. One possible implementation in #186. Could also do fs.readdir(path.dirname(current), callback) and spit out joined paths, but I think it might be more straightforward to delegate to compgen.

@bcoe
Copy link
Member

bcoe commented Jun 24, 2015

@tschaub if you run:

npm install yargs@next, your changes are in the release-candidate:

https://github.com/bcoe/yargs/blob/master/CHANGELOG.md#v3130-20150624-0412-0000

If everything runs great, we can release the candidate tomorrow.

@bcoe bcoe closed this as completed Jun 24, 2015
@prmichaelsen
Copy link

nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants