Skip to content
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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
### Next
### 0.2.2
Copy link
Collaborator

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.

Copy link
Author

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.


* Your contribution here.
* Added ability to make slots optional by adding "+" before and after slot name (example: {+SLOTNAME+|}) - [@dominicankev] (https://github.com/dominicankev).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this back for the next person please.


### 0.2.1

Expand Down
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,19 @@ You may want to work with [Custom Slot Types](https://developer.amazon.com/appsa
"your least favorite snack is {Fruit}"
```

You can use a special syntax to leave a curly-braced slot name unparsed and make it optional. For example, if you have defined in your skill a `ROOM_NAME` with the values `Bedroom`, `Office` and `Living Room` for the slot `Room_Name`, you can keep `Room_Name` a curly-braced literal and optional as follows

```javascript
"change channel {to|} {-|ChannelNumber} {in +ROOM_NAME+|+ROOM_NAME+|}"
=>
"change channel to {ChannelNumber} in {ROOM_NAME}"
"change channel {ChannelNumber} in {ROOM_NAME}"
"change channel to {ChannelNumber} {ROOM_NAME}"
"change channel {ChannelNumber} {ROOM_NAME}"
"change channel to {ChannelNumber} "
"change channel {ChannelNumber} "
```

### Contributing

See [CONTRIBUTING](CONTRIBUTING.md)
Expand Down
2 changes: 2 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ function generateUtterances(str, slots, dictionary, exhaustiveUtterances) {
// Convert all {-|Name} to {Name} to accomodate slot literals
for (var idx in utterances) {
utterances[idx] = utterances[idx].replace(/\{\-\|/g, "{");
utterances[idx] = utterances[idx].replace(/\+(.*?)/, "{");
utterances[idx] = utterances[idx].replace(/\+$/, "}");
}

return utterances;
Expand Down
16 changes: 10 additions & 6 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,19 @@ test('exhaustive vs non-exhaustive expansion', function (t) {

test('raw curly braces for custom slot types', function (t) {

Choose a reason for hiding this comment

The 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} ",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway to get rid of the hanging at the end of the line?

Copy link
Author

@dominicankev dominicankev Jan 18, 2017

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No doubt. Here's what I would do:

  1. Update the code to trim the whitespace (or not add it in the first place)
  2. Add a new test for the optional terms which doesn't include the trailing white space
  3. Run all the tests with your new code. They should all pass
  4. Report back if you have issues

Copy link
Author

Choose a reason for hiding this comment

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