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.

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

Currency not Latin fields issue #739

Closed
vomikan opened this Issue Feb 29, 2016 · 44 comments

Comments

Projects
None yet
5 participants
@vomikan
Contributor

vomikan commented Feb 29, 2016

screenshot 29

  • Windows 7x64
  • 1.3.0-alpha

@vomikan vomikan added the bug label Feb 29, 2016

@vomikan vomikan added this to the v1.3 milestone Feb 29, 2016

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 3, 2016

INSERT INTO CURRENCYFORMATS_V1 VALUES(119,'São Tomé and Príncipe dobra','Db','','.',' ','','',100,1,'STD');

@vomikan vomikan referenced this issue Mar 3, 2016

Closed

Correct currency info for "Israeli new shekel" #744

2 of 2 tasks complete
@guanlisheng

This comment has been minimized.

Contributor

guanlisheng commented Mar 7, 2016

it works fine for mac

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 10, 2016

On INSERT for all currencies the result incorrect if non ASCII symbols used
screenshot 37

@vomikan vomikan changed the title from Currency STD to Currency not Latin fields issue Mar 10, 2016

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 10, 2016

Also incorrect code

wxTRANSLATE("United States dollar"), wxTRANSLATE("$"), wxTRANSLATE(""), wxTRANSLATE("."), wxTRANSLATE(" "), wxTRANSLATE(""), wxTRANSLATE(""), wxTRANSLATE("USD")));

wxTRANSLATE(...) only for name needed. Should be

wxTRANSLATE("United States dollar"), L"$", L"", ".", " ", L"", L"", "USD")));
@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 11, 2016

I've discovered that the code:

    wxString s = wxString::Format(L"INSERT INTO CURRENCYFORMATS_V1 VALUES (2, '%s', '%s', '%s', '%s', '%s', '%s', '%s', 100, 1, '%s')"
        , "European euro", L"€", "", ".", " ", "", "", "EUR");

Working fine if placed, for example, in maincurrencydialog.cpp
screenshot 39

But this code in DB_Table_Currencyformats_V1.h provide another result:
screenshot 40

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 12, 2016

@stef145g could you reproduce this issue?

@stef145g

This comment has been minimized.

Contributor

stef145g commented Mar 12, 2016

I have now tested this and it works to some degree but this is not the solution.

I have created a temporary branch: https://github.com/stef145g/moneymanagerex/tree/test_only_non_functional
to allow others to experiment with it.

  1. I have created **DB_Table_Currencyformats_V1.cpp with a few methods moved to the this file.
  2. I have modified the euro currency Unicode character in the .cpp file
  3. I put similar code in mmframe.cpp. Warning: can only be executed once.

When starting the program and immediately creating a new database, the new currencies will be added. I have found that initial EUR character is wrong, but the EUR1 character is correct.

Still not sure why. Just splitting DB_Table_Currencyformats into a .h and .cpp file is not the solution.

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 15, 2016

I've converted DB_Table_Currencyformats_V1.h file to UTF-8 by Notepad++
screenshot 46
It started to work fine.

Now there are a lot of warnings appears:

1>c:\dev\mmex\src\db/DB_Table_Currencyformats_V1.h(223): warning C4566: character represented by universal-character-name '\u00E3' cannot be represented in the current code page (1251) (..\..\src\assetspanel.cpp)

But

#pragma execution_character_set("utf-8")

helps to fix this warnings but at the same time a correct encoding lost.

@guanlisheng

This comment has been minimized.

Contributor

guanlisheng commented Mar 15, 2016

it's already UTF-8...
src/db/DB_Table_Currencyformats_V1.h: UTF-8 Unicode C++ program text, with very long lines

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 15, 2016

There are two UTF-8 with BOM and without

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 15, 2016

How to implement this changes?

        db->ExecuteUpdate(wxString::Format("INSERT INTO CURRENCYFORMATS_V1 VALUES (3, '%s', '%s', '%s', '%s', '%s', '%s', '%s', 100, 1, '%s')", wxTRANSLATE("UK Pound"), wxTRANSLATE("£"), wxTRANSLATE(""), wxTRANSLATE("."), wxTRANSLATE(" "), wxTRANSLATE("Pound"), wxTRANSLATE("Pence"), wxTRANSLATE("GBP")));
to
        db->ExecuteUpdate(wxString::Format("INSERT INTO CURRENCYFORMATS_V1 VALUES (3, '%s', '%s', '%s', '%s', '%s', '%s', '%s', 100, 1, '%s')", wxTRANSLATE("UK Pound"), "\u00A3", "", ".", " ", wxTRANSLATE("Pound"), wxTRANSLATE("Pence"), "GBP"));
@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 15, 2016

It's vital to note that

wxTRANSLATE("")

unacceptable to use.

@guanlisheng

This comment has been minimized.

Contributor

guanlisheng commented Mar 15, 2016

@vomikan does this issue happen on Linux version?
it works pretty fine on Mac and i am suspecting it is a platform specific issue. @moneymanagerex/desktop-mmex-team

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 16, 2016

Workaround for VS is:
screenshot 48

screenshot 47

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 16, 2016

Final call here:
The file should be converted to UTF-8 with signature;
Pragma should be added:

#pragma execution_character_set("utf-8")

L prefix should be added to all non Latin strings

        db->ExecuteUpdate(wxString::Format("INSERT INTO CURRENCYFORMATS_V1 VALUES (4, '%s', '%s', '%s', '%s', '%s', '%s', '%s', 100, 1, '%s')"
, wxTRANSLATE(L"Русский рубль"), "", L"р", ",", " ", wxTRANSLATE(L"руб."), wxTRANSLATE(L"коп."), "RUB"));
@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 16, 2016

Personally, I believe that all non Latin symbols should go away from source files.

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 16, 2016

@guanlisheng

This comment has been minimized.

Contributor

guanlisheng commented Mar 23, 2016

would we close it right now?

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 23, 2016

It's still a mess

@vomikan

This comment has been minimized.

Contributor

vomikan commented Mar 31, 2016

@guanlisheng please remove all entries of wxTRANSLATE("") from py file then update the code and po files. There are many helpless string present now (about 500 like "." "," "$U" "Céntimo" "دج" etc).

@guanlisheng

This comment has been minimized.

Contributor

guanlisheng commented Apr 6, 2016

how is everything going after some wxTRANSLATE removal?

@guanlisheng guanlisheng closed this Apr 6, 2016

@vomikan vomikan reopened this Sep 7, 2016

@vomikan

This comment has been minimized.

@vomikan

This comment has been minimized.

Contributor

vomikan commented Jan 5, 2017

@vomikan vomikan referenced this issue Jan 8, 2017

Closed

Translations #964

@vomikan

This comment has been minimized.

Contributor

vomikan commented Jan 8, 2017

There is a source of CURRENCYFORMATS_V1 table inserts:

INSERT INTO CURRENCYFORMATS_V1 VALUES(1,'United States dollar','$','','.',' ','','',100,1,'USD');
INSERT INTO CURRENCYFORMATS_V1 VALUES(2,'European euro','€','','.',' ','','',100,1,'EUR');
INSERT INTO CURRENCYFORMATS_V1 VALUES(3,'UK Pound','£','','.',' ','Pound','Pence',100,1,'GBP');
INSERT INTO CURRENCYFORMATS_V1 VALUES(4,'Russian Ruble','','₽',',',' ','руб.','коп.',100,1,'RUB');

The Python script wrap all latin values into wxTRANSLATE.
Result:

db->ExecuteUpdate(wxString::Format("INSERT INTO CURRENCYFORMATS_V1 VALUES ('1', '%s', '$', '', '.', ' ', '', '', '100', '1', '%s')"
, wxTRANSLATE("United States dollar"), wxTRANSLATE("USD")));

Problem: Nobody needs translations of ISO codes and currency names as well and nobody will translate so huge amount of strings.
also

db->ExecuteUpdate(wxString::Format("INSERT INTO INFOTABLE_V1 VALUES ('1', '%s', '%s')", wxTRANSLATE("DATAVERSION"), wxTRANSLATE("3")));

This technology is OK only for Categories!

        db->ExecuteUpdate(wxString::Format("INSERT INTO CATEGORY_V1 VALUES ('1', '%s')", wxTRANSLATE("Bills")));
        db->ExecuteUpdate(wxString::Format("INSERT INTO CATEGORY_V1 VALUES ('2', '%s')", wxTRANSLATE("Food")));
@vomikan

This comment has been minimized.

Contributor

vomikan commented Jan 8, 2017

I suggest mark field that to be translated special symbols like "€ £ ₴ ؋ ¥ ...."

/*_*/ or /*wxTRANSLATE*/
or /*_(%s)*/
and /*L%s*/ for wchar_t or somthing like that
INSERT INTO CATEGORY_V1 VALUES(1, /*_*/'Bills');
INSERT INTO CATEGORY_V1 VALUES(2, /*_*/'Food');

INSERT INTO CURRENCYFORMATS_V1 VALUES(5,/*_*/'Ukrainian hryvnia', /*L*/'₴','',',',' ','','',100,1,'UAH');

Accordingly, the Python script should be changed .
Also if file contains non latin symbols like:

#pragma execution_character_set("utf-8")

should be added and file charicter encoding should be UTF8 with signature.

@stef145g stef145g assigned stef145g and unassigned guanlisheng Jan 8, 2017

@stef145g

This comment has been minimized.

Contributor

stef145g commented Jan 9, 2017

Currency values are initialised when the table is created. While we have a problem with the initialisation, this does not fix existing or old databases. The only way to fix existing databases is to rewrite the existing data in the currency lookup table.

The problem with doing this is that in some databases, the existing currencies will be replaced and these may not correctly align with the new currency definitions created on initialisation/readjustment.

The only way around this is to discover what files are affected, make the change then manually readjust the currency for each account that has been realigned with an incorrect currency, including the base currency.

MMEX version 1.3.x has a new command Database Debug under Tools / Database
Upgrade Check: currency_table_upgrade_check.mmdbg.txt
Upgrade Patch: currency_table_upgrade_patch.mmdbg.txt

These two files have been provided to fix this problem as follows:

  1. Down and remove the .txt extension to allow MMEX to access them.
  2. Run and save the output of the upgrade_check file
  3. Run the upgrade_patch file
  4. Restart MMEX
  5. Using the output of the upgrade_check, manually correct the currencies for
    a. The base currency
    b. Each account with the incorrect currency setting
@vomikan

This comment has been minimized.

Contributor

vomikan commented Jan 9, 2017

The problem in currencies for new DB (created exactly in v 1.3.x)
Your recommendation is not enough.

@stef145g

This comment has been minimized.

Contributor

stef145g commented Jan 9, 2017

I have used this procedure to fix my working database that was created at Version 0.9.6.0
My database still had 11 currencies initialised, and I am using the latest version of MMEX.

While this is not the total solution, it is a valid solution for all existing databases created prior to 1.3.0

@vomikan

This comment has been minimized.

Contributor

vomikan commented Jan 9, 2017

How to reproduce the issue:
Start mmex v 1.3.1 - create new DB - create 'Test' account with Euro currency.
Result (Prefix symbol incorrect):
screenshot 353

@gabriele-v

This comment has been minimized.

Contributor

gabriele-v commented Jan 9, 2017

Agree with Nikolay for new databases, we need to modify script in some way to identify field that needs translation.

Regarding fix existing DB, why not simply release a DB upgrade version with something like this (pseudo-SQL):
UPDATE Currency SET Symbol = "€" WHERE Code = "EUR"

@stef145g

This comment has been minimized.

Contributor

stef145g commented Jan 9, 2017

