Open jjustra opened 4 years ago
Thank you very much. Will consider it for version 0.11.0.
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.
Hello, I've played with 'monitors' and made some changes. Hope they will be helpful :)