Re: [PATCH] consequence: hook

[Thorsten Wißmann <edu _at_ thorsten _minus_ wissmann _dot_ de>]

Hi Tylo,

On Thu, Dec 13, 2012 at 07:40:08PM -0800, Tyler Thomas Hart wrote:
> This simple little patch adds a rule-consequence that emits a custom hook
> when the target window appears.

The patch looks good, except some points I like to fix for the final
patch.

The main thing is, that the rule should not directly emit the specified
hook but a hook called "rule" with a customizable parameter.

> ++hook+::
> +    emits the custom hook 'VALUE' to all idling herbstclients.
> +

IMO, it should be stated clearly:

  emits the custom hook +rule+ 'VALUE' when this rule is triggered. This
  consequence can be used multiple times, which will cause a hook to be
  emitted for each occurrence of a hook consequence.

The hook consequence behaves different to the other consequences where
only the last consequence takes effect. (e.g. only the last tag
consequence finally says where the window goes to).

> +void consequence_hook(HSConsequence* cons, HSClient* client,
> +                            HSClientChanges* changes) {
> +    char* hook_str[] = {cons->value.str, NULL};
> +    hook_emit(2, hook_str);
> +}

The last entry of the list needn't (and shouldn't) be NULL, that's only
for hook_emit_list. Of course we also have to add the "rule" parameter.
So the first possible solution is:

  char* hook_str[] = { "rule", cons->value.str };
  hook_emit(LENGTH(hook_str), hook_str);

And the second possibility is:

  hook_emit_list("rule", cons->value.str, NULL);

Please resend the patch or tell me via irc that I am allowed to
git-commit-amend it to your patch.

Happy holidays,
Thorsten