Checking further: the problem is not with python because the resulting DB_Table_CURRENCYFORMATS_V1 file is encoded as utf-8

The resulting code is :

wxString euro = wxString::Format("INSERT INTO CURRENCYFORMATS_V1 VALUES ('2', '%s', '€', '', '.', ' ', '', '', '100', '1', '%s')", wxTRANSLATE("European euro"), wxTRANSLATE("EUR"));
db->ExecuteUpdate(euro);

I have separated the command in two to see the result of 'euro'

Using the same line of code where I define euro to see the resulting string, the utf-8 character is OK when added to the call before calling the .h code.
When I add the same line of code in the .h file, the result is corrupted.
I attempted to put the code in a .cpp file for DB_Table_CURRENCYFORMATS_V1.h but get the same error.

I suspect it has something to do with how utf-8 code is used in wxWidgets. There is a call where the code is passing a wxString but expects a const *char and the conversion is getting corrupted within wxWidgets.

@stef145g

This comment has been minimized.

Contributor

stef145g commented Jan 9, 2017

@gabriele-v Your method would be OK when the database has all its currencies set. Some of the old databases are more than likely, still being used (Mine being a classic example) that will not have all the currencies installed.

Also any new database created will have the same problem, unless after being created on the 2nd run the database is immediately upgraded.

@vomikan

This comment has been minimized.

Contributor

vomikan commented Jan 9, 2017

@stef145g I've provided the working file ( see above ) It's utf8 but with BOM. Current file encoding is utf8 without BOM.
Also (I do not get tired of repeating) #pragma execution_character_set("utf-8") recomended.
And Py script generate too many useless wxTRANSLATION

@gabriele-v

This comment has been minimized.

Contributor

gabriele-v commented Jan 9, 2017

@stef145g
We should provide 2 fixes, one for new created DB and one for existing ones:

  1. For new databases => Working on C++ code side as discussed by you and Nikolay
  2. Create DB Version 8 that will patch currencies basing on Currency Symbol ( = ISO code)

I know that second won't fix all DBs (older ones don't have ISO code populated) but this issue seems to occur only after we have inserted translations into DB_Table_CURRENCYFORMATS_V1.h, so every DB created in this way has ISO codes too. Older DB will still remains without all currencies, but at least we can solve problem on recently created DB.

stef145g added a commit to stef145g/moneymanagerex that referenced this issue Jan 15, 2017

@stef145g

This comment has been minimized.

Contributor

stef145g commented Jan 15, 2017

Taking up @vomikan 's suggestion, sqlite2cpp.py has been modified to provide the correct output for building the .h files.

The database definitions in table_v1.sql for translations have been modified. Data requiring translation require a _tr_ prefix to the name. This allows sqlite2cpp to correctly identify the fields requiring translation and is treated accordingly.

Ref: #1024 Pull Request

@stef145g stef145g added the fixed label Jan 15, 2017

vomikan added a commit that referenced this issue Jan 15, 2017

Merge pull request #1024 from stef145g/UTF_8_SIG
fix(Issue.#739): Currency not Latin fields issue

@vomikan vomikan modified the milestones: v1.3.x, v2.0 Jan 16, 2017

@vomikan

This comment has been minimized.

Contributor

vomikan commented Jan 16, 2017

Now it working as expected.
This code is exactly what I've mentioned early.
screenshot 3
Thank you @stef145g ! Exelent job.

@vomikan vomikan closed this Jan 16, 2017

stef145g added a commit to stef145g/moneymanagerex that referenced this issue Jan 26, 2017

guanlisheng added a commit that referenced this issue Jan 26, 2017

fix(Version.Info): update info for v1.3.2 (#1047)
* fix(Issue.#739): Currency not Latin fields issue

* fix(Submodule.database): upgrade to include changes to sqlite2cpp.py

* fix(#1033): file exists check

* fix(#1043): null account

* fix(#1043): choose first if empty only

* fix(Version.Info): update for v1.3.2

@slodki slodki added i18n win labels Sep 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment