Re: [PATCH] Double Window Borders

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

Hi Bastien,

was that feature initially requested by you? There are some formal
things I don't like about the current patch, the code itself works as
expected.

On Sat, Jul 21, 2012 at 08:30:43PM +0200, Bastien Dejean wrote:
> +    * new setting: double_window_borders
> +    * new setting: window_border_inner_color

It is confusing that the feature is enabled with the "double window
borders" but controlled via the window_border_inner_color. Maybe the
"double" vs. "inner" problem is solved simply by adding a sentence like
"The inner color is controlled via the window_border_inner_color
setting."

Another thing that causes confusion is the usage of the plural here.
It's "window_border_width" and not "window_borders_width", so it should
be "double_window_border" too (For every window it controls if there's a
double window border or not).

So we maybe also should rename the smart_*_surroundings to
smart_*_surrounding. (But maybe surroundings is OK here; if we rename
that, that would be another commit of course)
 
>  void window_unfocus(Window window) {
> -    XSetWindowBorder(g_display, window, g_window_border_normal_color);
> +    if (*g_double_window_borders)
> +        set_window_double_border(window, g_window_border_inner_color, g_window_border_normal_color);
> +    else
> +        XSetWindowBorder(g_display, window, g_window_border_normal_color);

You copied this if-else-thing very very often, so you should move that
part to a helper function

    void window_update_border(Window window, unsigned long color);

which performs the if-check. Please also use brackets { } after the if
and the else statement. (I've forgot to add that rule to the HACKING
file)

> +    unsigned int g_depth = DefaultDepth(g_display, DefaultScreen(g_display));

g_depth neither is global nor a glib Function, so it must not have a g_
prefix.


Thorsten

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/