Add Token Counter Displayer in Repl #2775
Conversation
jayjay19630
force-pushed
the
ReplNotificationDisplayer
branch
from
b5944f4 to
32d9e1d
Compare
Pull Request Test Coverage Report for Build 7889640712Details
💛 - Coveralls |
|
Corresponding backend PR: source-academy/backend#1062 |
RichDom2185
left a comment
RichDom2185
left a comment
There was a problem hiding this comment.
Thanks for working on this, looking forward to the new additions 👍 just some comments below:
| type: 'notification' | ||
| }); | ||
| } | ||
| const customNotification = state[workspaceLocation].customNotification; |
There was a problem hiding this comment.
Is this for future plans, or already used?
There was a problem hiding this comment.
This is most likely for future plans. Not yet used.
There was a problem hiding this comment.
Would it make more sense for customNotification to be customNotifications (a string[]) instead then? Since it's reasonable to assume you can have either 0, 1, or more than 1; there's no reason to only have either 0 or 1 notification present.
Anyway, it's not used now, so I'll just approve the PR, but perhaps you can create an issue: either remove it (cause it's unused), or make it support multiple notifications?
RichDom2185
left a comment
RichDom2185
left a comment
There was a problem hiding this comment.
LGTM, thanks for the quick fixes! Please refer to the thread above regarding custom notifications.
Thanks again!
3 participants
Add Token Counter Displayer in Repl
Description:
This pull request creates a new assessment config which enables a token counter. This token counter is a notification which lets the user know how many tokens their program has. This notification can be seen as an output in the Repl once the program is run.
Tasks:
Notes:
Note that modifying custom Repl notifications hasn't been implemented yet (possible future updates).
Co-authored-by: Lau Xiang Yu