dthree / vorpal

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

hinting similiar commands #151 #152

Open minhchu opened 8 years ago

minhchu commented 8 years ago

Using leven package to calculate the difference between two strings. And displays similiar commands which have leven point smaller than 4. @dthree please check if I'm doing wrong.

dthree commented 8 years ago

Looks really good!

Could you add in some tests? I would be interested to see the relative results based on different sets of registered commands.

4 seems like a really high edit distance. For example, if you have the commands ls, bb and ck enabled, and entered a, it would probably suggest all of the above.

minhchu commented 8 years ago

I've just run gulp build and added a test suite https://gist.github.com/minhchu/4438f898a8a7bc5fcaa5c72218fca2c6 But there are some problems. When I run only that test suite mocha --grep "hinting command", it passes. But when I run the whole testmocha`, it throws some unexpected errors:

(node) warning: possible EventEmitter memory leak detected. 11 vorpal_ui_keypres
s listeners added. Use emitter.setMaxListeners() to increase limit.

Do you have any idea ?


If you set maximum distance to 4, it still works for your example because leven distance between a and ls is 2 < 4 ...

AljoschaMeyer commented 8 years ago

Node gives that warning when more than 10 (?) listeners are attached to the same emitter. Try require('events').EventEmitter.defaultMaxListeners = Infinity in the test file to fix this. Documentation

The cleaner solution would be to set the maximum for the particular event emitter objects. I was just lazy and copy-pasted that snippet from here.

Vorpal should probably do this itself, the threshold doesn't make sense for vorpal anyways.

dthree commented 8 years ago

Haven't looked far into it, but are you sure we aren't ignoring some problem?

Is this because the test instantiates at least 10 instances of Vorpal?

While developing it I only got this error when I messed something up on the prompt, and had to fix it.


I'm willing to believe the problem is in the repeated use of:

mute();
var fixture = '\n  fo is an invalid command. Maybe you mean:\n\n    foo\n    fooz\n    bar\n';
help.execSync('fo');
unmute();
minhchu commented 8 years ago

Let me check again. I think there is a problem with this approach. For example:
I have a list of commands:

foo:command
foo:anothercommand
foo:awesomecommand

When I type foo, there will be no suggestions.

dthree commented 8 years ago

Yeah, that's all considered one word, and those have a huge edit distance from foo.