dthree / vorpal

Node's framework for interactive CLIs
http://vorpal.js.org
MIT License
5.64k stars 280 forks source link

Fix command autocomplete behavior #83

Closed jeff-1amstudios closed 8 years ago

jeff-1amstudios commented 8 years ago

Changes command autocomplete behavior in the following cases:

Simplified longest match algorithm, added test cases (test/autocomplete.js)

dthree commented 8 years ago

Oh no!!! Jeff, you've done so much work on this!

When I wrote:

I'm mid refactoring the autocompletion as gone over in this thread.

I mean I've literally torn the existing autocompletion code to pieces (all of it, not just command autocompletion), it's now in a separate file and is at 250 lines of implementation.

Now I feel terrible as you've put so much effort into this!


How about this: hold off for a little, I'll be finishing my work today. Don't worry - it basically does what you intended and fixes all those messed up nuances, in addition to properly implementing completion for options, as you also wanted!

Once I'm at a flat point, we can go over the changes and fine-tune the PR. Love all the tests, I can definitely use those as well!

jeff-1amstudios commented 8 years ago

Cool! Yeah when I saw that you were hacking on it also I was hoping we weren't editing the same thing. Options autocompletion would be awesome, I'd like to pass an array or a function that returns a Promise of an array and have Vorpal do the printing of the possible matches

dthree commented 8 years ago

Okay, so in a complicated example, like this:

vorpal.command('foo').
  .option('-f, --food', 'What you want to eat', function (str, cb) {
    getFoods(str, function(foods) {
      cb(foods);
    });
  });

or this:

vorpal.command('foo').
  .option('-f, --food', 'What you want to eat', ['pizza', 'sushi', 'spaghetti']);

In the command autocompletion, it's going to be like this:

vorpal.command('eat').
  .autocomplete(['pizza', 'sushi', 'spaghetti']);

or this:

vorpal.command('foo').
  .autocomplete({
    data: iAmAFunctionThatReturnsAnArray
   });

The reason I include the data object in the main one, is that I still want to reserve the option of a completely manual autocomplete function, which would take effect when you pass a function, i.e.

vorpal.command('foo').
  .autocomplete(customAutocompleteFunctionThatIsComplicatedAndAbnormal);

However, If we do the above, there won't be a chance for a custom autocompletion method for options. I don't know that we really need one...


Input?

dthree commented 8 years ago

@jeff-1amstudios @fiatjaf @ristomatti @scotthovestadt

Getting really close to finishing, would like confirmation / agreement / disagreement on the above behavior before publishing. Thanks!


BTW: Decided on command.autocomplete.

scotthovestadt commented 8 years ago

I think that autocomplete should take either any Iterable (typically an array) or a function. The function should be allowed to use both a callback or a promise.

Examples:

vorpal.command('foo').
  .option('-f, --food', 'What you want to eat', ['pizza', 'sushi', 'spaghetti']);
vorpal.command('foo').
  .option('-f, --food', 'What you want to eat', function(str, callback) {
    return new Promise((resolve, reject) => {
      resolve(['pizza', 'sushi', 'spaghetti']);
    });
  });
