Fix tools arrays.
This will close #459 and #774.
I think this issue need to be merged at ceedling_0_32 branch
@mvandervoord Any idea on fixing tools arrays for command hooks? Maybe they can be supported only for command hooks tools and not for the normal ones (test compiler, test linker, etc.).
@mvandervoord @mkarlesky I want to resume my work on this. My initial plan was to fix tools array handling on setup and validation, but that will leave not uniform behavior between normal tools (preprocessor, compiler, linker, etc.) and command hooks plugin tools (pre_compile_execute, pre_link_execute, etc.). The former ones will not allow an Array of tools (I can not think of a use case for having an array of compilers, preprocessors, etc. Or may there be any?) and the last ones will accept both a tool Hash or an Array of tool Hashes, that is why I proposed here to take out command hooks plugin tools from tools section and move them to a dedicated command hooks section. Are you OK with the above behavior? Let me know your thoughts.
@deltalejo @mvandervoord It seems pretty clear that folks would love an array of tools for command hooks. I am in support. The current structure of the :tools configuration section does not support this and probably should not support this.
Once upon a time, it only made sense to place tool definitions in :tools because of Ceedling's simplistic validation scheme. That's no longer a limit, though. Tool validation has been broken out into its own utility object, ToolValidator, that many of the rewritten plugins now depend on. Specifically, if a particular feature in the plugin's configuration is enabled, it validates the corresponding tool definition (only internally defined to the plugin to date). Tools should always be validated at startup to avoid frustrating errors on the backside of a build, but tools should only be validated for features that depend on them. The plugin's configuration handling/validation and any tool validation happens in the plugin’s setup(). The Beep plugin is the simplest that demonstrates this.
Further, as Ceedling has been evolving for the next release a pattern has emerged for plugins: (1) enable the plugin in :plugins and (2) complement this with a dedicated top-level configuration section corresponding to that plugin (again, the Beep plugin demonstrates this simply).
So, I think Command Hooks should get these same treatments. This will bring it into alignment with other plugins and allow for multiple tools per hook.
- If Command Hooks is enabled, all its configuration should be in a top-level configuration block
:command_hooks. - The second tier of configuration beneath
:command_hooksshould be the command hook keys, e.g.:command_hooks↳:pre_compile_execute. - Instead of organizing command hook tools beneath
:toolsthey should be in an array beneath any hook key in the plugin config block. - At plugin startup, the plugin should validate that all hook keys are from among the recognized list, raising an exception for any that are not (these would be mistakes with silent failures otherwise). The plugin should also use
ToolValidatorto validate each tool associated with a hook. All this would happen from withinsetup(). If command hooks are enabled and tools are specified for any hooks, we know a build will rely on them and, therefore, the plugin should forcibly validate them from withinsetup(). - The rest of the plugin's behavior would remain unchanged apart from necessary refactoring to connect existing code with the new configuration above.
:plugins:
:enabled:
- command_hooks
:command_hooks:
:pre_compile_execute:
- <tool definition>
- <tool definition>
@mkarlesky @mvandervoord This is ready for review now.
Thank you for this, @deltalejo! This is good work. I'll get it merged possibly as soon as today.