lxde
Monitors colors+percentages+minor adjustments
Hello, I've played with 'monitors' and made some changes. Hope they will be helpful :)
For the most part, the patch is okay, but there are a few details that need to be addressed.
-
You changed the code (and instructions) by adding Txt and Bg colors. Configurable Bg seems fine, but what if the user doesn't want text? Text (yes/no) should be configurable, and Txt color should only appear (or be editable) then.
-
Don't change default values (DEFAULT_WIDTH, default_colors) unnecessarily, especially if they are configurable anyway.
-
Don't add dead code and commented code (text_size) that you don't use.
-
Don't use fixed size buf[] and sprintf()! There is g_strdup_printf().
-
Make "%.0f%%" translatable.
-
I suppose the positions in cairo_move_to() depend on the font size. If not configurable, define the font size as a macro and use it for the calculation.
-
Nitpick: A lower case txt and bg in CPUtxtColor, RAMbgColor etc. is IMHO easier to read.
I like the idea of showing percentages, but you should probably split the patch step by step (make a patch series): add background color (with a decent default, please!, your default must not change the current default), add text (configurable: yes/no), add text color (could be selectable, i.e. text color implies that the user wants text), add text_size, and so on. This way it should be easier and we can merge piece by piece, even if you are not quite finished, because each patch is self-contained.
