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

Cannot set the watch_all_modifications option #173

Closed
sutherland opened this issue Oct 28, 2011 · 3 comments
Closed

Cannot set the watch_all_modifications option #173

sutherland opened this issue Oct 28, 2011 · 3 comments

Comments

@sutherland
Copy link

In Guard::Listener, the initialize() method is attempting to set default options:

@watch_all_modifications = options.fetch(:watch_all_modifications, false)

The options hash is an instance of Thor::CoreExt::HashWithIndifferentAccess. This hash extension doesn't seem to support indifferent access for fetch(). Because the stored key is actually a string, this will always default to false.

This might be resolved with:

@watch_all_modifications = options[:watch_all_modifications] || false
@rymai
Copy link
Member

rymai commented Oct 28, 2011

Thanks, could you add a failing test (and even fix the issue) and submit a pull-request against the dev branch?

@netzpirat
Copy link
Contributor

Thanks for analyzing this issue and reporting it. Your suggestion

@watch_all_modifications = options[:watch_all_modifications] || false

works only in the case where the default value is false.

So what can we do?

One solution would be to just use the Thor default value and completely remove any default value in the listener itself. This seems fine, but it would lead to a long rat tail to update all methods and its docs to reflect that Thor::CoreExt::HashWithIndifferentAccess is passed instead of a Hash, so no options = {} anymore. In addition the specs must be updated to handle the new parameter type. I mainly decided against this solution because it would couple the listener to Thor, which makes it harder for other projects that are using Guard's listener framework.

In the end I simply changed the options.fetch key to a string, because Thor internally converts every key to a String. This is solution at not cost for Guard and also simple for projects using the listeners to ensure the option keys are string.

@netzpirat
Copy link
Contributor

After thinking a second time on this, I simply convert the options keys to symbols before evaluation them. This ensures the projects that are using the Listener framework don't have to change anything.

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

No branches or pull requests

3 participants