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

loadurl blocks until page is loaded #43

Closed
wants to merge 2 commits into from

Conversation

EricForgy
Copy link
Contributor

Closes #41

@EricForgy EricForgy mentioned this pull request Dec 29, 2015
@EricForgy
Copy link
Contributor Author

This needs revision, but I am reopening so that when I make the revision, I don't need to open a new PR.

@EricForgy EricForgy closed this Dec 29, 2015
Makes socket available to eval'ed code in main.js .
@EricForgy EricForgy reopened this Dec 30, 2015
@EricForgy
Copy link
Contributor Author

Hi @Varanas, @one-more-minute and @spencerlyon2 ,

I'm not sure how you feel about this change, but I like it and hope you do too (?)

This PR is based on #45 because I figure if you don't like that one, you won't like this one either 😅

These two PRs make it friendlier, in my experience, to program with and to test Blink.

For example, now we can do something like this in code:

    w = Window()
    loadurl("http://www.julialang.org")
    println("Got it!")
    close(w)

With asynchronous Window() and loadurl, there is no way I can think of to run the above code without manually waiting for each step to complete.

This will also make writing tests cleaner. Instead of

w = Window(); sleep(5.0)
@test 1 == 1

we can do things like

@test length(Blink.windows()) == 0
w = Window()
@test length(Blink.windows()) == 1
close(w)
@test length(Blink.windows()) == 0

What do you think?

@pfitzseb
Copy link
Member

I do think making these operations blocking is a good idea! That said, I haven't actually worked with Blink yet, so that opinion is purely theoretical ;)

@EricForgy
Copy link
Contributor Author

Haha :D Thanks for the theoretical support :)

There may be a better way to accomplish the same thing, so I am interested in suggestions too.

@sglyon
Copy link
Contributor

sglyon commented Jan 7, 2016

Hey @EricForgy you say that this and #45 implement the same thing. Do you think it is a good idea to close one of these, or should we review both?

@EricForgy
Copy link
Contributor Author

Hi @spencerlyon2 ,

#45 just has the blocking for opening a Window and this PR adds to #45 by also blocking loadurl. I suppose I can close #45 in favor of this one because this one includes #45 and if there are any issues with #45, they are probably also issues with this PR.

Sorry for the confusion. I'll go ahead and close #45. Thanks!

@sglyon
Copy link
Contributor

sglyon commented Jan 8, 2016

I will defer to @one-more-minute on this one.

He's done some real work to maintain a lot of js async semantics and I'm not sure if he wants to move away from that.

@twavv
Copy link
Member

twavv commented Jul 30, 2019

Closing as stale.

@twavv twavv closed this Jul 30, 2019
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.

Make loadurl synchronous
4 participants