clapp icon indicating copy to clipboard operation
clapp copied to clipboard

Duplicate commands cause crash

Open BluuArc opened this issue 7 years ago • 3 comments

Currently using Clapp with Discord.js when I ran into a strange issue.

  1. Say I have a command foo with a flag that takes in a string, something along the lines of --test <string>,
  2. Some user inputs ~mybotidentifier foo --test somestring1 --test somestring2.
  3. The bot crashes, with the error messages pointing me to the _convertType function in App.js in the dist folder
  4. After messing around with console.log for a bit in App.js, the error seems to be stemming from arg being an array of strings with no switch case to handle it.

BluuArc avatar May 26 '17 03:05 BluuArc

This is apparently a functionality that is not implemented. clapp depends on minimist-string (which I wrote), which also depends on minimist (which somoene else did). minimist puts somestring1 and somestring2 in an array and calls is test, and clappis not ready to handle that scenario.

I see two solutions here:

  1. Handle that scenario, and pass the result back to the command.
  2. Handle that scenario, and whenever happens prompt the user with an error. Allow the types string[], and number[] to be assigned to flags, where this would work (and if the user runs --test somestring1 without the second --test flag, it would return an array of only one element).

I'm a fan of option 2 because clapp aims to provide type safety, and string is not the same as string[]. So if in your command you operate with the string the user provides (something like argv.test.toLowerCase()) you'd get a TypeError.

So what do you think?

MeLlamoPablo avatar May 26 '17 18:05 MeLlamoPablo

I like option 2 as well since it's more in line with how Clapp currently handles input, like you mentioned.

BluuArc avatar May 26 '17 20:05 BluuArc

Sounds good. I'll work on it when I can. Feel free to send a PR if you need this asap though.

MeLlamoPablo avatar May 29 '17 11:05 MeLlamoPablo