Due to the rounding function, temperature is sometimes displayed as -0 °F. Users have requested that this should be trimmed to 0 °F. So this would be an easy task: Remove the minus sign when temperature is between -0.5 and 0.0. This should only be done for the Fahrenheit unit, but not for Celsius. Make sure you provide a patch for up-to-date git master, properly formatted with git format-patch so that it can be applied with git am.
Harald, why not for Celsius? -0 °C does not make much sense either? Or is it because there's no trimming?
Because the value 0 plays a significant role for temperatures on the Celsius scale. It makes somewhat a difference whether the temperature is -0.5 °C or 0.4 °C. A temperature below 0°C means freezing. So it is not bad if that distinction is made.
-0 is still quite inelegant.
Opinions differ.
I think I fixed this issue. I don't know how to test it so I'm not sure. I don't know how to upload a patch or anything since this is my first try at contributing. Any guidance on next steps would be helpful! I'm lakitu on IRC
Created attachment 6013 Patch to display 0F properly Let me know if I need to change anything but this ought to work. I was having issues checking the patch using git apply --check but I couldn't figure out why it was yelling at me.
Just checked the source code and patch, please note the following: - The patch should also be applied to DEWPOINT, APPARENT_TEMPERATURE and any other code path that uses F degrees, for consistency. - It'd be slower to compare the actual plugin label against -0F and swap it for 0, so I'm fine with the (faster) implementation proposed by Harald, BUT: could you please add a comment explaining that this + 0.5 is performed so that -0F is displayed as 0F? So, if someone else needs to modify this code later they know what this operation was added for. For the rest it's all good for me. Cheers,
Created attachment 6019 Updated: Patch to prevent -0F Added comments in all places the temperature is changed to inform future maintainers.
Looking good to me :-) Harald, care to review and push?
(For clarification, the patch builds, I have read the source and I expect the patch to work, but I cannot run the patched plugin now due to API mismatches)
Will review and test in a few days. Not enough time right now :-/
Gave it a quick glance. Should work so far and does apply only to the rounded values, that's fine and correct. Now just two annotations: Why do an addition of 0.5 instead of simply setting it to 0? Second, you will have figured out already this is quite repetitive (3x same code). Could you refactor (move) it to its own function, or maybe a macro similar to ROUND_TO_INT, let's name it CALC_FAHRENHEIT and it can include the calculation formula too, val = val * 9.0 / 5.0 + 32.0; ? If you need other examples, there are a lot of macros defined in weather-plugin, usually located rather at the top of the files.
Created attachment 6044 Fix_negative_zero_F I thought Harald's idea of just setting temperature to 0 rather than incrementing it by 0.5 is an easier to understand fix. I moved the code that calculates the Fahrenheit temperature to a macro as per his request as well. I made sure to leave a nice comment for future devs explaining exactly why that macro is there and where it's used.
Josh, that looks very nice now, thanks for the patch. The only thing I changed is putting a do while(0) around the multi-line macro, you can read more about that e.g. on http://www.commonsense4commonpeople.net/2008/11/tips-on-writing-c-macros.html. It wouldn't matter in the three cases used here, but if someone else used it in an if block some day, that might cause surprises. Pushed to master: http://git.xfce.org/panel-plugins/xfce4-weather-plugin/commit/?id=f872ecb906431f3ebd250a59248d536fccfca243
Harald, thanks for being patient while I finally got it right! I owe it to Steve. I'm hoping to contribute more. I've been scanning through the bug list here and it seems a lot of issues stem from GTK so I guess I'll have to learn that some more.
I had an issue with it showing -0 for Celsius, but I resolved the issue by myself by unchecking the "Round values" option in the preferences.