feat: send custom event with code eval state payload #296

KBHan · 2017-12-03T21:56:22Z

dviramontes · 2017-12-04T00:24:58Z

Note: this is still a bit of a WIP / need to find a better way to get a hold of the klipse container selector

viebel · 2017-12-06T05:27:45Z

src/klipse/klipse_editors.cljs

@@ -52,7 +52,8 @@
          (swap! state update-in [:eval-counter] inc)
          (let [evaluation-chan (eval-fn (str preamble src-code) @state)
                first-result (<! evaluation-chan)]
-            (setter first-result)
+              (setter first-result)


Here, you can handle different cases for first-result:

if it is a string, behave as before

it it is an array, it will be either: [:ok result] or [:err result]

In python.cljs, returns either [:ok result] or [:err result].

Dispatch the event from here and not from python.cljs

Hi @viebel, looks like we are only seeing case # 1:

if it is a string, behave as before

and never case # 2
do you have any idea why ? We are evaluating python code.

@dviramontes you have to modify the code in python.cljs so that it returns

[:ok result] - in case of success

[:err error] - in case of error

In order not to break other languages, you have to handle also cases where a string was returned.
Does it make sense?

@viebel yes it does, i'm going to give it a shot and let you know how it goes. Thanks!

dviramontes · 2017-12-30T00:38:22Z

@viebel I updated the PR. Lmk if this is closer to what you had proposed. Thanks!

dviramontes · 2017-12-30T00:50:05Z

Also closes #292

viebel · 2018-01-07T14:49:11Z

src/klipse/lang/python.cljs

             (fn [err]
-               (put! c (str "error: " err)))))
+               (put! c (str "error: " err))


In case of an error, you sould put only [:err err] on the channel

viebel · 2018-01-07T14:50:21Z

src/klipse/klipse_editors.cljs

-                  (setter results)
+                  (if (string? result)
+                    (setter results)
+                    (when-let [klipse-dom-node (js/document.querySelector ".klipse-container")]


In case result is not a string you should call (setter (second results))

would you recommend I do both dispatch custom event and call (setter (second results)) here ?

@dviramontes yes I recommend I do both dispatch custom event and call (setter (second results)) here

viebel · 2018-01-07T14:50:55Z

src/klipse/klipse_editors.cljs

+                      (let [event-payload (clj->js {:detail {:result result
+                                                             :result-element (:result-element @state)}})]
+                        (!> klipse-dom-node.dispatchEvent
+                            (js/CustomEvent. "klipse-snippet-evaled" event-payload)))))


You should add in the detail of the event whether there was an error or not

dviramontes · 2018-01-22T01:22:49Z

@viebel made some changes, lmk what you think / thanks!

viebel · 2018-01-28T06:18:02Z

src/klipse/klipse_editors.cljs

