GNOME Bugzilla – Bug 778695
Add a setting for the preferred temperature unit
Last modified: 2017-05-11 02:42:56 UTC
This requires adding support for settings first (currently, recipes has no settings). The "Building applications" chapter here: https://developer.gnome.org/gtk3/stable/ch01s04.html has a section about doing this: https://developer.gnome.org/gtk3/stable/ch01s04.html#id-1.2.3.12.9 Once we have the setting in place, we should make use of it here: https://git.gnome.org/browse/recipes/tree/src/gr-recipe-formatter.c?h=temperature#n148 and convert the temperature to the users preferred unit for display.
Note that I've now merged the temperature branch, so the 'Add temperature' button is available on git master.
Shaily is looking at this bug
We are now using gsettings, so the plumbing for this is in place. A temperature-unit key should be added in data/org.gnome.recipes.gschema.xml. It should probably use an enumeration with the possible values 'celsius' and 'fahrenheit'.
Created attachment 350515 [details] [review] Added setting for preferred temperature unit. Added a boolean setting in data/org.gnome/recipes.gchema.xml for whether to display temperatures in celcius or fahrenheit, with the default set to the more internationally recognized celcius. gr-recipe-formatter.c now finds that setting and converts the temp to the preferred unit.
Review of attachment 350515 [details] [review]: ::: data/org.gnome.Recipes.gschema.xml @@ +64,3 @@ </key> + <key type="b" name="prefer-celcius"> spelling: celsius @@ +71,3 @@ + </description> + </key> + Since you brought it up, I think I would prefer an enum for this, just in case we ever decide to support another temperature scale. An enum looks like this in the schema: <enum id="org.gnome.recipes.TemperatureUnit"> <value value="0" nick="celsius"/> <value value="1" nick="fahrenheit"/> </enum> ... <key name="temperature-unit" enum="org.gnome.recipes.TemperatureUnit"> <default>'celsius'</default> .... In the code, you can then use g_settings_get_enum() to get the numeric value, and you can define an enum in the C code to have readable names for the numeric values: typedef enum { GR_TEMPERATURE_UNIT_CELSIUS = 0, GR_TEMPERATURE_UNIT_FAHRENHEIT = 1 } GrTemperatureUnit; ... unit = g_settings_get_enum (settings, "temperature-unit"); if (unit == GR_TEMPERATURE_UNIT_CELSIUS) ... ::: src/gr-recipe-formatter.c @@ +38,3 @@ + GSettings *settings = gr_settings_get(); + return g_settings_get_boolean(settings, "prefer-celcius"); + } Some formatting issues here: space before (, always and move the closing } to the first column @@ +140,3 @@ int i; + gboolean in_celcius = get_temp_unit (); + g_message ("Should this display in celcius? %s", in_celcius ? "yup" : "nope"); Leftover debug spew @@ +186,3 @@ + num = (num - 32) / 1.8; + unit = "℃"; + } It looks like this is doing the right thing. I might suggest to break this out into an auxiliary function, for better readability. Something like: num = convert_temperature (num, recipe_unit, user_unit); tmp = format_temperature (num, 0, user_unit);
To clarify the convert_temperature idea: I was expecting it to look like this: static int convert_temperature (int recipe_temperature, GrTemperatureUnit recipe_unit, GrTemperatureUnit user_unit) { int user_temperature; if (user_unit == recipe_unit) user_temperature = recipe_temperature; else if (user_unit == GR_TEMPERATURE_UNIT_CELSIUS && recipe_unit == GR_TEMPERATURE_FAHRENHEIT) user_temperature = ... return user_temperature; } But if this is giving you problems, feel free to give me a cleaned up patch without the separate function. It was just an idea...
Created attachment 350684 [details] [review] Add a setting for the preferred temperature unit Changed the temp-unit setting from a boolean to an enum. removed some leftover debug clutter, fixed some of the formatting.
i'm hoping this includes the data from both of my commits but if it doesn't let me know and I'll figure it out. I'm going to go back and work on my git knowledge after this patch is done. I think I understand what you're saying about the seperate function. My concern was that it can only return an int, the number temp, or a string, the unit formatted with the little degree sign, but not both. So there would be at least a moment where it would change one before the other, and the value would go from 350 degrees fahrenheit to 176 degrees fahrenheit, before it changed the unit. So if something misfired in between the two functions you'd have the wrong temp. If there's a way to return two different value types in C in one function I'd be down to do that, I'm just not aware of it. Or maybe this whole issue isn't really of any consequence and I'm just worrying about nothing haha. thanks for the help though, and as always let me know what can be done to improve this.
Review of attachment 350684 [details] [review]: If you fix these two minor issues, and squash the two patches together, it looks good to me. ::: data/org.gnome.Recipes.gschema.xml @@ +69,3 @@ </key> + <key name="temp-unit" enum="org.gnome.recipes.TemperatureUnit"> Stylistically, I would prefer to spell this out: temperature-unit, instead of shortening it to temp-unit. ::: src/gr-recipe-formatter.c @@ +41,3 @@ get_temp_unit (void) { GSettings *settings = gr_settings_get(); This needs to be g_autoptr(GSettings), otherwise we're leaking the settings object
(In reply to Paxana from comment #8) > i'm hoping this includes the data from both of my commits but if it doesn't > let me know and I'll figure it out. I'm going to go back and work on my git > knowledge after this patch is done. It didn't include the changes, it was a second patch that applies after the first one. You can combine the two in various ways. What I usually do is an interactive rebase. In a case like this, it works as follows: Apply the two patches. You can verify that they are in place with git log: $ git log --oneline | head -3 86a6a42 Add a setting for the preferred temperature unit b0c7086 Added setting for preferred temperature unit. 0168c0f Avoid a critical Now run git rebase -i HEAD^^. What this does it is treats the two two commits at the tip of your branch (thats what HEAD^^ means) as a branch to 'rewrite'. It offers you a text editor to do so, looking roughly like this: pick b0c7086 Added setting for preferred temperature unit. pick 86a6a42 Add a setting for the preferred temperature unit # Rebase 0168c0f..86a6a42 onto 0168c0f (2 commands) # # Commands: ... It shows you a list of the commits to rebase (your 2). The first word in each line is a command. 'pick' means to use it as-is. 'squash' means to merge it with the previous commit. There's other commands, but this is just what we need here. So change the first two lines to read: pick b0c7086 Added setting for preferred temperature unit. squash 86a6a42 Add a setting for the preferred temperature unit and save. git will apply the commands, and after the squash, it will offer you to write a new commit message for the merged commit, based on the content of the existing two commit messages. Come up with a good commit message, and save. Done! Now you have a single commit that contains the changes from your two commits. Please add the two fixes I requested: change temp-unit to temperature-unit, and use g_autoptr(GSettings) to avoid leaking the settings object. You can use git commit --amend when committing these changes to merge them into the existing commit, instead of creating a new one that has to be squashed later. > I think I understand what you're saying about the seperate function. My > concern was that it can only return an int, the number temp, or a string, > the unit formatted with the little degree sign, but not both. So there > would be at least a moment where it would change one before the other, and > the value would go from 350 degrees fahrenheit to 176 degrees fahrenheit, > before it changed the unit. So if something misfired in between the two > functions you'd have the wrong temp. If there's a way to return two > different value types in C in one function I'd be down to do that, I'm just > not aware of it. Or maybe this whole issue isn't really of any consequence > and I'm just worrying about nothing haha. I think you are in fact worrying about something that is not a problem, here. While you are correct in sofar as my suggested: num = convert_temperature (num, recipe_unit, user_unit); tmp = format_temperature (num, 0, user_unit); does things in two steps. The first line changes the numeric value, and the second line formats the new numeric value with the correct unit as a string. But that does really not cause any problem, since we only ever put the end result into the ui for the user to see. If you wanted to avoid the mental discomfort of knowing that the numeric value in num changes its association from being a value according to recipe_unit to being a value according to user_unit, you could introduce a second variable here, and write this as: user_num = convert_temperature (recipe_num, recipe_unit, user_unit); tmp = format_temperature (user_num, 0, user_unit); I prefer it this way, actually. Maximum clarity!
Created attachment 350906 [details] [review] Added setting for preferred temperature unit. Added a enum setting in data/org.gnome/recipes.gchema.xml for whether to display temperatures in celcius or fahrenheit, with the default set to the more internationally recognized celcius. gr-recipe-formatter.c now finds that setting and converts the temp to the preferred unit.
Attachment 350906 [details] pushed as c393383 - Added setting for preferred temperature unit.
Created attachment 351591 [details] [review] Add locale setting for temperature conversion In addition to Fahrenheit and Celsius, added locale to the possible options for preferred temperature. If locale is selected, it looks for a locale and chooses the preferred temperature unit for that area (generally Fahrenheit in the US and Celsius elsewhere).
Attachment 351591 [details] pushed as 3bb40fa - Add locale setting for temperature conversion