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.

Sign up for GitHub

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

Jump to bottom

Cleanup LiveReloadReactor #6607

Merged
merged 6 commits into from
Dec 14, 2017
Merged

Cleanup LiveReloadReactor #6607

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7541820
use braces for blocks with a single-line body
ashmaroli Dec 7, 2017
0810a20
Jekyll.logger is usually called without brackets
ashmaroli Dec 7, 2017
69eec47
raise error and use `--trace` for backtrace msgs
ashmaroli Dec 8, 2017
aaee438
remove unnecessary ivar initialization
ashmaroli Dec 8, 2017
4de0137
format messages in sync with elsewhere on Jekyll
ashmaroli Dec 9, 2017
6d90e87
dump reload json data into a local variable
ashmaroli Dec 14, 2017
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
60 changes: 17 additions & 43 deletions lib/jekyll/commands/serve/live_reload_reactor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ class LiveReloadReactor
attr_reader :thread

def initialize
@thread = nil
@websockets = []
@connections_count = 0
@started_event = Utils::ThreadEvent.new
Expand All @@ -25,29 +24,18 @@ def stop
# There is only one EventMachine instance per Ruby process so stopping
# it here will stop the reactor thread we have running.
EM.stop if EM.reactor_running?
Jekyll.logger.debug("LiveReload Server:", "halted")
Jekyll.logger.debug "LiveReload Server:", "halted"
end

def running?
EM.reactor_running?
end

def handle_websockets_event(ws)
ws.onopen do |handshake|
connect(ws, handshake)
end

ws.onclose do
disconnect(ws)
end

ws.onmessage do |msg|
print_message(msg)
end

ws.onerror do |error|
log_error(error)
end
ws.onopen { |handshake| connect(ws, handshake) }
ws.onclose { disconnect(ws) }
ws.onmessage { |msg| print_message(msg) }
ws.onerror { |error| log_error(error) }
end

# rubocop:disable Metrics/MethodLength
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this comment:

lib/jekyll/commands/serve/live_reload_reactor.rb:41:9: W: [Corrected] Unnecessary disabling of Metrics/MethodLength.
        # rubocop:disable Metrics/MethodLength

Expand All @@ -56,9 +44,7 @@ def start(opts)
# Use epoll if the kernel supports it
EM.epoll
EM.run do
EM.error_handler do |e|
log_error(e)
end
EM.error_handler { |e| log_error(e) }

EM.start_server(
opts["host"],
Expand All @@ -70,17 +56,11 @@ def start(opts)
end

# Notify blocked threads that EventMachine has started or shutdown
EM.schedule do
@started_event.set
end

EM.add_shutdown_hook do
@stopped_event.set
end
EM.schedule { @started_event.set }
EM.add_shutdown_hook { @stopped_event.set }

Jekyll.logger.info(
"LiveReload address:", "#{opts["host"]}:#{opts["livereload_port"]}"
)
Jekyll.logger.info "LiveReload address:",
"#{opts["host"]}:#{opts["livereload_port"]}"
end
end
@thread.abort_on_exception = true
Expand All @@ -96,11 +76,9 @@ def reload(pages)
:liveCSS => true,
}

Jekyll.logger.debug("LiveReload:", "Reloading #{p.url}")
Jekyll.logger.debug(JSON.dump(msg))
@websockets.each do |ws|
ws.send(JSON.dump(msg))
end
Jekyll.logger.debug "LiveReload:", "Reloading #{p.url}"
Jekyll.logger.debug JSON.dump(msg)
@websockets.each { |ws| ws.send(JSON.dump(msg)) }
Copy link
Member Author

Choose a reason for hiding this comment

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

@awood does JSON.dump(msg) need to be called separately for each websocket or can I store the value of the first dump to a local variable and then use that variable in the block..?

Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to store the serialized JSON and re-use it. Not sure why I didn't do that in the first place. I believe @websockets will generally just have one item in it anyway (each item in the list represents a browser that's connected IIRC) unless someone is really excited about livereload. But it would be better just to dump it to JSON once per msg

end
end

Expand All @@ -110,7 +88,7 @@ def connect(ws, handshake)
if @connections_count == 1
message = "Browser connected"
message += " over SSL/TLS" if handshake.secure?
Jekyll.logger.info("LiveReload:", message)
Jekyll.logger.info "LiveReload:", message
end
ws.send(
JSON.dump(
Expand All @@ -134,18 +112,14 @@ def print_message(json_message)
# Not sure what the 'url' command even does in LiveReload. The spec is silent
# on its purpose.
if msg["command"] == "url"
Jekyll.logger.info("LiveReload:", "Browser URL: #{msg["url"]}")
Jekyll.logger.info "LiveReload:", "Browser URL: #{msg["url"]}"
end
end

private
def log_error(e)
Jekyll.logger.warn(
"LiveReload experienced an error. "\
"Run with --verbose for more information."
)
Jekyll.logger.debug("LiveReload Error:", e.message)
Jekyll.logger.debug("LiveReload Error:", e.backtrace.join("\n"))
raise e, "LiveReload experienced an error. " \
"Run with --trace for more information."
end
end
end
Expand Down