vorpal icon indicating copy to clipboard operation
vorpal copied to clipboard

hinting similiar commands #151

Open minhchu opened this issue 9 years ago • 6 comments
trafficstars

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.

minhchu avatar May 31 '16 03:05 minhchu

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.

dthree avatar Jun 01 '16 16:06 dthree

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

minhchu avatar Jun 02 '16 09:06 minhchu

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.

AljoschaMeyer avatar Jun 02 '16 10:06 AljoschaMeyer

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();

dthree avatar Jun 02 '16 16:06 dthree

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.

minhchu avatar Jun 03 '16 14:06 minhchu

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

dthree avatar Jun 03 '16 16:06 dthree