Allow user selection of decimal places for cpu freq#683
Allow user selection of decimal places for cpu freq#683insaner wants to merge 1 commit intomate-desktop:masterfrom
Conversation
| <?xml version="1.0"?> | ||
| <schemalist gettext-domain="@GETTEXT_PACKAGE@"> | ||
| <schema id="org.mate.panel.applet.cpufreq"> | ||
| <schema id="org.mate.panel.applet.cpufreq" path="/org/mate/panel/applet/cpufreq/"> |
There was a problem hiding this comment.
This is a breaking change. You're forcing the applet to be non-relocatable and forces all instances of the applet to share the same settings.
| TRUE); | ||
| gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (prefs->priv->show_unit), | ||
| FALSE); | ||
| gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (prefs->priv->decimal_places_combo), |
There was a problem hiding this comment.
This is the wrong GtkWidget type (it's a GtkComboBox but you're treating it as a GtkToggleButton)
| } | ||
|
|
||
| // Keep settings updated when user changes selection | ||
| g_signal_connect (prefs->priv->decimal_places_combo, "changed", |
There was a problem hiding this comment.
You have this signal duplicate across two distinct functions, each calling a different call back. Consolidate this into a single one.
| g_object_class_install_property (object_class, | ||
| PROP_DECIMAL_PLACES, | ||
| g_param_spec_int ("decimal-places", | ||
| "Decimal Places", | ||
| "The number of decimal places to show for the cpu frequency", | ||
| 0, | ||
| MAX_DECIMAL_PLACES, | ||
| MAX_DECIMAL_PLACES, | ||
| G_PARAM_CONSTRUCT | | ||
| G_PARAM_READWRITE)); |
There was a problem hiding this comment.
This doesn't belong here. Decimal places is a display concern, but this refers to the CPU frequency monitor, which is a hardware monitoring abstraction.
|
|
||
| gov_text = g_strdup (governor); | ||
| gov_text[0] = g_ascii_toupper (gov_text[0]); | ||
| freq_label = cpufreq_utils_get_frequency_label (freq, MAX_DECIMAL_PLACES); // show max decimals in tooltip |
There was a problem hiding this comment.
You're overriding freq_label without freeing it first, thus leaking memory.
| // decimal_places = cpufreq_monitor_get_decimal_places (popup->priv->monitor); | ||
| // ^^^ if you want the freq selector to also use the same number of decimal places as for display |
There was a problem hiding this comment.
What's this? You should probably remove it.
| gchar *label; | ||
| gchar *unit; | ||
| gint freq; | ||
| gint decimal_places = 2; |
There was a problem hiding this comment.
Hardcoded value ignores the gsettings entirely.
| PROP_DECIMAL_PLACES, | ||
| g_param_spec_uint ("decimal-places", | ||
| "DecimalPlaces", | ||
| "The monitored cpu", |
There was a problem hiding this comment.
This description is copy-pasted. Needs to reflect the property.
| gtk_widget_set_sensitive (prefs->priv->show_unit, FALSE); | ||
| gtk_widget_set_sensitive (prefs->priv->show_perc, FALSE); | ||
| gtk_widget_set_sensitive (prefs->priv->decimal_places_combo, FALSE); | ||
| gtk_widget_set_sensitive (prefs->priv->decimal_places_label, FALSE); |
2 participants
I didn't need to know my cpu frequency to 2 decimal places, and with 8 of these applets on my panel, the space usage was just too much. So I gave the user the ability to select how many decimal places to show on the cpu frequency. Tooltip still shows the maximum, and the maximum is now configurable (and defaults to 2).