On Wed, 29 Feb 2012 17:51:07 +0800, Michael Stapelberg <michael@xxxxxxxx>
wrote:
Hi darkraven,
Excerpts from DR's message of 2012-02-27 16:09:07 +0100:
Here is a patch which enables i3 to use 32bit visual only when needed.Thanks for your patch. It removes a commandline switch, which is a good thing,
and it seems to work just fine on my system.
Here are a few comments about it:
1) The commit message is far too long and contains a snippet you pasted from
IRC. I don’t see the reason for that excerpt even being in the commit
message. Could you reduce the commit message to a reasonable length please?
2) You introduce trailing whitespace in a number of occasions. Please remove that.
My git log says lines 21, 225, 245, 248 and 268 are affected.
3) In x_con_init(), you create a colormap for each window, without ever freeing
them. This is an X11 resource leak and will make X11 go out of memory after
a while. Can you avoid creating new colormaps for every window entirely?
4) Can you please use XCB_COPY_FROM_PARENT instead of an explicit 0 for the
depth? I think this makes the effect clearer.
5) In get_visual_depth and get_visualid_by_depth, please adhere to our coding
style: no spaces between the function name and its arguments. So, use
"xcb_depth_next(&depth_iter)" instead of "xcb_depth_next (&depth_iter)".
Thanks!
Best regards,
Michael
Attachment:
0001-Use-32bit-visual-only-when-needed.patch
Description: Binary data