Re: [PATCH] New client/monitor attributes

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

Hi Florian,

On Thu, Jun 20, 2013 at 10:38:07PM +0200, Florian Bruhin wrote:
> The pid attribute will be -1 if the client didn't set it, I'm not sure
> if this is the best solution, or if I should handle this attribute as
> a custom string instead, and return an empty string if there is no pid
> set. Opinions?

I'd prefer the -1 option (and document it in the manpage)

You also could add some description of the attributes to the manpage
(there's already a OBJECTS section[1] describing the attributes)

>      HSAttribute attributes[] = {
> +        ATTRIBUTE_STRING(   "tag",          client->tag->display_name, ATTR_READ_ONLY),

This looks wrong, because ATTRIBUTE_STRING remembers the address of the
string you gave it. So in your case this attribute will always give the
name of the tag the client initially was on (and it should crash as soon
as the initial tag is removed). I recommend using a custom string
attribute here. (ATTRIBUTE_STRING only works for strings whose address
is constant during the livetime of the object or client in this case).

> diff --git a/src/monitor.c b/src/monitor.c
> @@ -409,6 +409,7 @@ HSMonitor* add_monitor(XRectangle rect, HSTag* tag, char* name) {
>      HSAttribute attributes[] = {
>          ATTRIBUTE_CUSTOM_INT("index",   monitor_attr_index,ATTR_READ_ONLY  ),
> +        ATTRIBUTE_STRING(   "tag",      m->tag->display_name,ATTR_READ_ONLY  ),

It's the same issue here.

Regards,
Thorsten

[1] http://herbstluftwm.org/herbstluftwm.html#OBJECTS



Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev