Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Initial value of IBU Adjustments erroneous on systems that use coma as decimal separator #158

Closed
theophae opened this issue Jan 29, 2016 · 6 comments
Closed

Initial value of IBU Adjustments erroneous on systems that use coma as decimal separator #158

theophae opened this issue Jan 29, 2016 · 6 comments
Labels
bug normal priority
Milestone
v2.4.0

Comments

@theophae
Copy link
Contributor

After a fresh install of BT 2.3.0 on windows, I noticed that the default values of IBU Adjustments were set to 10 000,000 instead of 10,000. This is obviously due to the use of coma as separator in my system configuration.
This bug is not blocking as if I set 10,000 as a new value it will remains as is.
@theophae theophae changed the title Initial value of IBU adjustmenta Initial value of IBU Adjustments Jan 29, 2016
@theophae theophae changed the title Initial value of IBU Adjustments Initial value of IBU Adjustments erronous on system use coma as decimal separator Jan 29, 2016
@rocketman768 rocketman768 modified the milestone: v2.4.0 Feb 2, 2016
mikfire pushed a commit to mikfire/brewtarget that referenced this issue Jan 12, 2017
Possibly closes Brewtarget#166 and Brewtarget#158
235e979 
This is a hard thing.

The essence is that in BtLineEdit::setText(BeerXMLElement *element), the
conversion went from QVariant to QString to double. The problem was that the
QString ended up as something like "1.045". The conversion from QString to
double is locale aware, so en_US would get 1.045 but locales like bt_PR would
get 1e3+45.

I fixed this by replacing the QVariant->QString->double conversion with a
QVariant->double conversion. It *seems* to work -- I've tried this with en_US,
bt_PR, de_DE and fr_FR and they all *appear* to do the right thing.

I don't think we need to fix anything else. The values written to the database
are written as doubles, so this kind of conversion is irrelevant. On the other
hand, I have demonstrated an annoying inability to test my own code and my
database my not behave like anybody elses.
mikfire added a commit that referenced this issue Jan 27, 2017
@mikfire
Merge pull request #336 from mikfire/bugs/international
92cd88f 
Possibly closes #166 and #158
@pricelessbrewing
Copy link
Contributor

Tested on latest dev build, looks like it's fixed. Close?

@pricelessbrewing
Copy link
Contributor

@dpettersson can you test this on linux and see if it's an issue for you?

@dpettersson
Copy link
Contributor

dpettersson commented Sep 30, 2018 •
edited
Loading

Sure, i can test it.
But i saw something earlier. The develop branch has (compared to 2.4) 141 more commits and there are 2 that doesn't exist in develop that are in 2.4. Maybe we should merge develop into 2.4 (or vice versa). It would give a great boost to the functionality if we could do something like that.

And also the fix for this has been merged into develop. I don't think it has been included in 2.4 (yet)

@dpettersson
Copy link
Contributor

dpettersson commented Oct 2, 2018 •
edited
Loading

It seems to work for me.
I tested it like this:
I moved the sqlite-db and started the program. It looked OK.

But im not sure if it is 2.4. So if we should close it then maybe we should import the patch.

dpettersson added a commit to dpettersson/brewtarget that referenced this issue Oct 2, 2018
@dpettersson
Closes Brewtarget#158. Imported the patch from develop
30f8f1e
dpettersson added a commit that referenced this issue Oct 7, 2018
@dpettersson
Merge pull request #429 from dpettersson/bug_158
21af59d 
Closes #158. Imported the patch from develop
@mattiasmaahl
Copy link
Collaborator

closed?

@matty0ung
Copy link
Contributor

Since, from the most recent comments, it looks like this is fixed, I'm closing. Please re-open if I've misunderstood.

@matty0ung matty0ung changed the title Initial value of IBU Adjustments erronous on system use coma as decimal separator Initial value of IBU Adjustments erroneous on systems that use coma as decimal separator Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug normal priority
Projects
None yet
Milestone
v2.4.0
Development

No branches or pull requests
6 participants
@rocketman768 @mattiasmaahl @pricelessbrewing @dpettersson @theophae @matty0ung