search

Brewtarget / brewtarget

Main brewtarget source code repository.
GNU General Public License v3.0
312 stars 134 forks source link

Some more confusion about decimal separators #733

Closed Jazzbeerman closed 1 year ago

Jazzbeerman commented 1 year ago

Some more confusion about decimal separators

Weekend, time to brew beer and do some new program tests! (I think I flooded you with messages, I'm sorry) This time the problem of decimal separators arose in the priming calculator. If you enter the desired volumes with a separator in the form of a comma, then the calculator does not work correctly, giving negative values. If through a dot, then everything works correctly. Not a major problem, but a bit confusing as the rest of the numbers are entered separated by commas. With commas. 1 With dots 2

It has also been noticed. that in the style editor the ABV range must also be written with a dot, otherwise it will not be saved. Again - if you know about it - it does not cause inconvenience, but of course it is confusing, since the rest of the fields work with commas. Here's what it looks like for me: 3

Just in case, system data: win10, 21H1, program version 3.0.5, 3.0.6, 3.0.7 versions appear the same. It seems to be from the same area as https://github.com/Brewtarget/brewtarget/issues/719

matty0ung commented 1 year ago

Thanks for all the feedback -- much appreciated, and do keep it coming.

Will definitely take a look at these. I am hoping they are simple to fix!

matty0ung commented 1 year ago

FYI I haven't forgotten about this! I have a fix that works on my machine, and that tidies up the code a bit, but I'm still thinking about whether to refactor the way we deal with input/edit fields. (We are about to add a lot of new ones for BeerJSON, so it's a good time to make a change.)

Jazzbeerman commented 1 year ago

This sounds great!

matty0ung commented 1 year ago

Making progress on the input/edit fields. The two main cases (one label and one edit field; one label for two edit fields) are handled, but I need to do some more work for the third case (label with no edit field because the "field" is a graphical "slider" bar). This should ultimately simplify the code, including allowing us to remove special handling for units of measurement for colors (because it is now handled generically).

The main complexity is that we allow you to change the units and scale on a field-by-field basis (as well as globally), so we need to make sure we are consistent in how we "remember" the settings for each field. Current system is a bit fragile/complex for edge cases (no edit fields and two edit fields). I think a small-ish change can make it more robust. Just have to make the small-ish change in a lot of places, but fortunately the compiler can find them all for us! :smile:

matty0ung commented 1 year ago

OK, sorry that took a bit longer than expected. Found lots of interesting different cases for display fields and labels. Have done a bit of rework which should make things a lot easier for all the new units. Anyway, the problems you raised here should be fixed in https://github.com/Brewtarget/brewtarget/releases/tag/v3.0.8.

If you find other similar problems with decimals etc, they should be a lot quicker to fix!