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
black hole #3
base: master
Are you sure you want to change the base?
black hole #3
Conversation
function schedule_a_mock_reply(message_text, callback) | ||
{ | ||
message_text = g_scm2host(message_text); | ||
console.log("translating " + message_text.toString() + "..."); |
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.
If message_text
is a scheme string, not need to call toString
after g_scm2host
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.
There is several things that are not correct, but I have to recognize that I am impressed 💯 Did you know Scheme before?
The first error is that the translate
procedure should be exposed Scheme side, g_scm2host
inside main.scm
... but that is not all!
The main problem is that the framework does not support async callbacks or xhr... and those are dealt differently. xhr
should be a procedure that I already created in the past but did not have time to add to this code based and that is slightly complex to come up with (even its use is straightforward, it is like await fetch(...)
without the await keyword`.
Regarding async callbacks, one need to take advantage of the change
procedure defined inside webui-app
.
Lines 573 to 577 in 2efdb68
;; change procedure allows to sneak into the app closure | |
(define (change proc) | |
(let ((new (proc model))) | |
(set! model new) | |
(render))) |
change
should be passed to proc
inside make-controller
:
Lines 563 to 567 in 2efdb68
(define (make-controller proc) | |
(lambda (event) | |
(let ((new (proc model event))) | |
(set! model new) | |
(render)))) |
Anyway, I was under the impression that communication with nlu-convo would be done over a REST API, with some HTTP hook to send messages from the users? What do you think @logicmoo?
.catch(err => { | ||
console.log('translate err'); | ||
console.error(err); | ||
g_scm_call(callback,err.toString()) |
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.
I am not sure what you want to do with g_scm_call
if you want to retrieve the result scheme side that is not the way to go... see the finale review note.
(begin (new-message! model) (vector-set! model 1 "") model) | ||
model))) | ||
(let ((msg-uid (new-message! model))) | ||
(begin |
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.
Inside let
you do not need begin
.
(let ((msg-uid (new-message! model))) | ||
(begin | ||
(vector-set! model 1 "") | ||
(schedule-a-mock-reply msg-uid) |
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.
eventually the schedule-a-mock-reply
will look like (mock-reply msg (then change) (err change))
where then look like:
(define (then change)
(lambda (translation)
(change (lambda (model) (new-message! translation) model)))
Or something along those lines...
) | ||
) | ||
) | ||
) |
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.
Except the non-idiomatic indentation of parentheses I see that the javascript is bound in scheme, that is good but the wrong interface like explained in the review message, the interface should be different.
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.
... should be different but it is not doable at the moment.
alrighty. I wasn't trying to imply exactly how the UI should eventually work, just wanted to understand your stack a bit |
not really a PR, just so we don't forget this