Re: [PATCH] consequence: hook

[Tyler Thomas Hart <htyler _at_ pdx _dot_ edu>]

Thorsten,

I made your suggested changes.

On Mon, Dec 24, 2012 at 04:18:18PM +0100, Thorsten Wi?mann wrote:
> The main thing is, that the rule should not directly emit the specified
> hook but a hook called "rule" with a customizable parameter.

Agreed. I included a colon eg. 'rule:', feel free to change it back as you see fit.

> 
> > ++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.

Alright, I see how this is different from other consequences. I copy-pasted this in.

> > +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);

I went with the first choice. As with my suggestion on irc, avoiding the call to g_new and the va loop in emit_list() seems more efficient.

I've attached the amended patch. If you want to make any other changes, feel free to make them.

Happy Holidays as well,
Tylo

>From f2ddc452ee5f4454670572969c473d7265e0449b Mon Sep 17 00:00:00 2001
From: Tyler Thomas Hart <htyler _at_ pdx _dot_ edu>
Date: Mon, 24 Dec 2012 21:04:58 -0800
Subject: [PATCH] Add consequence: hook

---
 NEWS                 | 1 +
 doc/herbstluftwm.txt | 5 +++++
 src/rules.c          | 8 ++++++++
 3 files changed, 14 insertions(+)

diff --git a/NEWS b/NEWS
index 64fef3a..f759336 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,7 @@ Changes:
     * add error messages for herbstclient
     * new commands: focus_edge, shift_edge
     * new command: shift_to_monitor
+    * new consequence: hook
 
 Release: 0.4.1 on 2012-08-30
 ----------------------------
diff --git a/doc/herbstluftwm.txt b/doc/herbstluftwm.txt
index 2de02d7..ac68617 100644
--- a/doc/herbstluftwm.txt
+++ b/doc/herbstluftwm.txt
@@ -936,6 +936,11 @@ Each 'CONSEQUENCE' consists of a 'NAME'='VALUE' pair. Valid 'NAMES' are:
     sets the fullscreen flag of the client. 'VALUE' can be *on*, *off* or
     *toggle*.
 
++hook+::
+    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.
+
 A rule's behaviour can be configured by some special 'FLAGS':
 
     * +not+: negates the next 'CONDITION'.
diff --git a/src/rules.c b/src/rules.c
index 95a0ac5..11a99c3 100644
--- a/src/rules.c
+++ b/src/rules.c
@@ -9,6 +9,7 @@
 #include "ewmh.h"
 #include "clientlist.h"
 #include "ipc-protocol.h"
+#include "hook.h"
 
 #include <glib.h>
 #include "glib-backports.h"
@@ -60,6 +61,7 @@ DECLARE_CONSEQUENCE(consequence_fullscreen);
 DECLARE_CONSEQUENCE(consequence_switchtag);
 DECLARE_CONSEQUENCE(consequence_ewmhrequests);
 DECLARE_CONSEQUENCE(consequence_ewmhnotify);
+DECLARE_CONSEQUENCE(consequence_hook);
 
 /// GLOBALS ///
 
@@ -86,6 +88,7 @@ static HSConsequenceType g_consequence_types[] = {
     { "fullscreen",     consequence_fullscreen      },
     { "ewmhrequests",   consequence_ewmhrequests    },
     { "ewmhnotify",     consequence_ewmhnotify      },
+    { "hook",           consequence_hook            },
 };
 
 GQueue g_rules = G_QUEUE_INIT; // a list of HSRule* elements
@@ -644,3 +647,8 @@ void consequence_ewmhnotify(HSConsequence* cons, HSClient* client,
     client->ewmhnotify = string_to_bool(cons->value.str, client->ewmhnotify);
 }
 
+void consequence_hook(HSConsequence* cons, HSClient* client,
+                            HSClientChanges* changes) {
+    char* hook_str[] = { "rule:" , cons->value.str };
+    hook_emit(LENGTH(hook_str), hook_str);
+}
-- 
1.8.0.2