Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Cleanup the interactor code #355

Closed
wants to merge 7 commits into from

3 participants

@netzpirat
Owner

This is a follow up to the Pry pull request #327 and introduces a continuous Pry session, so no stop/start cycle of the interactor on a Guard action anymore. It also cleans up some legacy support code that isn't needed anymore.

Current changes:

  • Remove interactor thread
  • Remove listen mutex
  • Remove tty store/restore

Known issues:

  • The prompt is not visible after a Guard action. We need to redraw the current line after a Guard action has been finished.
  • Using binding.pry starts another Pry instance.
netzpirat added some commits
@netzpirat netzpirat Cleanup Pry interactor.
Remove all the interactor start and stop synchronization
and the terminal setting reset  completely, just to see
how well Pry does.
def484c
@netzpirat netzpirat Merge branch 'master' into interactor/cleanup
Conflicts:
	lib/guard.rb
477fe9b
@netzpirat netzpirat Merge branch 'master' into interactor/cleanup b3861e2
@netzpirat netzpirat Remove interactor threading code.
This cleans up old thread synchronization
code that is not used anymore with a
continuous Pry session.
5774c8f
@rking

I love the way the diffs are shaping up, but I am also having a few issues.

I have one Rails project where, if I require'pry';binding.pry at some point in test execution, the prompt comes up but is in contention with the Guard interactor Pry.

That same project does similarly with pry-rescue/minitest Pries.

I tried to replicate it on a tiny project but it doesn't exhibit the same behavior. Do you see it on your end, or should I bisect these two to figure out what triggers it? Sorry for not doing it right away — am at work and will be busy for next several evenings.

@netzpirat
Owner

Thanks for the feedback. I've added it to the known issues and will have a look soon.

@netzpirat
Owner

Since there's no way that bindings.pry would stop the Guard Pry session in another thread, the idea of a continuously running Guard Pry session will never work. So I'm going to abandon this pull request and cherry-pick the good parts.

@netzpirat netzpirat closed this
@thibaudgg
Owner

:cry:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 27, 2012
  1. @netzpirat

    Cleanup Pry interactor.

    netzpirat authored
    Remove all the interactor start and stop synchronization
    and the terminal setting reset  completely, just to see
    how well Pry does.
  2. @netzpirat

    Merge branch 'master' into interactor/cleanup

    netzpirat authored
    Conflicts:
    	lib/guard.rb
Commits on Oct 30, 2012
  1. @netzpirat
  2. @netzpirat

    Remove interactor threading code.

    netzpirat authored
    This cleans up old thread synchronization
    code that is not used anymore with a
    continuous Pry session.
Commits on Nov 5, 2012
  1. @netzpirat

    Merge branch 'master' into interactor/cleanup

    netzpirat authored
    Conflicts:
    	lib/guard.rb
  2. @netzpirat

    Cleanup specs.

    netzpirat authored
  3. @netzpirat
This page is out of date. Refresh to see the latest.
View
60 lib/guard.rb
@@ -22,7 +22,7 @@ module Guard
HOME_TEMPLATES = File.expand_path('~/.guard/templates')
class << self
- attr_accessor :options, :interactor, :runner, :listener, :lock
+ attr_accessor :options, :interactor, :runner, :listener
# Initialize the Guard singleton:
#
@@ -40,11 +40,9 @@ class << self
# @deprecated @option options [Boolean] no_vendor ignore vendored dependencies
#
def setup(options = {})
- @lock = Mutex.new
@options = options.dup
@watchdir = (options[:watchdir] && File.expand_path(options[:watchdir])) || Dir.pwd
@runner = ::Guard::Runner.new
- @allow_stop = Listen::Turnstile.new
::Guard::UI.clear(:force => true)
deprecated_options_warning
@@ -110,9 +108,7 @@ def setup_listener
listener_callback = lambda do |modified, added, removed|
::Guard::Dsl.reevaluate_guardfile if ::Guard::Watcher.match_guardfile?(modified)
- ::Guard.within_preserved_state do
- runner.run_on_changes(modified, added, removed)
- end
+ runner.run_on_changes(modified, added, removed)
end
listener_options = { :relative_paths => true }
@@ -158,24 +154,17 @@ def start(options = {})
setup(options)
::Guard::UI.info "Guard is now watching at '#{ @watchdir }'"
- within_preserved_state do
- runner.run(:start)
- end
-
+ runner.run(:start)
listener.start(false)
-
- @allow_stop.wait if @allow_stop
+ interactor.start
end
# Stop Guard listening to file changes
#
def stop
listener.stop
- interactor.stop if interactor
runner.run(:stop)
::Guard::UI.info 'Bye bye...', :reset => true
-
- @allow_stop.signal if @allow_stop
end
# Reload Guardfile and all Guard plugins currently enabled.
@@ -185,15 +174,13 @@ def stop
# @param [Hash] scopes hash with a Guard plugin or a group scope
#
def reload(scopes = {})
- within_preserved_state do
- ::Guard::UI.clear(:force => true)
- ::Guard::UI.action_with_scopes('Reload', scopes)
+ ::Guard::UI.clear(:force => true)
+ ::Guard::UI.action_with_scopes('Reload', scopes)
- if scopes.empty?
- ::Guard::Dsl.reevaluate_guardfile
- else
- runner.run(:reload, scopes)
- end
+ if scopes.empty?
+ ::Guard::Dsl.reevaluate_guardfile
+ else
+ runner.run(:reload, scopes)
end
end
@@ -202,11 +189,9 @@ def reload(scopes = {})
# @param [Hash] scopes hash with a Guard plugin or a group scope
#
def run_all(scopes = {})
- within_preserved_state do
- ::Guard::UI.clear(:force => true)
- ::Guard::UI.action_with_scopes('Run', scopes)
- runner.run(:run_all, scopes)
- end
+ ::Guard::UI.clear(:force => true)
+ ::Guard::UI.action_with_scopes('Run', scopes)
+ runner.run(:run_all, scopes)
end
# Pause Guard listening to file changes.
@@ -320,25 +305,6 @@ def add_group(name, options = {})
group
end
- # Runs a block where the interactor is
- # blocked and execution is synchronized
- # to avoid state inconsistency.
- #
- # @yield the block to run
- #
- def within_preserved_state
- lock.synchronize do
- begin
- interactor.stop if interactor
- @result = yield
- rescue Interrupt
- end
-
- interactor.start if interactor
- end
- @result
- end
-
# Tries to load the Guard plugin main class. This transforms the supplied Guard plugin
# name into a class name:
#
View
4 lib/guard/commands/change.rb
@@ -20,9 +20,7 @@ def process(*entries)
scopes, rest = ::Guard::Interactor.convert_scope(entries)
if rest.length != 0
- ::Guard.within_preserved_state do
- ::Guard.runner.run_on_changes(rest, [], [])
- end
+ ::Guard.runner.run_on_changes(rest, [], [])
else
output.puts 'Please specify a file.'
end
View
57 lib/guard/interactor.rb
@@ -20,6 +20,8 @@ class Interactor
GUARD_RC = '~/.guardrc'
HISTORY_FILE = '~/.guard_history'
+ attr_accessor :pry_instance
+
# Initialize the interactor. This configures
# Pry and creates some custom commands and aliases
# for Guard.
@@ -44,8 +46,9 @@ def initialize
# Loads the `~/.guardrc` file when pry has started.
#
def load_guard_rc
- Pry.config.hooks.add_hook :when_started, :load_guard_rc do
+ Pry.config.hooks.add_hook :when_started, :load_guard_rc do |target, options, pry_instance|
load GUARD_RC if File.exist? File.expand_path GUARD_RC
+ @pry_instance = pry_instance
end
end
@@ -127,52 +130,20 @@ def configure_prompt
def start
return if ENV['GUARD_ENV'] == 'test'
- store_terminal_settings if stty_exists?
-
- if !@thread || !@thread.alive?
- ::Guard::UI.debug 'Start interactor'
-
- @thread = Thread.new do
- Pry.start
- ::Guard.stop
- exit
- end
- end
+ ::Guard::UI.debug 'Start interactor'
+ Pry.start
+ ::Guard.stop
+ exit
end
- # Kill interactor thread if not current
+ # Redraw the current line. This outputs
+ # the prompt at the moment and misses the current
+ # line input.
#
- def stop
- return if !@thread || ENV['GUARD_ENV'] == 'test'
-
- unless Thread.current == @thread
- ::Guard::UI.debug 'Stop interactor'
- @thread.kill
+ def redraw
+ if pry_instance
+ print "\n#{ pry_instance.select_prompt('', pry_instance.current_context) }"
end
-
- restore_terminal_settings if stty_exists?
- end
-
- # Detects whether or not the stty command exists
- # on the user machine.
- #
- # @return [Boolean] the status of stty
- #
- def stty_exists?
- @stty_exists ||= system('hash', 'stty')
- end
-
- # Stores the terminal settings so we can resore them
- # when stopping.
- #
- def store_terminal_settings
- @stty_save = `stty -g 2>/dev/null`.chomp
- end
-
- # Restore terminal settings
- #
- def restore_terminal_settings
- system("stty #{ @stty_save } 2>/dev/null") if @stty_save
end
# Converts and validates a plain text scope
View
7 lib/guard/runner.rb
@@ -50,8 +50,8 @@ def deprecation_warning
#
def run(task, scopes = {})
Lumberjack.unit_of_work do
- scoped_guards(scopes) do |guard|
- run_supervised_task(guard, task)
+ scoped_guards(scopes) do |guard|
+ run_supervised_task(guard, task)
end
end
end
@@ -69,6 +69,7 @@ def run(task, scopes = {})
#
def run_on_changes(modified, added, removed)
::Guard::UI.clearable
+
scoped_guards do |guard|
modified_paths = ::Guard::Watcher.match_files(guard, modified)
added_paths = ::Guard::Watcher.match_files(guard, added)
@@ -80,6 +81,8 @@ def run_on_changes(modified, added, removed)
run_first_task_found(guard, ADDITION_TASKS, added_paths) unless added_paths.empty?
run_first_task_found(guard, REMOVAL_TASKS, removed_paths) unless removed_paths.empty?
end
+
+ ::Guard.interactor.redraw if ::Guard.interactor
end
# Run a Guard plugin task, but remove the Guard plugin when his work leads to a system failure.
View
2  spec/guard/notifiers/terminal_notifier_spec.rb
@@ -4,7 +4,7 @@
let(:fake_terminal_notifier) do
Module.new do
- def self.show(options) end
+ def self.execute(options) end
end
end
View
7 spec/guard/notifiers/terminal_title_spec.rb
@@ -2,15 +2,8 @@
describe Guard::Notifier::TerminalTitle do
- let(:fake_terminal_title) do
- Class.new do
- def self.show(options) end
- end
- end
-
before do
subject.stub!(:puts)
- stub_const 'TerminalTitle', fake_terminal_title
end
describe '.available?' do
View
10 spec/guard/notifiers/tmux_spec.rb
@@ -2,16 +2,6 @@
describe Guard::Notifier::Tmux do
- let(:fake_tmux) do
- Class.new do
- def self.show(options) end
- end
- end
-
- before do
- stub_const 'Tmux', fake_tmux
- end
-
describe '.available?' do
context "when the TMUX environment variable is set" do
before :each do
View
24 spec/guard_spec.rb
@@ -488,8 +488,8 @@ class Guard::FooBaz < Guard::Guard; end
before do
::Guard.stub(:setup)
::Guard.stub(:listener => mock('listener', :start => true))
+ ::Guard.stub(:interactor => mock('listener', :start => true))
::Guard.stub(:runner => mock('runner', :run => true))
- ::Guard.stub(:within_preserved_state).and_yield
::Guard.instance_variable_set('@allow_stop', mock('turnstile', :wait => true, :signal => true))
end
@@ -609,28 +609,6 @@ class Guard::FooBaz < Guard::Guard; end
end
end
- describe '.within_preserved_state' do
- subject { ::Guard.setup }
- before { subject.interactor = stub('interactor').as_null_object }
-
- it 'disables the interactor before running the block and then re-enables it when done' do
- subject.interactor.should_receive(:stop)
- subject.interactor.should_receive(:start)
- subject.within_preserved_state &Proc.new {}
- end
-
- it 'disallows running the block concurrently to avoid inconsistent states' do
- subject.lock.should_receive(:synchronize)
- subject.within_preserved_state &Proc.new {}
- end
-
- it 'runs the passed block' do
- @called = false
- subject.within_preserved_state { @called = true }
- @called.should be_true
- end
- end
-
describe ".get_guard_class" do
after do
[:Classname, :DashedClassName, :UnderscoreClassName, :VSpec, :Inline].each do |const|
Something went wrong with that request. Please try again.