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

Conversation

aaronc
Copy link

@aaronc aaronc commented Jul 4, 2014

Qt 5.3 added a QtWebsockets module which makes Weasel the perfect candidate for a QML/JS Clojurescript repl.

I added a weasel.repl.qml namespace which tries to mirror weasel.repl as much as possible.

I also added a trivial weasel.impls.print ns to so that the same repl server can set the print function regardless of whether it's a regular Websocket, QML or possibly say a Node.js repl.

I posted details here about how I actually got a REPL working in QML: https://groups.google.com/forum/#!topic/clojurescript/bSBfH1hazSg

@tomjakubowski
Copy link
Collaborator

Wow, this sounds very cool! I will take a look this weekend.

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

@tomjakubowski
Copy link
Collaborator

Overall this looks very nice! I'll have some more 'bigger picture' comments later. I'm very excited by the prospect of running Weasel REPL clients in non-browser JS environments!

[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"]
;;[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!

@tomjakubowski
Copy link
Collaborator

I'd love to play around with this but I'm stuck on getting a QML + ClojureScript example set up. Do you have a demo project somewhere where I could try this out?

I'm concerned by how much code is duplicated from weasel.repl in weasel.repl.qml. Notice how weasel.repl doesn't call much Closure WebSocket code directly, but instead calls functions defined on the net/IConnection protocol and the polymorphic event/listen function to listen for events on the current connection object.

We could make weasel.repl totally implementation-agnostic by allowing connect to take an optional keyword parameter as a "connection constructor function", defaulting to ws/websocket-connection, and an optional parameter of keyword arguments to pass on to the net/connect function after the server URL. Then we could have a new namespace weasel.impls.qml which contains implementations of net/IConnection and event/EventType (as well as the appropriate event dispatching code) for the QML WebSocket type and the appropriate connection constructor.

Connecting from QML might look something like:

(ns example
  (:require [weasel.impls.qml :as qml-ws])

(weasel.repl/connect
  :connection-fn qml-ws/connection
  :connection-args {:qml-parent ... })

What do you think?

@aaronc
Copy link
Author

aaronc commented Jul 6, 2014

So, getting QML and Clojurescript running together isn't super easy - it was actually somewhat of a feat to figure it out and there are still some idiosyncracies. I described my approach here: https://groups.google.com/forum/#!topic/clojurescript/bSBfH1hazSg. I intend to put together a little example project or even a lein template... I'll let you know when I have something...

I do agree that it would be nice to have a common implementation. I think my initial thought was to do it this way, but because it seemed complex I did it the quickest way that worked. In QML, for instance, the event system is very different. It probably is possible if I implement a type that implements goog.events.Listenable and converts the status. Also, I'm not sure how to use extend-type on QML types yet because QML doesn't give you a type constructor directly but requires this sort of convoluted Qt.createQmlObject to be called with a string of QML. But, I could wrap the WebSocket in a deftype and it should work. So, I think it's possible but just a little more complex. Would you want me to see if I can create an impl that uses weasel.repl directly as it is?

How about doing what you said with a connection-fn but also creating a helper function such as this?

(defn qml-repl [parent url ...]
  (weasel.repl/connect
    :connection-fn qml-ws/connection
    :connection-args {:qml-parent ... }))

@aaronc
Copy link
Author

aaronc commented Jul 7, 2014

Hey Tom, I made a lein template that should hopefully work.

If you execute lein new qml-cljs test-qml-cljs --snapshot this should generate an example project which has a README.md with quickstart instructions.

If you get a chance to try this, let me know if you run into any problems. Thx!

@tomjakubowski
Copy link
Collaborator

Hey! I haven't forgotten about this, just have been lacking time. I should be able to carve out some time this weekend to hopefully land this!

@MalloZup
Copy link

MalloZup commented Aug 5, 2019

autogenerated with https://github.com/MalloZup/doghub: pr inactive since 200 days. Please update the PR or close it

@MalloZup
Copy link

MalloZup commented Aug 5, 2019

autogenerated with https://github.com/MalloZup/doghub: issue inactive since 450 days. Please update the issue or close it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants