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

Add LiveReload functionality to Jekyll. #5142

Merged
merged 2 commits into from Dec 6, 2017

Conversation

Projects
None yet
@awood
Contributor

awood commented Jul 27, 2016

Invoke using jekyll serve --livereload. The Serve command will inject
some necessary JavaScript links into pages. When loaded in a browser,
this JavaScript opens a connection over port 35729 using WebSockets to
an EventMachine reactor listening to that port in a separate thread.

When Jekyll finishes rendering a page, --livereload hooks create a
JSON message with the relative URL of the refreshed page. This message
is then pushed to the browser over the WebSockets connection. The
client-side JavaScript then reloads the page if the URL in the message
matches the URL of the current page.

Implements #4644

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Jul 27, 2016

Contributor

This PR doesn't include any significant documentation on the feature, but I figured I'd wait until some of the details get hammered out before writing anything.

Other notes:

  • It may be desirable to prune the number of options available to configure LiveReload. Specifically, the Flash version for browsers that don't support WebSockets might be worth dropping since no one is going to be developing there Jekyll site in Android and the overlap of IE 8 only users and Jekyll users is probably pretty small.
  • The semantics around --ignore are weird could probably lead to false bug reports. From my experiments and reading of the JavaScript (which I did not write), if any portion of URL the browser is pointed to matches the URL to reload in the JSON message, the browser will refresh. This is especially noticeable on index pages ("/blog/" would match every refresh message for pages in the directory)
  • The tests are very much functional tests instead of unit tests. In my opinion the functionality is complex enough to merit it, but others may disagree.
Contributor

awood commented Jul 27, 2016

This PR doesn't include any significant documentation on the feature, but I figured I'd wait until some of the details get hammered out before writing anything.

Other notes:

  • It may be desirable to prune the number of options available to configure LiveReload. Specifically, the Flash version for browsers that don't support WebSockets might be worth dropping since no one is going to be developing there Jekyll site in Android and the overlap of IE 8 only users and Jekyll users is probably pretty small.
  • The semantics around --ignore are weird could probably lead to false bug reports. From my experiments and reading of the JavaScript (which I did not write), if any portion of URL the browser is pointed to matches the URL to reload in the JSON message, the browser will refresh. This is especially noticeable on index pages ("/blog/" would match every refresh message for pages in the directory)
  • The tests are very much functional tests instead of unit tests. In my opinion the functionality is complex enough to merit it, but others may disagree.
@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Jul 27, 2016

Contributor

@envygeeks do you want revisions as separate commits to squash merge or for me to just adjust this current commit?

Contributor

awood commented Jul 27, 2016

@envygeeks do you want revisions as separate commits to squash merge or for me to just adjust this current commit?

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jul 27, 2016

Contributor

@awood whatever works best for you and allows you to flow the best! We normally only ask for a rebase just before we merge it, before that, it's whatever workflow you wish to use since Github now has that fancy "view all files changed".

Contributor

envygeeks commented Jul 27, 2016

@awood whatever works best for you and allows you to flow the best! We normally only ask for a rebase just before we merge it, before that, it's whatever workflow you wish to use since Github now has that fancy "view all files changed".

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jul 27, 2016

Contributor

I kinda like this, maybe we should add an updater like we've for normalize.css so that we can auto-update it every once in a while as it'll be quite easy to forget it exists. Other than that just a few comments and we can do another deeper review.

Contributor

envygeeks commented Jul 27, 2016

I kinda like this, maybe we should add an updater like we've for normalize.css so that we can auto-update it every once in a while as it'll be quite easy to forget it exists. Other than that just a few comments and we can do another deeper review.

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Jul 27, 2016

Contributor

I kinda like this, maybe we should add an updater like we've for normalize.css so that we can auto-update it every once in a while as it'll be quite easy to forget it exists. Other than that just a few comments and we can do another deeper review.

So you automatically pull the file every so often? That's a really good idea. I think I will need to modify the code somewhat because it's been modified slightly to read from variables defined in the inserted script tags, but I think I can figure something else out so the stock livereload.js will work

Contributor

awood commented Jul 27, 2016

I kinda like this, maybe we should add an updater like we've for normalize.css so that we can auto-update it every once in a while as it'll be quite easy to forget it exists. Other than that just a few comments and we can do another deeper review.

So you automatically pull the file every so often? That's a really good idea. I think I will need to modify the code somewhat because it's been modified slightly to read from variables defined in the inserted script tags, but I think I can figure something else out so the stock livereload.js will work

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Jul 28, 2016

Contributor

So you automatically pull the file every so often? That's a really good idea. I think I will need to modify the code somewhat because it's been modified slightly to read from variables defined in the inserted script tags, but I think I can figure something else out so the stock livereload.js will work

Okay, I've implemented the above. The PR now just uses the stock version of the most recent released livereload.js. Pulling it in via bower might be a good way of making sure it stays up to date, but that is something for the maintainers to decide.

Contributor

awood commented Jul 28, 2016

So you automatically pull the file every so often? That's a really good idea. I think I will need to modify the code somewhat because it's been modified slightly to read from variables defined in the inserted script tags, but I think I can figure something else out so the stock livereload.js will work

Okay, I've implemented the above. The PR now just uses the stock version of the most recent released livereload.js. Pulling it in via bower might be a good way of making sure it stays up to date, but that is something for the maintainers to decide.

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Aug 1, 2016

Contributor

Looks like this is failing in JRuby. I think the issue has to do with the WebSockets server thread introduced in this PR. I'll do some investigation, but if anyone out there has some experience with the subtleties of JRuby, I'd wouldn't mind a second opinion.

Contributor

awood commented Aug 1, 2016

Looks like this is failing in JRuby. I think the issue has to do with the WebSockets server thread introduced in this PR. I'll do some investigation, but if anyone out there has some experience with the subtleties of JRuby, I'd wouldn't mind a second opinion.

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Aug 2, 2016

Contributor

So I've made some changes for JRuby, but JRuby's TLS support is broken. I can either remove the TLS support for the WebSockets connection entirely or just ignore those options when running under JRuby (not sure if that's an acceptable solution though).

Additionally, while the tests pass, the actual functionality under JRuby is broken. For whatever reason, the message to reload is sent but the browser doesn't react. I'll look into this issue later and try and get some tests that actually ensure receipt of the reload command.

Contributor

awood commented Aug 2, 2016

So I've made some changes for JRuby, but JRuby's TLS support is broken. I can either remove the TLS support for the WebSockets connection entirely or just ignore those options when running under JRuby (not sure if that's an acceptable solution though).

Additionally, while the tests pass, the actual functionality under JRuby is broken. For whatever reason, the message to reload is sent but the browser doesn't react. I'll look into this issue later and try and get some tests that actually ensure receipt of the reload command.

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Sep 20, 2016

Contributor

I have tracked the JRuby functionality issue down to a problem within either EM-Websocket or EventMachine. I've filed an issue with EM-Websocket.

Contributor

awood commented Sep 20, 2016

I have tracked the JRuby functionality issue down to a problem within either EM-Websocket or EventMachine. I've filed an issue with EM-Websocket.

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Sep 20, 2016

Contributor

@envygeeks the Travis failures for this PR appear to be code style releated in code that I haven't modified. Otherwise I think this PR is ready for review at least. There's an outstanding issue with EM-Websocket/EventMachine under JRuby but I don't think that should block the review process. I can go ahead and address any issues the maintenance team sees while waiting to hear back from on the issue I filed for the JRuby bug.

Contributor

awood commented Sep 20, 2016

@envygeeks the Travis failures for this PR appear to be code style releated in code that I haven't modified. Otherwise I think this PR is ready for review at least. There's an outstanding issue with EM-Websocket/EventMachine under JRuby but I don't think that should block the review process. I can go ahead and address any issues the maintenance team sees while waiting to hear back from on the issue I filed for the JRuby bug.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Sep 20, 2016

Contributor

@awood I'll take some time this week to start reviewing /cc @jekyll/core @jekyll/ecosystem

Contributor

envygeeks commented Sep 20, 2016

@awood I'll take some time this week to start reviewing /cc @jekyll/core @jekyll/ecosystem

@envygeeks envygeeks assigned envygeeks and unassigned parkr Sep 20, 2016

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Sep 20, 2016

Contributor

Looks like the JRuby issue is a known problem with an EventMachine reactor running under a thread.

Contributor

awood commented Sep 20, 2016

Looks like the JRuby issue is a known problem with an EventMachine reactor running under a thread.

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Sep 21, 2016

Contributor

An alternative to having LiveReload just not work under JRuby would be to switch from WEBrick to an EventMachine compatible http server. Jekyll could then just start the EventMachine reactor when it's time to start serving and the reactor would manage both the LiveReload server on 35729 and the http server on 4000.

I actually have a working proof-of-concept using Thin as the http server, and it's pretty nice since it removes all the need for threading from Jekyll proper and just delegates everything to the EventMachine event loop. Plus EventMachine can daemonize so LiveReload would continue to work in daemon mode (whereas it does not in this PR since that would require Jekyll spinning off two processes which I thought a little dodgy especially if one process dies and the other doesn't).

The downside is that Thin doesn't run under JRuby at all. There are JRuby compatible http server (notably Puma) but they aren't EventMachine based and so there would have to be some sort of low-level shim layer that brokered between EventMachine's Connection class and the http server which would probably end up being a nightmare.

But if Thin ever runs under JRuby, I think the strategy described above would be a worthwhile refactoring.

Contributor

awood commented Sep 21, 2016

An alternative to having LiveReload just not work under JRuby would be to switch from WEBrick to an EventMachine compatible http server. Jekyll could then just start the EventMachine reactor when it's time to start serving and the reactor would manage both the LiveReload server on 35729 and the http server on 4000.

I actually have a working proof-of-concept using Thin as the http server, and it's pretty nice since it removes all the need for threading from Jekyll proper and just delegates everything to the EventMachine event loop. Plus EventMachine can daemonize so LiveReload would continue to work in daemon mode (whereas it does not in this PR since that would require Jekyll spinning off two processes which I thought a little dodgy especially if one process dies and the other doesn't).

The downside is that Thin doesn't run under JRuby at all. There are JRuby compatible http server (notably Puma) but they aren't EventMachine based and so there would have to be some sort of low-level shim layer that brokered between EventMachine's Connection class and the http server which would probably end up being a nightmare.

But if Thin ever runs under JRuby, I think the strategy described above would be a worthwhile refactoring.

@ghost

Looking good, just a few things that probably need changes. 😄

Show outdated Hide outdated lib/jekyll/commands/serve.rb
Show outdated Hide outdated lib/jekyll/commands/serve.rb
Show outdated Hide outdated lib/jekyll/commands/serve.rb
Show outdated Hide outdated lib/jekyll/commands/serve.rb
@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Sep 21, 2016

@awood

An alternative to having LiveReload just not work under JRuby would be to switch from WEBrick to an EventMachine compatible http server.
The downside is that Thin doesn't run under JRuby at all.

😆

I've never liked LiveReload but I'm sure it'll be a very useful feature for many Jekyll users.

ghost commented Sep 21, 2016

@awood

An alternative to having LiveReload just not work under JRuby would be to switch from WEBrick to an EventMachine compatible http server.
The downside is that Thin doesn't run under JRuby at all.

😆

I've never liked LiveReload but I'm sure it'll be a very useful feature for many Jekyll users.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Sep 21, 2016

Contributor

I've never really found it useful, I can F5 as fast as that thing loads lol. But at this point we need to bring in @jekyll/ecosystem and @jekyll/core so they can comment on your suggestions.

Contributor

envygeeks commented Sep 21, 2016

I've never really found it useful, I can F5 as fast as that thing loads lol. But at this point we need to bring in @jekyll/ecosystem and @jekyll/core so they can comment on your suggestions.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Sep 21, 2016

But you're polling in the test, not using the actual Queue!

ghost commented Sep 21, 2016

But you're polling in the test, not using the actual Queue!

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Sep 21, 2016

Contributor

But you're polling in the test, not using the actual Queue!

The test pollsrunning? which is in turn based on whether or not the @running Queue is empty or nil.

Contributor

awood commented Sep 21, 2016

But you're polling in the test, not using the actual Queue!

The test pollsrunning? which is in turn based on whether or not the @running Queue is empty or nil.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Sep 21, 2016

@awood that defeats the purpose of using Queue, a Queue is designed so you can Queue#pop without polling

ghost commented Sep 21, 2016

@awood that defeats the purpose of using Queue, a Queue is designed so you can Queue#pop without polling

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Sep 21, 2016

Contributor

@awood that defeats the purpose of using Queue, a Queue is designed so you can Queue#pop without polling

Yeah, I'm using the Queue more like "if the Queue has anything in it, that means Webrick and EventMachine are running" and it only gets cleared when the servers shutdown. So the queue is really just a semaphore type thing I suppose. I'm open to suggestions on the implementation. My main goal is to avoid sporadic test failures due to tests running before the server is up. Maybe using a Mutex would make more sense?

(I think this is the exact SO article that I used for reference when I ran into the "wait for the web server to be listening before running tests" issue. Looking at some of the other answers it looks like a Mutex + ConditionalVariable would be more appropriate.)

Contributor

awood commented Sep 21, 2016

@awood that defeats the purpose of using Queue, a Queue is designed so you can Queue#pop without polling

Yeah, I'm using the Queue more like "if the Queue has anything in it, that means Webrick and EventMachine are running" and it only gets cleared when the servers shutdown. So the queue is really just a semaphore type thing I suppose. I'm open to suggestions on the implementation. My main goal is to avoid sporadic test failures due to tests running before the server is up. Maybe using a Mutex would make more sense?

(I think this is the exact SO article that I used for reference when I ran into the "wait for the web server to be listening before running tests" issue. Looking at some of the other answers it looks like a Mutex + ConditionalVariable would be more appropriate.)

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Sep 23, 2016

Contributor

@envygeeks @spudowiar Is there anyway to unregister a hook once it's been registered? The test failures are order dependent (seed 52792 fails while seed 59920 passes). I think what's happening is that the LiveReload hook is getting registered and then later Serve is getting reinitialized and resetting a variable that the hook references to nil. I can work around that by just always initializing a LiveReload reactor but only start it when the --livereload option is given, but I think it'd be better to have my tests clean up after themselves.

Contributor

awood commented Sep 23, 2016

@envygeeks @spudowiar Is there anyway to unregister a hook once it's been registered? The test failures are order dependent (seed 52792 fails while seed 59920 passes). I think what's happening is that the LiveReload hook is getting registered and then later Serve is getting reinitialized and resetting a variable that the hook references to nil. I can work around that by just always initializing a LiveReload reactor but only start it when the --livereload option is given, but I think it'd be better to have my tests clean up after themselves.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Sep 23, 2016

Contributor

@awood there isn't, because hooks aren't named.

Contributor

envygeeks commented Sep 23, 2016

@awood there isn't, because hooks aren't named.

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Sep 23, 2016

Contributor

I'm not sure how to interpret those Appveyor build errors. Most of them look like code I haven't touched.

Contributor

awood commented Sep 23, 2016

I'm not sure how to interpret those Appveyor build errors. Most of them look like code I haven't touched.

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Dec 6, 2017

Member

@awood Could you please fix the remaining conflicts? Sorry for not merging this sooner, it got approved and should ship in next minor version.

Member

DirtyF commented Dec 6, 2017

@awood Could you please fix the remaining conflicts? Sorry for not merging this sooner, it got approved and should ship in next minor version.

Add LiveReload functionality to Jekyll
Invoke using jekyll serve --livereload. The Serve command will inject
some necessary JavaScript links into pages. When loaded in a browser,
this JavaScript opens a connection over port 35729 using WebSockets to
an EventMachine reactor listening to that port in a separate thread.

When Jekyll finishes rendering a page, --livereload hooks create a
JSON message with the relative URL of the refreshed page. This message
is then pushed to the browser over the WebSockets connection. The
client-side JavaScript then reloads the page if the URL in the message
matches the URL of the current page.

Implements #4644
@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Dec 6, 2017

Contributor

@DirtyF Conflicts resolved

Contributor

awood commented Dec 6, 2017

@DirtyF Conflicts resolved

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Dec 6, 2017

Member

Can you skip the specs that use LiveReload on Windows if they won't work?

Member

parkr commented Dec 6, 2017

Can you skip the specs that use LiveReload on Windows if they won't work?

@DirtyF DirtyF added the accepted label Dec 6, 2017

@DirtyF DirtyF added this to the v3.7.0 milestone Dec 6, 2017

Address issues from code review of LiveReload.
* Improve variable names
* Improve resiliency when conflicting options are given
* Refactor the LiveReloadReactor class into a separate file
* Correct typos
* Use alias_method for brevity
* Use __dir__ instead of File.dirname(__FILE__)
* Set and use default LiveReload port
@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Dec 6, 2017

Contributor

Can you skip the specs that use LiveReload on Windows if they won't work?

Done!

Contributor

awood commented Dec 6, 2017

Can you skip the specs that use LiveReload on Windows if they won't work?

Done!

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Dec 6, 2017

Member

💯 A big thanks @awood for all the work you put into this, I hope this really improves the developer experience for a majority of Jekyll users.

@jekyllbot: merge +minor

Member

DirtyF commented Dec 6, 2017

💯 A big thanks @awood for all the work you put into this, I hope this really improves the developer experience for a majority of Jekyll users.

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 9d68b1b into jekyll:master Dec 6, 2017

3 checks passed

WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Dec 6, 2017

Member

@awood

If I add the --livereload option on the command line this works fine. (I only tested this before merging)

But if I add the "livereload" => "true" option to our rake file then the following message appears:

rake aborted!
NoMethodError: undefined method `start' for nil:NilClass
/Users/frank/code/jekyll/jekyll/lib/jekyll/commands/serve.rb:224:in `start_up_webrick'
/Users/frank/code/jekyll/jekyll/lib/jekyll/commands/serve.rb:104:in `process'
/Users/frank/code/jekyll/jekyll/rake/site.rake:37:in `block (2 levels) in <top (required)>'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:251:in `block in execute'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:251:in `each'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:251:in `execute'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:195:in `block in invoke_with_call_chain'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/2.4.0/monitor.rb:214:in `mon_synchronize'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:188:in `invoke_with_call_chain'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:181:in `invoke'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:160:in `invoke_task'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:116:in `block (2 levels) in top_level'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:116:in `each'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:116:in `block in top_level'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:125:in `run_with_threads'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:110:in `top_level'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:83:in `block in run'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:186:in `standard_exception_handling'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:80:in `run'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/exe/rake:27:in `<top (required)>'
/Users/frank/.rbenv/versions/2.4.2/bin/rake:23:in `load'
/Users/frank/.rbenv/versions/2.4.2/bin/rake:23:in `<top (required)>'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli/exec.rb:75:in `load'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli/exec.rb:75:in `kernel_load'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli/exec.rb:28:in `run'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli.rb:424:in `exec'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/vendor/thor/lib/thor.rb:387:in `dispatch'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli.rb:27:in `dispatch'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/vendor/thor/lib/thor/base.rb:466:in `start'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli.rb:18:in `start'
/Users/frank/.rbenv/versions/2.4.2/bin/bundle:30:in `block in <main>'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/friendly_errors.rb:122:in `with_friendly_errors'
/Users/frank/.rbenv/versions/2.4.2/bin/bundle:22:in `<main>'
Tasks: TOP => site:preview
Member

DirtyF commented Dec 6, 2017

@awood

If I add the --livereload option on the command line this works fine. (I only tested this before merging)

But if I add the "livereload" => "true" option to our rake file then the following message appears:

rake aborted!
NoMethodError: undefined method `start' for nil:NilClass
/Users/frank/code/jekyll/jekyll/lib/jekyll/commands/serve.rb:224:in `start_up_webrick'
/Users/frank/code/jekyll/jekyll/lib/jekyll/commands/serve.rb:104:in `process'
/Users/frank/code/jekyll/jekyll/rake/site.rake:37:in `block (2 levels) in <top (required)>'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:251:in `block in execute'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:251:in `each'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:251:in `execute'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:195:in `block in invoke_with_call_chain'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/2.4.0/monitor.rb:214:in `mon_synchronize'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:188:in `invoke_with_call_chain'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/task.rb:181:in `invoke'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:160:in `invoke_task'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:116:in `block (2 levels) in top_level'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:116:in `each'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:116:in `block in top_level'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:125:in `run_with_threads'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:110:in `top_level'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:83:in `block in run'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:186:in `standard_exception_handling'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/lib/rake/application.rb:80:in `run'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/rake-12.3.0/exe/rake:27:in `<top (required)>'
/Users/frank/.rbenv/versions/2.4.2/bin/rake:23:in `load'
/Users/frank/.rbenv/versions/2.4.2/bin/rake:23:in `<top (required)>'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli/exec.rb:75:in `load'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli/exec.rb:75:in `kernel_load'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli/exec.rb:28:in `run'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli.rb:424:in `exec'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/vendor/thor/lib/thor.rb:387:in `dispatch'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli.rb:27:in `dispatch'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/vendor/thor/lib/thor/base.rb:466:in `start'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/cli.rb:18:in `start'
/Users/frank/.rbenv/versions/2.4.2/bin/bundle:30:in `block in <main>'
/Users/frank/.rbenv/versions/2.4.2/lib/ruby/site_ruby/2.4.0/bundler/friendly_errors.rb:122:in `with_friendly_errors'
/Users/frank/.rbenv/versions/2.4.2/bin/bundle:22:in `<main>'
Tasks: TOP => site:preview

DirtyF added a commit that referenced this pull request Dec 7, 2017

DirtyF added a commit that referenced this pull request Dec 7, 2017

@burrich

This comment has been minimized.

Show comment
Hide comment
@burrich

burrich Jan 24, 2018

Hi, can you confirm that livereload is not supported on Windows please ?

@awood

burrich commented Jan 24, 2018

Hi, can you confirm that livereload is not supported on Windows please ?

@awood

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Jan 24, 2018

Member

@burrich --livereload is definitely supported on Windows.. just not with --ssl_cert and --ssl_key as well..

Member

ashmaroli commented Jan 24, 2018

@burrich --livereload is definitely supported on Windows.. just not with --ssl_cert and --ssl_key as well..

@burrich

This comment has been minimized.

Show comment
Hide comment
@burrich

burrich Jan 24, 2018

Ok thanks @ashmaroli , i will try.

burrich commented Jan 24, 2018

Ok thanks @ashmaroli , i will try.

@awood

This comment has been minimized.

Show comment
Hide comment
@awood

awood Jan 24, 2018

Contributor

