As a question for the reviewers: I wonder if there should be additional tests for this. While the existing tests had to be adapted to reflect the code change, I'm not (entirely) sure whether this is sufficient (given that the bug slipped through with the tests). Even though

> CUCUMBER_COLORS='passed=red,bold' cucumber --help
[…]/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/cucumber-8.0.0/lib/cucumber/formatter/ansicolor.rb:95:in `<module:ANSIColor>': undefined method `apply_custom_colors' for Cucumber::Formatter::ANSIColor:Module (NoMethodError)

      apply_custom_colors(ENV['CUCUMBER_COLORS']) if ENV['CUCUMBER_COLORS']
      ^^^^^^^^^^^^^^^^^^^

would have revealed the bug, I'm not sure this would be a good fit for another scenario in a feature file.

Thanks for the feedback.

I had to change the definition of apply_custom_colors to be a class method, in order to be able to call it right after the definition. That's because the context is the module.

Here's an example:

module Example
  def for_instances(arg)
    p arg
  end

  def self.for_class(arg)
    p arg
  end

  begin
    for_class('Calling the class method is OK')
  rescue NoMethodError => e
    p e
  end
  
  begin
    for_instances('The instance method is not available in the class')
  rescue NoMethodError => e
    puts "That didn't work"
    puts "Message:"
    puts "*" * 40
    puts e.message
    puts "*" * 40
  end
end

Running it yields:

"Calling the class method is OK"
That didn't work
Message:
****************************************
undefined method `for_instances' for Example:Module

    for_instances('The instance method is not available in the class')
    ^^^^^^^^^^^^^
Did you mean?  for_class
****************************************

In other words: self inside a class (or module) definition is the class/module itself. To be able to call a method within the class/module definition it needs to be a class method (similar to attr_reader etc.).

I hesitate to provide an instance method, because the method is not meant to be called on an instance (but provided it anyway 🙂), so we can discuss it.

Thanks for the update.

@luke-hill what is your opinion on this?

No worries, I'll just need to revert 1560c6e, right?

No worries, I'll just need to revert 1560c6e, right?

Yes, in addition to the last comments from @luke-hill

OK, as far as I can see we could now merge this PR, no?

This needs releasing. The bug with CUCUMBER_COLORS is in the current release 8.0.0 7 months after it has been fixed in cucumber:main. Can we get a point release out to address this?