chatskills icon indicating copy to clipboard operation
chatskills copied to clipboard

Throw error on nonalphanumeric namespace

Open goodmike opened this issue 8 years ago • 0 comments

[Closes #2]

Here's the PR to prevent users from accidentally choosing a namespace for a skill that will cause errors later.

I have 2 separate commits. The first sets up a minimal tape test environment and a single unit test for the error. If you're like me, you're more comfortable squashing a bug when there's a test to demonstrate it's fixed. However, if you don't want the dev dependency on tape, which also pulls a number of other packages in (even though it's supposed to be the lightweight testing alternative), or you prefer a different library like mocha or jasmine, feel free to ask me to discard or change this commit.

The second commit has the actual edit to chatskills.js. I decided to keep it as simple as I could, so I have a RegExp test for non-alphanumerics. I did not extract the namespace part of the RegExp expression in the session method because I think it would be difficult to use the partial pattern both in the add method and the session method. I don't think RegExps do not compose easily. Again, if you'd prefer I try to DRY things out, let me know.

goodmike avatar Dec 30 '16 23:12 goodmike