Bug 328050

Did you set those up on the 'numbers' tab? You need to modify the settings on the 'money' tab of the locale settings in KDE which should be picked up by KMyMoney.

Yes, I set up both tab numbers and money. Then I click Apply and restart my computer also, but there isn't any changes.

*** Bug 300070 has been marked as a duplicate of this bug. ***

It could be that KMyMoney will not allow a space as numbers separator, need to check this.

Is there any progress on this issue ?

I'm french, and I used to have a space as group separator for numbers. Currently, kmymoney 4.8.0 (running with KDE 4.14.29 on Linux Arch 4.9.11-1) is not able to be configured this way.

Thx.

According to https://docs.oracle.com/cd/E19455-01/806-0169/overview-9/index.html the affected languages are Canadian, German, Dansk, Finish, French and Swedish.

Setting thousand separator is handled by MyMoneyMoney::setThousandSeparator() which simply ignores ' ' as separator. Changing is really simple by removing some lines inside the mentioned method.

The much bigger task is to verify that removing the former "ignore" behavior does not introduce unwanted regressions.

(Candian French and Canadian English are two separate languages)
Does this only affect KMM, or is there anything at a higher level in KDE which should be changed also?

(In reply to Jack from comment #7)
> (Candian French and Canadian English are two separate languages)
> Does this only affect KMM, or is there anything at a higher level in KDE
> which should be changed also?

This does only affect KMM only. I kmm main function the KDE thousand separator system settings is copied into kmm, see below

main.cpp
138: // setup the MyMoneyMoney locale settings according to the KDE settings
139: MyMoneyMoney::setThousandSeparator(KGlobal::locale()->monetaryThousandsSeparator()[0]);

Because ' ' as thousand separator is ignored the whole application does not know anything about this value except the cases where KGlobal::locale()->monetaryThousandsSeparator() is used directly, which is 
1. MyMoneyQifProfile::clear()
2. kMyMoneyMoneyValidator::validate()

An interesting location is inside GncObject::hide() 
... 
      case MONEY2:
        in = MyMoneyMoney(data);
        if (data == "-1/0") in = MyMoneyMoney();
        mresult  = MyMoneyMoney(m_moneyHideFactor) * in;
        mresult.convert(10000);
        MyMoneyMoney::setThousandSeparator(' ');
        result = mresult.formatMoney("", 2);
        break;

Because ' ' is converted to '', which means no thousand separator this could be rewritten to 

        result = mresult.formatMoney("", 2, false);

The third parameter is a value for showThousandSeparator.

I'm talking about code because the task is to inspect any call of formatMoney() if it falls into one of the following category: 

1. used for debug output
2. used for gui output
3. used for further processing
4. used for file export

Category 1. and 2. are uncritical (from my current knowledge) but the other need to be inspected  and optional fixed like the mentioned example.

<offtopic>Refactoring the above mentioned code fragment also fixes an additional bug: Because the original thousand separator is not saved and restored (as it is done in the qif reader), this piece of code kills the thousand separator in the whole application until kmymoney will be been restarted.</offtopic>

Created attachment 106587 [details]
related method calls

With the help of the following script I got a list of related calls:
for i in $(find -name '*.cpp' | grep -v test.cpp); do  grep -A4 -B4 -HnP "formatMoney\(.*[, ]+[0-9]+[ ]*\)" $i; done > bug-328050-grep-result.txt

This list excludes all calls with 

- formatMoney(..., false) -> safe case
- formatMoney(..., true)  -> not used
- all calls in test code

/kmymoney/plugins/ofximport/dialogs/mymoneyofxconnector.cpp
- The ofx spec 2.2 at http://www.ofx.net/downloads/OFX%202.2.pdf mentions in chapter '3.2.9.1 Basic Format' Format: A-32 that amounts should not include any punctuation separating thousands, millions, and so forth, so the solution is to not use thousand separator anymore here

./kmymoney/plugins/csvexport/csvwriter.cpp
- there are two cases were the money are formatted with thousand separator, but the separator is removed afterwards -> do not format with thousand separator

./kmymoney/plugins/weboob/dialogs/mapaccount.cpp
- The formatted money is displayed in an account list -> safe to use thousand separator

./kmymoney/views/kgloballedgerview.cpp
- the money is formatted for internal debugging -> safe to use thousand separator

./kmymoney/reports/listtable.cpp
- the money is formatted for the html report -> safe to use thousand separator

/kmymoney/widgets/kbudgetvalues.cpp
- the money is formatted for a dialog box -> safe to use thousand separator

Git commit 0e75cd0973d7e19d09efd4ac99a4eee0e521f6c5 by Thomas Baumgart.
Committed on 04/06/2018 at 09:40.
Pushed by tbaumgart into branch 'master'.

Adjust usage of thousands separator

Modifications according to the analysis in bug 328050

OFX: the modified code is commented out and only used for test purposes
CSV: just a simplification of the code, no change in functionality

M  +2    -7    kmymoney/plugins/csv/export/csvwriter.cpp
M  +13   -13   kmymoney/plugins/ofx/import/dialogs/mymoneyofxconnector.cpp

https://commits.kde.org/kmymoney/0e75cd0973d7e19d09efd4ac99a4eee0e521f6c5

./kmymoney/widgets/transaction.cpp
- the money is formatted for transactions in the ledger view -> safe to use

./kmymoney/converter/mymoneyqifprofile.cpp
- qif profile editor setup thousand separator for its own -> safe to use

./kmymoney/converter/transactionmatchfinder.cpp
- formatted money is used for debug output -> safe to use

./kmymoney/converter/mymoneyqifwriter.cpp
- same as qif profile editor -> safe to use

./kmymoney/converter/mymoneyqifreader.cpp
- formatted money is used for debug output -> safe to use

./kmymoney/converter/mymoneystatementreader.cpp
- formatted money is used for debug output -> safe to use

./kmymoney/dialogs/kfindtransactiondlg.cpp
- formatted money is used for ui output -> safe to use

./kmymoney/models/accountsmodel.cpp
- formatted money is used for displaying vat rate in accounts table -> safe to use

/kmymoney/mymoney/mymoneytransaction.cpp
- formatted money is used for generating hashes -> supporting space thousand separator is safe to use

./kmymoney/mymoney/storage/mymoneystoragedump.cpp
- formatted money is used for dumping storage -> supporting space thousand separator is safe to use

./kmymoney/mymoney/storage/mymoneystoragesql.cpp:2410
- formatted money is saved as column 'priceFormatted' to database only for informational purpose -> will now write space as thousand separator too, which is safe to use

./kmymoney/wizards/endingbalancedlg/kendingbalancedlg.cpp
- formatted money is used for debug output -> safe to use

./kmymoney/wizards/newloanwizard/keditloanwizard.cpp:129
- thousand separator is not shown in loan rate and other money related fields -> need to be fixed in MyMoneyEdit::setValue()

./kmymoney/wizards/newloanwizard/knewloanwizard.cpp:!67
./kmymoney/wizards/newloanwizard/knewloanwizard.cpp:239
./kmymoney/wizards/newloanwizard/knewloanwizard.cpp:415
- formatted money is used for displaying interest rate in loan wizard -> safe to use

./kmymoney/wizards/newaccountwizard/knewaccountwizard.cpp
- formatted money is used for displaying money related edit fields in new account wizard -> safe to use
- thousand separator is not shown in loan rate and other money related fields -> need to be fixed in MyMoneyEdit::setValue()

> ./kmymoney/wizards/newloanwizard/keditloanwizard.cpp:129
> - thousand separator is not shown in loan rate and other money related
> fields -> need to be fixed in MyMoneyEdit::setValue()

MyMoneyEdit cares about only allowing the configured language specific thousand separators (tested with de and en_us language)

(In reply to Thomas Baumgart from comment #11)
> Git commit 0e75cd0973d7e19d09efd4ac99a4eee0e521f6c5 by Thomas Baumgart.
> Committed on 04/06/2018 at 09:40.
> Pushed by tbaumgart into branch 'master'.
Because master diverged too much from 4.8 I'm going to recreate similar patch(es) for 4.8

(In reply to Thomas Baumgart from comment #11)
> Git commit 0e75cd0973d7e19d09efd4ac99a4eee0e521f6c5 by Thomas Baumgart.
> Committed on 04/06/2018 at 09:40.
> Pushed by tbaumgart into branch 'master'.
> 
> Adjust usage of thousands separator
> 
> Modifications according to the analysis in bug 328050
> 
> OFX: the modified code is commented out and only used for test purposes
> CSV: just a simplification of the code, no change in functionality
> 
> M  +2    -7    kmymoney/plugins/csv/export/csvwriter.cpp
> M  +13   -13   kmymoney/plugins/ofx/import/dialogs/mymoneyofxconnector.cpp
> 
> https://commits.kde.org/kmymoney/0e75cd0973d7e19d09efd4ac99a4eee0e521f6c5
You fixed this in CsvWriter::writeTransactionEntry() but not in  CsvWriter::writeSplitEntry(), see https://cgit.kde.org/kmymoney.git/tree/kmymoney/plugins/csv/export/csvwriter.cpp#n256

BTW: It looks that removing this replacement, which intention was to fixes clash with field separator, introduces a new bug resulting into invalid csv file in case the memo field contains the field delimiter.

Hi Thomas, see my 4.8 staging branch for this bug - https://github.com/rhabacker/kmymoney/tree/328050-thousand-separator

Git commit 06322c4b889b019c6a15c7f140dd5d43b9864f5f by Ralf Habacker.
Committed on 04/06/2018 at 06:41.
Pushed by habacker into branch '4.8'.

Fix resetting of thousand separator in gnc reader

M  +1    -2    kmymoney/converter/mymoneygncreader.cpp

https://commits.kde.org/kmymoney/06322c4b889b019c6a15c7f140dd5d43b9864f5f

Git commit be41c8d8e23b665619505bf0577be09f11510d05 by Ralf Habacker.
Committed on 04/06/2018 at 10:21.
Pushed by habacker into branch '4.8'.

Fix formatting MyMoneyMoney instances in csvwriter

Format the MoneyMoney instance without the thousands separator instead
of formatting it with thousands separators and then removing it.

M  +2    -3    kmymoney/plugins/csvexport/csvwriter.cpp

https://commits.kde.org/kmymoney/be41c8d8e23b665619505bf0577be09f11510d05

Git commit f5d5764af24ef75f889ae3ba2200ce616e9270ea by Ralf Habacker.
Committed on 04/06/2018 at 10:20.
Pushed by habacker into branch '4.8'.

In ofx connector do not format MyMoneyMoney instances with thousand separator

The ofx spec 2.2 at http://www.ofx.net/downloads/OFX%202.2.pdf mentions
in chapter '3.2.9.1 Basic Format' Format: A-32 that amounts should not
include any punctuation separating thousands, millions, and so forth.

M  +13   -13   kmymoney/plugins/ofximport/dialogs/mymoneyofxconnector.cpp

https://commits.kde.org/kmymoney/f5d5764af24ef75f889ae3ba2200ce616e9270ea

Git commit a5edf47d2566a3259607c13edf480b48244ad80c by Ralf Habacker.
Committed on 04/06/2018 at 11:26.
Pushed by habacker into branch '4.8'.

Show thousand separator in money edit field
FIXED-IN:4.8.3

M  +1    -2    kmymoney/widgets/kmymoneyedit.cpp

https://commits.kde.org/kmymoney/a5edf47d2566a3259607c13edf480b48244ad80c