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

Restore tmux options immediately after each notification #396

Closed
wants to merge 5 commits into from

Conversation

Projects
None yet
3 participants
@lackac
Copy link

commented Jan 25, 2013

I use tmux messages for all kinds of things. When using the tmux notifier in Guard it messes up the settings for these messages. This change restores the options immediately after the notification disappears so that other messages display with the defaults.

system("#{ DEFAULTS[:client] } set display-time #{ display_time * 1000 }")
system("#{ DEFAULTS[:client] } set message-fg #{ message_color }")
system("#{ DEFAULTS[:client] } set message-bg #{ color }")
fork do

This comment has been minimized.

Copy link
@netzpirat

netzpirat Jan 25, 2013

Contributor

fork cannot be used in Guard, because it's not supported on the JVM and breaks JRuby support.

@netzpirat

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2013

We cannot merge this currently, because it's not working on all Rubies Guard supports. Are you planning to change your implementation or should this PR be closed?

@netzpirat

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2013

Thanks for the update. Unfortunately the specs for the Tmux notifier are failing and need to be updated according to the changes in the implementation.

@lackac

This comment has been minimized.

Copy link
Author

commented Feb 7, 2013

Yes, sorry, I was in a hurry and didn't check the specs. Now they should be all fixed, but I only tested it on 1.9.3.

@netzpirat

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2013

Thanks for the update, but I still can't merge this, since it fails on Travis. Can you have a look?

@lackac

This comment has been minimized.

Copy link
Author

commented Feb 18, 2013

This failure is puzzling. I could only reproduce the failure once on Ruby 1.8.7, but it disappeared as soon as I started debugging. After that I could not reproduce again. Not even after removing all debugging messages an restoring commit state.

@netzpirat

This comment has been minimized.

Copy link
Contributor

commented Feb 18, 2013

I think this may be related to Thread.new because thread scheduling is not guaranteed nor in strict order. You can workaround this by just make it run inside the current thread with something like:

before do
  Thread.stub(:new).and_yield
end

in the spec suite (I didn't test the above snippet, but you get the idea).

@netzpirat

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2013

I hope you understand that I cannot merge a pull request without working specs on all supported platforms and I'm going to close this PR now because of inactivity. If you decide to fix the specs, please reopen the pull request and I'll happily review it. I'm fine when it only works on 1.9, because we anyway drop 1.8.7 support very soon. Thanks a lot.

@netzpirat netzpirat closed this Mar 5, 2013

@lackac

This comment has been minimized.

Copy link
Author

commented Mar 5, 2013

Of course. I haven't had time to get to the bottom of this. I'll reopen if
I can deal with this in the future.

@coveralls

This comment has been minimized.

Copy link

commented Oct 18, 2017

Coverage Status

Changes Unknown when pulling e09f22d on lackac:restore-tmux-options into ** on guard:master**.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.