Issue #572: make kernel warn configurable

[ColinDKelley]

• Makes the usage of Kernel#warn configurable through Listen.adapter_warn_behavior =.
• Adds this documentation to the README on that feature:

Adapter Warnings

If listen is having trouble with the underlying adapter, it will display warnings with Kernel#warn by default,
which in turn writes to STDERR.
Sometimes this is not desirable, for example in an environment where STDERR is ignored.
For these reasons, the behavior can be configured using Listen.adapter_warn_behavior =:

Listen.adapter_warn_behavior = :warn   # default (true means the same)
Listen.adapter_warn_behavior = :log    # send to logger.warn
Listen.adapter_warn_behavior = :silent # suppress all adapter warnings (nil or false mean the same)

Also there are some cases where specific warnings are not helpful.
For example, if you are using the polling adapter--and expect to--you can suppress the warning about it
by providing a callable object like a lambda or proc that determines the behavior based on the message:

Listen.adapter_warn_behavior = ->(message) do
  case message
  when /Listen will be polling for changes/
    :silent
  when /directory is already being watched/
    :log
  else
    :warn
  end
end

[houndci-bot]

Some files could not be reviewed due to errors:

Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` paramete...

Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` parameter (in .rubocop.yml).
Supported versions: 2.4, 2.5, 2.6, 2.7, 2.8

[AlexB52]

Question: Indirect use of Listen

Having access to Listen gem directly allows configuration but what about people using Listen indirectly through guard for example. I'm not super familiar with guard, would people need to be able to silence these or would it be the gem using Listen to make a port for Listen configuration.

[ColinDKelley]

@AlexB52, you asked over in the issue:

It seems that rails/webpacker#1990 will be able to configure listen in a rails initializer too right?

Yes, exactly. I've read through a couple of those issues and they were able to be closed without needing this... so I get a little worried that the feature here that would allow them to be silenced might simply mask a more general issue that still wasn't fixed. But I think that's an ok risk to take.

[ColinDKelley]

Having access to Listen gem directly allows configuration but what about people using Listen indirectly through guard for example. I'm not super familiar with guard, would people need to be able to silence these or would it be the gem using Listen to make a port for Listen configuration.

I'm not super-familiar with guard either. If you're using Listen in the same process, then you can simply configure it there, through the Listen.adapter_warn_behavior = interface.

But if it's a command-line interface, that interface would need to expose this Listen configuration as an option somewhere. One hacky way to do that would be as an environment variable, I suppose. Listen already has a few of those already.

[AlexB52]

But if it's a command-line interface, that interface would need to expose this Listen configuration as an option somewhere. One hacky way to do that would be as an environment variable, I suppose. Listen already has a few of those already.

Makes sense, thanks.

Yes, exactly. I've read through a couple of those issues and they were able to be closed without needing this... so I get a little worried that the feature here that would allow them to be silenced might simply mask a more general issue that still wasn't fixed. But I think that's an ok risk to take.

This is how I currently silence these warnings in retest by monkey patching the SymlinkDetector#_fail which seems not ideal. https://github.com/AlexB52/retest/blob/main/lib/config/listen.rb.
Looking at it I can't remember why I used silence_warnings method with a block AND override #_fail.

I'll definitely try to migrate over the :log or :slient adapter.
I was concerned about ruby support. Retest currently supports Ruby 2.5, and uses Listen 3.2. I see that Listen supports Ruby 2.4 which is nice and should be straight forward.

[houndci-bot]

Some files could not be reviewed due to errors:

Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` paramete...

Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` parameter (in .rubocop.yml).
Supported versions: 2.4, 2.5, 2.6, 2.7, 2.8

[ColinDKelley]

@AlexB52 For separate processes like guard, what do you think of this to support an environment variable LISTEN_GEM_ADAPTER_WARN_BEHAVIOR =? That doesn't have the power of a callable object that can look at the message, but it covers the static cases of warn, log and silent.

Do you think that's sufficient for now?

Invoca@cb8c9c5

[AlexB52]

@ColinDKelley

The environment variable is a nice addition and the change provides plenty of options for people to silence logger warnings.

That said I see a few issues unless I'm misunderstanding something.

TL;DR

1. The Kernel warnings are still not silenced or piped to the logs.
2. Logger warnings can now be piped to STDERR with the :warn adapter which wasn't possible before

1. What about Kernel.warn and Kernel#warn?

Unless mistaken, the change still uses Kernel.warn and Kernel#warn on these files which are the ones I'd like to mute when I raised #572. Looking at my previous PR #574, I would expect more spec and lib files to change. Example: symlink_detector is the warning I would love to get rid of nicely. There are so many of them.

• https://github.com/guard/listen/blob/master/lib/listen/adapter.rb#L39
• https://github.com/guard/listen/blob/master/lib/listen/adapter/bsd.rb#L37
• https://github.com/guard/listen/blob/master/lib/listen/adapter/darwin.rb#L33
• https://github.com/guard/listen/blob/master/lib/listen/adapter/windows.rb#L23
• https://github.com/guard/listen/blob/master/lib/listen/record/symlink_detector.rb#L36

It looks like the change still needs to replace Kernel.warn and warn by Logger.adapter_warn across these 5 files.

2. More warnings piped to STDERR

In case we set the adapter to :warn, error messages from Logger.warn will now be printed to STDERR (which weren't before). Is that a regression issue?

If we're using Logger.adapter_warn(message) instead of Kernel.warn then there are no regression issues with existing Logger.warn messages.

[houndci-bot]

Some files could not be reviewed due to errors:

Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` paramete...

Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` parameter (in .rubocop.yml).
Supported versions: 2.4, 2.5, 2.6, 2.7, 2.8

[houndci-bot]

Some files could not be reviewed due to errors:

Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` paramete...

Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` parameter (in .rubocop.yml).
Supported versions: 2.4, 2.5, 2.6, 2.7, 2.8

[ColinDKelley]

@AlexB52 I just pushed the 5 changes to call adapter_warn with specs here.

Notice that I left a SymlinkDetector#warn method--with a deprecation comment--since you mentioned having monkey patched that as a work-around. By stopping off there, I think we can avoid breaking any code like that.

[AlexB52]

@ColinDKelley

Sounds good. It's a good point that people could have monkey-patched the #warn method.
I tested locally with your branch on a local repository that I know shows the symlink warnings

configuration	expectation	result
LISTEN_GEM_ADAPTER_WARN_BEHAVIOR=warn	shows	ok
LISTEN_GEM_ADAPTER_WARN_BEHAVIOR=silent	muted	ok
LISTEN_GEM_ADAPTER_WARN_BEHAVIOR=log	muted	ok
Listen.adapter_warn_behavior = ->(message) { :warn }	shows	ok
Listen.adapter_warn_behavior = ->(message) { :silent }	muted	ok
Listen.adapter_warn_behavior = ->(message) { :log }	muted	ok
Listen.adapter_warn_behavior = :warn	shows	ok
Listen.adapter_warn_behavior = :silent	muted	ok
Listen.adapter_warn_behavior = :log	muted	ok

Note: I did not try to test whether the warnings were pushed to a log file but only that logs were muted with :log configuration.

[ColinDKelley]

That looks great, @AlexB52. Thanks for the thorough testing.

I'm good with merging it now. Sound good?

[AlexB52]

@ColinDKelley yup sounds good