fix: remove sade in favor of commander to actually allow 2 character option aliases

[ACHP]

Following this discussion https://github.com/antoine-coulon/skott/issues/83

I suggest we remove sade in favor of commander because sade actually does not read 2 letters aliases "correctly".

Note: Commander does not officially support 2 letters aliases neither. But from my test it handle it pretty well

if one allow 3 options aliases -a, --aaa, -b, --bbb, and -ab, --abab specifying: -ab will set the abab option to true as expected -a will set aaaoption to true -b will set bbboption to true -ab -a -b will set all options to true -abab will set all options to true as well

I didn't spent much time on testing actually. (tests seems to pass, command seems to work but I didn't try a lot of different use case, linter is failing but it does not seems to be caused by my changes )

[antoine-coulon]

As you mentioned the "lint" is broken and it's not related to your PR so ignore that, I'll fix it on all projects :)

[ACHP]

I just tested and it seems that there is some weird behavior with the parsing of the Commander CLI for commands where strings are expected:

// throws: new Error("Not a number.");
skott -c 4 

It seems that providing the type in the option fixes the issue for instance adding <number>

option(
    "-c, --exitCodeOnCircularDependencies <number>",
    "Specify the exit code to use when circular dependencies are found",
    commanderParseInt,
    1
  )

And now this works. Even worse, some options are not parsed correctly without the information:

skott --ignorePattern="**/*.ts"
// throws -> error: unknown option '--ignorePattern=**/*'

Also it seems that by default Commander will try to coerce everything to a boolean, no matter what the default value is (different behavior of sade which was looking for the default value to infer the expected type at runtime).

For the ignorePattern option without the type, commander correctly parses it using the alias, but the boolean type replaces the value:

skott -ig "**/*"

The option is converted to => { ignorePattern: true } :/

Consequently we definitely need to explicitly define <type> for each option, either <number>, <boolean> or <char>, otherwise we'll have a weird behavior.

Lol I actually had these changes in local but not merged/pushed, sorry about that 😬 fixed in https://github.com/antoine-coulon/skott/pull/88/commits/655f8da02d2ba8f11da86dff3ec22301d0cd4c92

[ACHP]

I also added a changeset in https://github.com/antoine-coulon/skott/pull/88/commits/b5d4fc87ca8a77f46d8c21689ff5a470d9cf7bb3

IMO we should consider it a minor change. It's a fix, but an important one, that might break actual usage ( if someone were using options that were not working until now,these options will suddently work,potentially changing the behavior of the command)

[ACHP]

@antoine-coulon sorry for the delay, I did run pnpm install and run eslint:fix (and also fixed some eslint thing by hand)

[antoine-coulon]

@ACHP all good, it's open source mate, no hurry ;) I'm already grateful for your time!

I'll merge it tomorrow 100%, tonight I'll be short in time