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 support for QML to Weasel #15

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
7 changes: 4 additions & 3 deletions project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
:url "http://unlicense.org/UNLICENSE"
:distribution :repo}

:dependencies [[org.clojure/clojure "1.5.1"]
[org.clojure/clojurescript "0.0-2202" :scope "provided"]
[org.clojure/google-closure-library "0.0-20140226-71326067" :scope "provided"]
:dependencies [[org.clojure/clojure "1.6.0"]
[org.clojure/clojurescript "0.0-2261" :scope "provided"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not quite sure how provided dependencies work - does this mean users of weasel will be locked into using 0.0-2261 or newer? I'm concerned about forcing Safari users to upgrade and breaking their repl (see #17).

Copy link
Author

Choose a reason for hiding this comment

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

Looks like this bug just got fixed - would you want to push it to 0.0-2268? I am also a little unclear as to how provided dependencies work. I did find there was some issue compiling the REPL for QML with 0.0-2202 - that's why I upgraded. Your call really what dependencies to use - I wanted to exclude project.clj from the pull request, but couldn't find a way to do that.

;;[org.clojure/google-closure-library "0.0-20140226-71326067" :scope "provided"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

(also, can you remove these commented-out lines?)

Copy link
Author

Choose a reason for hiding this comment

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

sorry about that!

;;[org.clojure/google-closure-library "0.0-20130212-95c19e7f0f5f"]
[http-kit "2.1.18"]]

:profiles {:dev {:dependencies [[com.cemerick/piggieback "0.1.3"]]}}
Expand Down
2 changes: 1 addition & 1 deletion src/clj/weasel/repl/websocket.clj
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
(binding [*out* (or @repl-out *out*)]
(send-for-eval! (cljsc/compile-form-seq
'[(ns cljs.user)
(set-print-fn! weasel.repl/repl-print)])))))
(set-print-fn! @weasel.impls.print/print-fn)])))))

(defn- websocket-setup-env
[this]
Expand Down
3 changes: 3 additions & 0 deletions src/cljs/weasel/impls/print.cljs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
(ns weasel.impls.print)

(def print-fn (atom (constantly nil)))
4 changes: 3 additions & 1 deletion src/cljs/weasel/repl.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
(:require [clojure.browser.event :as event :refer [event-types]]
[clojure.browser.net :as net]
[cljs.reader :as reader :refer [read-string]]
[weasel.impls.websocket :as ws]))
[weasel.impls.websocket :as ws]
[weasel.impls.print :as wp]))

(def ^:private ws-connection (atom nil))

Expand Down Expand Up @@ -45,6 +46,7 @@
[repl-server-url & {:keys [verbose on-open on-error on-close] :or {verbose true}}]
(let [repl-connection (ws/websocket-connection)]
(swap! ws-connection (constantly repl-connection))
(swap! wp/print-fn (constantly repl-print))

(event/listen repl-connection :opened
(fn [evt]
Expand Down
91 changes: 91 additions & 0 deletions src/cljs/weasel/repl/qml.cljs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
(ns weasel.repl.qml
(:require
[cljs.reader :as reader :refer [read-string]]
[weasel.impls.print :as wp]))

(def ^:private ws-connection (atom nil))

(defn alive? []
"Returns truthy value if the REPL is attempting to connect or is
connected, or falsy value otherwise."
(not (nil? @ws-connection)))

(defmulti process-message :op)

(defmethod process-message
:error
[message]
(.error js/console (str "Websocket REPL error " (:type message))))

(defmethod process-message
:eval-js
[message]
(let [code (:code message)]
{:op :result
:value (try
{:status :success, :value (str (js* "eval(~{code})"))}
(catch js/Error e
{:status :exception
:value (pr-str e)
:stacktrace (if (.hasOwnProperty e "stack")
(.-stack e)
"No stacktrace available.")})
(catch :default e
{:status :exception
:value (pr-str e)
:stacktrace "No stacktrace available."}))}))

(defn repl-print
[x]
(if-let [conn @ws-connection]
(.sendTextMessage @ws-connection (pr-str {:op :print :value (pr-str x)}))))

(def ws-status
{0 :Connecting
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you make these lowercase?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, definitely. The reason they're uppercase is because they represent the QML enum's that can't be imported directly from JS - http://qt-project.org/doc/qt-5/qml-qt-websockets-websocket.html#status-prop. But, I guess in Clojure, the convention would be to make everything lowercase where possible, correct?

1 :Open
2 :Closing
3 :Closed
4 :Error})

(defn connect [qml-parent repl-server-url & {:keys [verbose on-open on-error on-close]}]
"Connects to a Weasel REPL socket at the provided repl-server-url. The underlying implementation
depends on Qt.WebSockets (found in Qt 5.3 or higher). Because this is a Qml object it requires
a non-nil parent object passed in as qml-parent."
(let [repl-connection
(.createQmlObject
js/Qt
(str "import Qt.WebSockets 1.0; WebSocket {url: \"" repl-server-url "\"}")
;;"import Qt.WebSockets 1.0; WebSocket {}"
qml-parent
"weasel.repl.qml.websocket")]
(swap! ws-connection (constantly repl-connection))
(swap! wp/print-fn (constantly repl-print))
(.connect (.-onStatusChanged repl-connection)
(fn [status]
(let [status (ws-status status)]
(cond
(= status :Open)
(do
(.sendTextMessage repl-connection (pr-str {:op :ready}))
(when verbose (.info js/console "Opened Websocket REPL connection"))
(when (fn? on-open) (on-open)))

(= status :Closed)
(do
(reset! ws-connection nil)
(when verbose (.info js/console "Closed Websocket REPL connection"))
(when (fn? on-close) (on-close)))

(= status :Error)
(do
(.error js/console "WebSocket error" (.-errorString repl-connection))
Copy link
Collaborator

Choose a reason for hiding this comment

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

the browser REPL client now only logs an error when verbose is truthy, can you make the same change here?

Copy link
Author

Choose a reason for hiding this comment

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

Sure.

(when (fn? on-error) (on-error (.-errorString repl-connection))))))))
(.connect (.-onTextMessageReceived repl-connection)
(fn [msg]
(when verbose (.log js/console (str "Received: " msg)))
(let [{:keys [op] :as message} (read-string msg)
response (-> message process-message pr-str)]
(when verbose (.log js/console (str "Sending: " response)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Right now verbose is default and this seems a little bit noisy, can you remove these? I'm planning to make logging a little more tuneable and these logs could return when a debug log level is active or something.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, could we add a debug level? There are times especially with QML that it's useful to see this output. Many times I found QML throwing exceptions and then had to look into these messages to see what the problem was - that's partially how I figured out what tweaks were needed for the QML environment - not easy!

Would it be possible to have the log level as an atom somewhere? There may be cases where one wants to enable the debug level during an active session.

(.sendTextMessage repl-connection response))))
(set! (.-active repl-connection) true)
repl-connection))