-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added code to make slots optional #24
base: master
Are you sure you want to change the base?
Changes from 2 commits
fbe64eb
ed7df41
fbd2d3c
90c8628
97357ed
11bb8c4
a6bc3e9
f56e7ed
bfe4a30
1cb73ee
6adb42d
b97559e
dbc7bd5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
### Next | ||
### 0.2.2 | ||
|
||
* Your contribution here. | ||
* Added ability to make slots optional by adding "+" before and after slot name (example: {+SLOTNAME+|}) - [@dominicankev] (https://github.com/dominicankev). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Put this back for the next person please. |
||
|
||
### 0.2.1 | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -111,15 +111,19 @@ test('exhaustive vs non-exhaustive expansion', function (t) { | |
|
||
test('raw curly braces for custom slot types', function (t) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion you should add a new test instead of repurposing an existing one. |
||
var dictionary = {}; | ||
var slots = {"Artist": "CUSTOM_TYPE"}; | ||
var template = "{my|your} {favorite|least favorite} fruit is {-|Fruit}"; | ||
var slots = {"Artist": "CUSTOM_TYPE","ROOM_NAME": "AMAZON.Room"}; | ||
var template = "{my|your} {favorite|least favorite} fruit is {-|Fruit} {in +ROOM_NAME+|}"; | ||
|
||
var result = utterances(template, slots, dictionary); | ||
t.deepEqual(result, [ | ||
"my favorite fruit is {Fruit}", | ||
"your favorite fruit is {Fruit}", | ||
"my least favorite fruit is {Fruit}", | ||
"your least favorite fruit is {Fruit}" | ||
"my favorite fruit is {Fruit} in {ROOM_NAME}", | ||
"your favorite fruit is {Fruit} in {ROOM_NAME}", | ||
"my least favorite fruit is {Fruit} in {ROOM_NAME}", | ||
"your least favorite fruit is {Fruit} in {ROOM_NAME}", | ||
"my favorite fruit is {Fruit} ", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Anyway to get rid of the hanging There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @harrisonhjones If I trim the trailing space it seems to break the "optional terms" test which also has trailing spaces in the result. I can add a trim to the returned utterances ( ie: return utterances.trim(); ) but not sure if that would be desired or not. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No doubt. Here's what I would do:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @harrisonhjones Cool. Thanks. Was thinking of doing that but wasn't sure if the trailing space was desired or not. Assumed it wasn't. It's more of a result of the utterance containing the space and the optional value not being there. Will give it a shot. Thanks again. |
||
"your favorite fruit is {Fruit} ", | ||
"my least favorite fruit is {Fruit} ", | ||
"your least favorite fruit is {Fruit} " | ||
]); | ||
t.end(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't add a version here, the maintainer will do that when they do a release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dblock Thanks. Wasn't sure on that piece. Will update.