@burrich If I recall correctly, it won't work with Ruby 2.4 on Windows due to a dependency (EventMachine) not working.

Contributor

awood commented Jan 24, 2018

@burrich If I recall correctly, it won't work with Ruby 2.4 on Windows due to a dependency (EventMachine) not working.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Jan 24, 2018

Member

won't work with Ruby 2.4 on Windows due to a dependency (EventMachine) not working.

@burrich That issue is being tracked here 👉 eventmachine/eventmachine#806

Member

ashmaroli commented Jan 24, 2018

won't work with Ruby 2.4 on Windows due to a dependency (EventMachine) not working.

@burrich That issue is being tracked here 👉 eventmachine/eventmachine#806

# EventMachine's SSL support in Windows requires building the gem's
# native extensions against OpenSSL and that proved to be a process
# so tedious that expecting users to do it is a non-starter.
Jekyll.logger.abort_with "Error:", "LiveReload does not support SSL"

This comment has been minimized.

@RobertDeRose

RobertDeRose Jan 24, 2018

I've stepped way back from this PR, never had the time to look at it as my plugin was just working for me. However, Livereload can support SSL, I have support for it in my plugin RobertDeRose/jekyll-livereload@34df507

@RobertDeRose

RobertDeRose Jan 24, 2018

I've stepped way back from this PR, never had the time to look at it as my plugin was just working for me. However, Livereload can support SSL, I have support for it in my plugin RobertDeRose/jekyll-livereload@34df507

This comment has been minimized.

@DirtyF

DirtyF Jan 24, 2018

Member

Thanks @RobertDeRose, I was using your plugin until now :)
Are you interested in submitting a patch?

@DirtyF

DirtyF Jan 24, 2018

Member

Thanks @RobertDeRose, I was using your plugin until now :)
Are you interested in submitting a patch?

This comment has been minimized.

@RobertDeRose

RobertDeRose Jan 24, 2018

I might, if I can find the time. So, that's a big maybe

@RobertDeRose

RobertDeRose Jan 24, 2018

I might, if I can find the time. So, that's a big maybe

This comment has been minimized.

@awood

awood Jan 25, 2018

Contributor

@RobertDeRose The problem wasn't with LiveReload not supporting SSL; it was with getting EventMachine and OpenSSL to cooperated on Windows. I actually had it working fine in Linux, but getting the OpenSSL libraries to install and work on the Windows CI environment was a nightmare, so I just gave up 😭. I figured most people would only be using LiveReload for rapid prototyping so I didn't consider SSL support to be a must-have.

@awood

awood Jan 25, 2018

Contributor

@RobertDeRose The problem wasn't with LiveReload not supporting SSL; it was with getting EventMachine and OpenSSL to cooperated on Windows. I actually had it working fine in Linux, but getting the OpenSSL libraries to install and work on the Windows CI environment was a nightmare, so I just gave up 😭. I figured most people would only be using LiveReload for rapid prototyping so I didn't consider SSL support to be a must-have.

This comment has been minimized.

@RobertDeRose

RobertDeRose Jan 26, 2018

@awood that's a fair point, I never did testing for Windows, and to be honest, not a lot of benefit for it when developing locally, unless you really need to ensure SSL isn't messing with thing, but at that point, you don't really need live reload. OK ignore my extremely late to the party comments then.

@RobertDeRose

RobertDeRose Jan 26, 2018

@awood that's a fair point, I never did testing for Windows, and to be honest, not a lot of benefit for it when developing locally, unless you really need to ensure SSL isn't messing with thing, but at that point, you don't really need live reload. OK ignore my extremely late to the party comments then.

@Sphinxs

This comment has been minimized.

Show comment
Hide comment
@Sphinxs

Sphinxs commented Aug 3, 2018

Fixed ?

@jekyll jekyll locked as resolved and limited conversation to collaborators Aug 3, 2018

@jekyll jekyll deleted a comment Aug 3, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.