vorpal.command('foo').
  .autocomplete(function(str, callback) {
    return new Promise((resolve, reject) => {
      resolve(['pizza', 'sushi', 'spaghetti']);
    });
var myIterable = {}
myIterable[Symbol.iterator] = function* () {
    yield 'pizza';
    yield 'sushi';
    yield 'spaghetti';
};
vorpal.command('foo').
  .option('-f, --food', 'What you want to eat', myIterable);
dthree commented 8 years ago

Thanks. Okay so basically as above, with the inclusion of Promises. On Iterables, I unfortunately haven't converted this to ES6 with Babel yet, and this would drop support of older Node versions. I would like to eventually ES6-ify this project, and at that point I'll implement it.


BTW option listings also autocomplete now :fire: :

$ foo -[tab]
-A --bar --bizzle

I have literally the exact functionality of a Linux CLI working now :smiley:

scotthovestadt commented 8 years ago

RE: ES6, happy to help Babel-ify it. I've done it a bunch of times.

For now, just attempt to iterate any value passed. That should cover the use-case.

dthree commented 8 years ago

Thanks.


That'd be really cool if you would be willing to help. However, it'd probably have to be definite and during a condensed period of time, as it will modify all of the code and any other changes during that time would be pretty hard to merge in.

scotthovestadt commented 8 years ago

I think it's best to do the Babel thing in phases. First, just add a build step, and then you can change code to be more ES6-friendly as you touch it (and make sure you have tests for that particular code first...)

dthree commented 8 years ago

I can agree with that.

jeff-1amstudios commented 8 years ago

Looks great, but Promises, please!

scotthovestadt commented 8 years ago

Yeah. I rely very heavily on Promises. I don't use callbacks at all anymore, and when I have to, I wrap them in Promises...

dthree commented 8 years ago

Haha, got it! Promises are on the list!

dthree commented 8 years ago

Okay, v1.7.0 has the new autocompletion.

Would love some code review:

https://github.com/dthree/vorpal/commit/e7e2bc96273744a294e9ee76ea7350745fde75c3


Deprecation

Due to the nature of the updates, I did an interesting means of deprecation. If you have a single command in your Vorpal instance that uses command.autocompletion, the old autocompletion will take effect. If not, it will default to the new command.autocomplete methods.


Wiki

Here's the updated Wiki pages:

jeff-1amstudios commented 8 years ago

Just want to say.. this is great! Nice work :+1:

nnoto commented 8 years ago

Just curious, is there a way to do nested autocompletion? For example the command "list foo bar," where "list foo" and "list foo bar" are complete commands that can be executed, and only "list" is offered as an initial autocomplete? Your system makes it easy to do one level of autocompletion, offering "foo" in the autocomplete array, but what about for a third subcommand? Thanks!

nnoto commented 8 years ago

Alternatively, can you provide similar functionality to autocomplete that you provide to "help" for multi-word commands? For instance: "list foo" and "list bar" commands show both "list foo" and "list bar" as completions when starting the app and pressing tab, since they are the base commands. However, since they're multi-word commands that share the word "list", it might make sense to offer "list" as the only initial completion, and append the additional possibilities once a tab has been pressed?

dthree commented 8 years ago

@nnoto thanks for your input! On point one, I was thinking about that! While this shouldn't be that hard to implement, the main trouble I had was figuring out how to make this syntactically simple for the person implementing it.

@jeff-1amstudios do you have any suggested API for multiple autocompletes? Here's a terrible exmaple:

put-food-in-file [food] [file]

The first autocompletion would be a list of foods, and the second would let you browse the file system via tab, such as with the ls command. So how do we say to use these two different autocompletes, and not be super complex about it?

Oh I have an idea!

Perhaps it's just each parameter passed in .autocomplete, i.e.:

var fsAutocomplete = require('vorpal-autocomplete-fs');

vorpal
  .command('put-food-in-file')
  .autocomplete(['steak', 'pork'], fsAutocomplete);

Does that sound okay? Options will not be supported.


Got it on the second part. That's going to be kind of tricky, but I'll try to do it!

dthree commented 8 years ago

@nnoto got your first request working! Available in v1.7.3.

nnoto commented 8 years ago

@dthree Awesome thanks! Excited to try it out shortly.

dthree commented 8 years ago

Okay great!


@jeff-1amstudios @scotthovestadt first Vorpal autocomplete extension, using the new syntax :fire::

https://github.com/vorpaljs/vorpal-autocomplete-fs

nnoto commented 8 years ago

Just want to clarify functionality here:

var vorpal = require('vorpal')();

vorpal
  .command("list", 'desc')
  .autocomplete(['instances', 'volumes'], ['running', 'stopped', 'shutting-down']);
vorpal
  .delimiter('>')
  .show();

The command "list" and built-in commands are initially tab-completed. Once "list" is completed, "instances" and "volumes" are offered as the next completions when tab is pressed. Finally, when either "instances" or "volumes" is completed, the completions in the second array are provided. This setup isn't currently working for me, so I wanted to make sure it's what was intended with 1.7.3. By the way, I noticed once the first array's completed, another tab press will display the first array as a completion again. Example: prompt> list instan prompt> list instances prompt> list instances instances volumes

Thanks for all the great work by the way!

dthree commented 8 years ago

Oops - I meant I did the second request, not the first! Sorry about that. I got this working:

"list foo" and "list bar" commands show both "list foo" and "list bar" as completions when starting the app and pressing tab, since they are the base commands. However, since they're multi-word commands that share the word "list", it might make sense to offer "list" as the only initial completion, and append the additional possibilities once a tab has been pressed?

On the other thing, I just want to go over it a bit to make sure this syntax will work out - but we're both on the same page on the requirements!

nnoto commented 8 years ago

Oh gotcha! That's great as well. Only needed one of those requests to get the functionality I'm looking for. Looks like it's almost working. Here's the most simple case I could generate:

var vorpal = require('vorpal')();
vorpal
  .command("list instances", "desc");
vorpal
  .command("list volumes", "desc");
vorpal
  .delimiter('>')
  .show();

It works perfectly with the exception of the initial registered command completion generated when two keypresses are made without any text to complete. prompt> exit help instances volumes quit

but if I start typing "list", it begins working the way intended. prompt> lis prompt> list prompt> list instances volumes

Thanks again!

dthree commented 8 years ago

Huh. Okay, will fix.

nnoto commented 8 years ago

You're probably already well aware, but I believe the issue lies in the filterData function, specifically the if(parts.length >1) block. I'm guessing var parts = parts.slice(wordParts.length); was intended to be var parts = parts.slice(0, wordParts.length); but unfortunately that results in more issues. You're probably already looking into it, but I wanted to mention it just in case. and I haven't gotten any time to look into fixing it myself beyond a brief investigation

dthree commented 8 years ago

Okay thanks. It's some really touchy code, only because the multi-spaced-word commands deviate from regular option parsing, so it takes a lot of custom code to ensure there's no conflicts. I'll take a stab at it as soon as I can!

dthree commented 8 years ago

Okay Jeff, what do we want to do with this PR? I think we should work together on the tests, so do you want me to keep this open?

dthree commented 8 years ago

@jeff-1amstudios hey. Would like to close this up when you have a moment. Thanks.

jeff-1amstudios commented 8 years ago

Sure, I'll just close it as its all out of date with all the work you've done with autocomplete!

dthree commented 8 years ago

KK. Will probably still steal your tests! Thanks for all your help and advice!

jeff-1amstudios commented 8 years ago

No problem! I've been meaning to email you - I gave a talk at my company's meetup a month ago where I live-coded a CLI app using Vorpal showing off autocomplete, automatic help text generation etc. And the coolest thing was I was coding against a REST API I built for the old game Doom. So I created a spawn command in the CLI which did a POST to http://localhost:2001/api/world/objects and generated a new monster inside the running Doom game. It was pretty epic, even though I crashed Doom at one point! I'm going to write a post about it when I get a chance, will flick you a link :)

dthree commented 8 years ago

Haha that's totally awesome!!!! :sunglasses:

I'd love to see the link!

jeff-1amstudios commented 8 years ago

Really off topic now - heres a screenshot showing an early buildscreen shot 2016-01-28 at 10 51 16 am

dthree commented 8 years ago

Haha that's amazing!

dthree commented 8 years ago

BTW - while we're at it on destroying any sense of pertinence in this thread - I'm about to release my next big Vorpal project - you like?

jeff-1amstudios commented 8 years ago

looks very cool! I barely ever have to boot into windows these days (except for occasional bursts of Skyrim...), but love the idea!

dthree commented 8 years ago

Thanks!

I barely ever have to boot into windows these days

Must be nice

MaestroJurko commented 6 years ago

I cannot get autocomplete working with v1.12.0

vorpal
  .command('auth [action]')
  .autocomplete(['verify', 'generate'])

When pressing tab no autocomplete option is shown, the cursor just jumps for a tab.