@@ -142,14 +155,9 @@
 (defmethod create-editor :code-mirror [_ {:keys [snippet-num element source-code eval-fn default-txt idle-msec editor-in-mode editor-out-mode indent? codemirror-options-in codemirror-options-out loop-msec preamble no-result] :as editor-args}]
  (let [[in-editor-options out-editor-options] (editor-options editor-in-mode editor-out-mode codemirror-options-in codemirror-options-out)
        container  (create-div-after element (klipse-container-attrs snippet-num))
-        _ (create-div-after container {:class "klipse-separator"})


bring back the line that creates the klipse-separator

viebel · 2018-01-28T06:18:45Z

src/klipse/klipse_editors.cljs

-                                                      :klass "klipse-result")) ; must be called before `element` is replaced
-
-        in-editor (replace-element-by-editor element source-code in-editor-options :indent? indent? :klass "klipse-snippet")
+        result-element (when-not no-result (create-editor-after-element element default-txt out-editor-options :indent? false :remove-ending-comments? false)) ; must be called before `element` is replaced


break this line in several lines as it was before

fix: send eval error msg when :err

dviramontes · 2018-02-08T04:42:00Z

@viebel updated PR / sorry I must have rebased it against my fork therefore the changes to klipse editor. Should be good now. Thanks!

viebel · 2018-02-09T12:28:22Z

src/klipse/klipse_editors.cljs

+                  (if (string? result)
+                    (setter results)
+                    (let [hasError (and (= (first result) :err))]
+                      (when hasError (setter (second result)))      ;; only report the :err messages for now


we need to call setter even when there is no error.
The reason is that when the code evaluation succeeds, we pass [:ok res]

Otherwise everything looks fine

hi @viebel the problem with setting (setter (second result)) here when there is no error is that the receiver throws

I think is is because the message passed in from
klipse/lang/python.cljs:33

(put! c [:ok mod])

puts something on that channel that cannot be stringified.

Let me know if that makes sense.

you have to call str before putting on the channel.

(put! c [:ok (str mod)])

Makes sense but let me show you what im running into now, it might make more sense to you..

^^That's with (put! c [:ok (str mod)]). So its not longer throwing an error but we no longer get the output.

KBHan force-pushed the feat-eval-dispatch-custom-event branch from 10af53f to 3b0fa4d Compare December 3, 2017 22:35

viebel requested changes Dec 6, 2017

View reviewed changes

dviramontes force-pushed the feat-eval-dispatch-custom-event branch from 3371965 to bb071a5 Compare December 8, 2017 03:11

dviramontes force-pushed the feat-eval-dispatch-custom-event branch from 3c0aee6 to 8e62a6a Compare December 29, 2017 09:20

viebel reviewed Jan 7, 2018

View reviewed changes

viebel reviewed Jan 28, 2018

View reviewed changes

dviramontes added 6 commits February 7, 2018 17:59

fix: dispatch from python.cljs

0274f3d

fix: better key name, pass result-element with payload

ab39787

refactor: indicate whether err has occurred on event payload

edfd5ce

chore: move comment

2a7569f

fix: apply setter

8c7e57e

revert: klipse separator removal,

5dd7971

fix: send eval error msg when :err

dviramontes force-pushed the feat-eval-dispatch-custom-event branch from d95b30d to 5dd7971 Compare February 8, 2018 04:40

rm: debugging guard

9532bf2

viebel reviewed Feb 9, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: send custom event with code eval state payload #296

feat: send custom event with code eval state payload #296

KBHan commented Dec 3, 2017

dviramontes commented Dec 4, 2017

viebel Dec 6, 2017

dviramontes Dec 8, 2017

viebel Dec 13, 2017 •

edited

Loading

dviramontes Dec 14, 2017

dviramontes commented Dec 30, 2017

dviramontes commented Dec 30, 2017 •

edited

Loading

viebel Jan 7, 2018

viebel Jan 7, 2018

dviramontes Jan 22, 2018 •

edited

Loading

viebel Jan 23, 2018

viebel Jan 7, 2018

dviramontes commented Jan 22, 2018

viebel Jan 28, 2018 •

edited

Loading

viebel Jan 28, 2018

dviramontes commented Feb 8, 2018 •

edited

Loading

viebel Feb 9, 2018

viebel Feb 9, 2018

dviramontes Feb 16, 2018

dviramontes Feb 16, 2018

viebel Feb 16, 2018

dviramontes Feb 16, 2018 •

edited

Loading

dviramontes Feb 16, 2018 •

edited

Loading

feat: send custom event with code eval state payload #296

Are you sure you want to change the base?

feat: send custom event with code eval state payload #296

Conversation

KBHan commented Dec 3, 2017

dviramontes commented Dec 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viebel Dec 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dviramontes commented Dec 30, 2017

dviramontes commented Dec 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dviramontes Jan 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dviramontes commented Jan 22, 2018

viebel Jan 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dviramontes commented Feb 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dviramontes Feb 16, 2018 • edited Loading

Choose a reason for hiding this comment

dviramontes Feb 16, 2018 • edited Loading

Choose a reason for hiding this comment

viebel Dec 13, 2017 •

edited

Loading

dviramontes commented Dec 30, 2017 •

edited

Loading

dviramontes Jan 22, 2018 •

edited

Loading

viebel Jan 28, 2018 •

edited

Loading

dviramontes commented Feb 8, 2018 •

edited

Loading

dviramontes Feb 16, 2018 •

edited

Loading

dviramontes Feb 16, 2018 •

edited

Loading