@carlos-granados I'm not that familiar with profile formatters feature.
Could you explain me the different way and options we have to use them?

The documentation is not great about them.

Do we have "verbose", "paths" and "snippets" options for all of them?

@loic425 The options available for each formatter are:

Pretty:

• 'timer',
• 'expand',
• 'paths',
• 'multiline',
  (not really sure what the expand and multiline options do, never used them)

Progress:

• 'timer'

Junit:

• 'timer'

(other formatters out there may define other options). You can see these definitions in these classes:
PrettyFormatterFactory, ProgressFormatterFactory, JUnitFormatterFactory

Please take into account that apart from these options, these formatters can also be enabled or disabled

@loic425 it's essentially very flexible - each formatter can define its own options.

There is a work-in-progress cookbook for adding a custom formatter at Behat/docs#149 - https://behat--149.org.readthedocs.build/en/149/cookbooks/custom_formatter.html#create-the-extension which adds a bit more detail on how formatters work / can be configured.

In addition to the list @carlos-granados provided, I think every formatter array can also include output_path, output_verbosity, output_decorate, output_styles options (though how much effect they have will vary by formatter). These are processed by the OutputManager before any remaining formatter-specific parameters are passed through to the formatter implementation.

For the pretty formatter, I believe:

• expand: true causes it to print a full scenario with all steps for each example of an Outline: ... instead of the default which looks more like the feature file with the steps printed only once and the results shown in / between rows of the table (like this https://github.com/Behat/Behat/blob/master/features/pretty_format.feature#L116).
• multiline:false means it renders ... instead of any PyStringNode / TableNode or similar arguments - by default they're rendered in full https://github.com/Behat/Behat/blob/master/src/Behat/Behat/Output/Node/Printer/Pretty/PrettyStepPrinter.php#L129

I'd suggest perhaps instead of withFormatters(...) we have:

• individual withFormatter() to keep it neat if the object has chained helpers
• disableFormatter(string $name) - which translates to ['formatters'][$name] = false

I am fairly sure that enabling a formatter in YAML (e.g. providing progress: true) just means configure it with the default arguments (rather than e.g. enable it but merge config from the default profile) - but I'm not 100% on that. If I'm right then I don't think we need a separate enableFormatter method.

And then probably provide an implementation of FormatterConfigInterface for each of our standard formatters with the specific options they take?

I might be tempted to implement these as named constructor arguments with the defaults matching the ones that are defined in the XXXFormatterFactory. This would avoid too much verbosity and mean the defaults are obvious to the user rather than buried away.

If we did that I'd suggest then having the XXXFormatterFactory reference those class defaults instead of redefining them, to keep them consistent over time.

So we'd have something like:

class PrettyFormatter implements FormatterConfigInterface {

   public function __construct(
       bool $timer = true,
       bool $expand = false,
       bool $paths = true,
       bool $multiline = true
   ) {/*...*/}
  
  public function withOutputPath(string $path) {
    // Probably need withOutputPath, withOutputVerbosity, etc to be chained helpers as we don't want to specify values here by default
   // We could perhaps implement these on a base `FormatterConfig` object that individual formatters extend from, as
   // third-party formatters would also need to support these...
  }
}

And then this in PrettyFormatterFactory:

 $definition = new Definition('Behat\Testwork\Output\NodeEventListeningFormatter', array(
            'pretty',
            'Prints the feature as is.',
            array(
                'timer'     => true,
                'expand'    => false,
                'paths'     => true,
                'multiline' => true,
            ),

Would become something more like:

 $definition = new Definition('Behat\Testwork\Output\NodeEventListeningFormatter', array(
            PrettyFormatter::NAME, // probably need this as a const to make it easy to pass to ->disableFormatter()
            'Prints the feature as is.',
            (new PrettyFormatter())->toArray()

What do you and @carlos-granados think?

@acoulton I didn't see your previous comment before this new commit.

I like your idea with "disableFormatter"

@acoulton I've updated the PR with your suggested changes.

This is looking good to me.

I think I'd still ideally like to see the FormatterFactory classes now taking their defaults from these classes, to avoid the risk a parameter gets added / changed in one place but not the other.

I've added a commit with that proposal (edit two commits)

One thing I've realised having now had a chance to test this is that with YAML, it actually is possible to do this:

default:
  formatters:
    pretty:
      expand: true
      multiline: false

ci:
  formatters:
    pretty:
      multiline: true
      # This block is merged with the block from the `default` profile above
      # So because expand is not specified here, it will inherit the customised `true` value,
      # not the formatter default of `false`

This is not possible with the current PHP implementation.

Personally I think that's OK. I think it's much clearer to specify all the non-default formatter options in each separate profile, rather than relying on them being deep-merged implicitly. Especially because most other parts of the config are overwritten rather than merged.

If people really want to avoid typing the parameters twice, since they're in PHP they have quite a lot of options to use variables or similar to explicitly share values between different profiles in their config file.

I also think using constructor parameters for these is much easier for users to read and to discover the available / default options, and the benefit of that outweighs the loss.

But just thought worth highlighting in case others think the internal deep-merge is an important feature.

If you have time to look at adding a short documentation for the parameters as #1558 (comment) that would be great. If not I think we can merge this and I can pick that up

Yes, I'll work on it on monday.

@carlos-granados I think everything is ok now :)

@loic425 this is looking good to me. Would you be able to rebase onto latest master and potentially squash the commits? Thanks :)

@loic425 this is looking good to me. Would you be able to rebase onto latest master and potentially squash the commits? Thanks :)

Yes, squashed & rebased.