After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 723145 - Currency display does not respect locale
Currency display does not respect locale
Status: RESOLVED OBSOLETE
Product: GnuCash
Classification: Other
Component: Currency and Commodity
2.6.1
Other Windows
: Normal normal
: ---
Assigned To: gnucash-core-maint
gnucash-core-maint
Depends on:
Blocks: 725512 727466
 
 
Reported: 2014-01-28 04:56 UTC by account+gnome@scott.armitage.name
Modified: 2018-06-29 23:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Look at the locale symbol before anything (682 bytes, patch)
2014-03-11 22:35 UTC, Frédéric Perrin
rejected Details | Review
Look at the locale symbol before anything (3.27 KB, patch)
2014-03-26 00:24 UTC, Frédéric Perrin
needs-work Details | Review
Warn when using the same currency symbol twice (3.06 KB, patch)
2014-03-26 00:25 UTC, Frédéric Perrin
none Details | Review
Look at the locale symbol before anything (5.61 KB, patch)
2014-04-06 19:55 UTC, Frédéric Perrin
none Details | Review
Look at the locale symbol before anything (6.23 KB, patch)
2014-04-06 19:58 UTC, Frédéric Perrin
rejected Details | Review
Look at the locale symbol before anything (8.29 KB, patch)
2014-05-19 21:57 UTC, Frédéric Perrin
committed Details | Review

Description account+gnome@scott.armitage.name 2014-01-28 04:56:58 UTC
The default national currency symbols displayed in 2.6.1 do not respect locale. Specifically, it assumes that the USD should be shown as $ while the CAD should be shown as C$, both of which are incorrect. With my locale set to Canada, CAD should be displayed as $ while USD should be shown as USD.

This works correctly in 2.4.15 (I get "$3.14" for CAD and "USD 3.14" for USD).

A work-around for 2.6.x is to open "Tools"->"Security Editor", select "Show National Currencies", and modify USD, USN, and USS to use those symbols instead of $, $n, and $s.

OS: Windows 7 64-bit
GnuCash: 2.4.15 and 2.6.1 tested
Locale: Canada
Default currency: CAD
Comment 1 Frank H. Ellenberger 2014-01-30 07:08:14 UTC
Yeah, the problem is the display symbols are not localized.

But adding over 200 strings, from which only up to 4 have to be "translated" seems to be overkill.

Frédéric, can we first get the symbol from the OS for the default currency; if it is not empty set it for the current locale and replace other occurrences in the local-symbol column with the mnemonic?
Eventually if the symbol is "$", but the region != "US", apply the change also on "$n" and "$s".

As fred is  not registered here, I will forward this report to him per mail.
Comment 2 Frank H. Ellenberger 2014-01-31 21:46:10 UTC
Fred answered:
Hi Frank,

During the implementation it was decided that the confusion between all currencies using e.g. the $ sign was unlikely to be an issue, see the thread starting at http://thread.gmane.org/gmane.comp.gnome.apps.gnucash.devel/36216/focus=36219

What you suggest makes sense. I am on holidays so won't be able to do it myself till the 10th Feb or so.

Fred
Comment 3 Frédéric Perrin 2014-02-10 23:57:28 UTC
Hi Frank and Scott,

The current default symbols are:
USD - $
AUD - $
CAN - C$
GIP - £
GBP - £

Do we want the following :

Locale CAN, CAN-C$, USD-USD, AUD-AUD
Locale AUD, CAN-CAN, USD-USD, AUD-$

And don't care about people from Canada having accounts in GIP and GBP (both with the same symbol)?

Fred.
Comment 4 account+gnome@scott.armitage.name 2014-02-11 00:29:04 UTC
IMO $, USD, and AUD (i.e. drop the "C") makes more sense for the CAN locale and is more consistent overall.

As for £, maybe forgo the symbol entirely and use GIP and GBP when neither are the user's default currency or locale. Semantically this is equivalent to using the currency symbol for the locale's own currency and using ISO 4217 for any other currency.
Comment 5 Frédéric Perrin 2014-02-11 23:00:13 UTC
Oh, so what you're asking for is the revert of bc1970bf (and possibly the fixup 71649793).

I don't have a very strong opinion about the use of the currency symbol by default, so I'll let Geert interject about whether we want to remove this?

Use the symbol in iso-4217-currencies by default
https://github.com/Gnucash/gnucash/commit/bc1970bf71533a5b5c8660ba9ab3d23c39a614d2

https://github.com/Gnucash/gnucash/commit/716497934c0aa81328102023986c224e9134e16d
Comment 6 John Ralls 2014-02-11 23:07:02 UTC
Perhaps it should be a preference.
Comment 7 Geert Janssens 2014-03-10 19:10:56 UTC
(In reply to comment #5)
> Oh, so what you're asking for is the revert of bc1970bf (and possibly the fixup
> 71649793).
> 
> I don't have a very strong opinion about the use of the currency symbol by
> default, so I'll let Geert interject about whether we want to remove this?
> 
> Use the symbol in iso-4217-currencies by default
> https://github.com/Gnucash/gnucash/commit/bc1970bf71533a5b5c8660ba9ab3d23c39a614d2
> 
> https://github.com/Gnucash/gnucash/commit/716497934c0aa81328102023986c224e9134e16d

I find my name mentioned here. That's probably because I committed the first patch ?

It took me a while to reconstruct what this issue is all about. In the end it turns out we are running into issues with the same symbol being used for several currencies in one book. Canadians will probably be the first to run into this.

But this is no reason for me to drop the two commits. We can improve on the situation though IMO.

A few suggestions:

1. It looks like the symbols in our iso-4217-currencies file don't always match the symbol chosen by the locale. The Canadian dollar is one such example: when locale is set to en_CA locale will return "$" but the iso file contains "C$". Obviously a Canadian user will want to see "$" for accounts in Canadian $. To fix this, I believe we should have locale value take precedence if the account is in the locale currency. This means reverting a small part of
https://github.com/Gnucash/gnucash/commit/bc1970bf71533a5b5c8660ba9ab3d23c39a614d2

2. Then to deal with the issue of two currencies having the same symbol in one book. We already have a UI in place to manually fix this. Except that it's not obvious to find. (Is this documented already BTW ?) To help the user, we could add a check when new currencies are added to the book (for example at account creation). If the new currency has the same symbol as an already existing currency pop up a warning to the user with an option to choose a different symbol right there and then. Changing the symbol can be done either by opening the commodity editor for the user, or with a text field in the warning dialog. I prefer to open the commodity editor. That gives the user a hint at where she can change this or other symbols again in the future.

3. To improve the user experience even further, the commodity editor should also check if a changed symbol is already in use by another existing commodity and issue the same warning.

4. Lastly we can't make all users happy with symbols. Some users obviously prefer mnemonics instead of symbols. For those users an option to turn off the logic to load symbols could be helpful. But with the above improvements, this option may no longer be necessary. I'm not too fond of adding options in general. They are often an excuse for poor ux design.
Comment 8 Geert Janssens 2014-03-10 19:26:40 UTC
Another approach for the duplicate symbol issue could be to disambiguate symbols in the iso-4217-currencies file.

This assumes the locale symbol is used for the locale currency.

If the iso-4217-currencies then never has a "$" or "£" symbol but immediately "C$", "US$", "GB£" and so on a user would never end up with duplicate symbols by default.

The locale own currency would get the preferred symbol (say "$" in the Canadian example) and the foreign currency would get a disambiguated symbol (say "US$" in the example).

Just ignore my choices of prefixes though. I'm not acquainted enough with them to know which ones it should be exactly. But you can get the picture from the example.
Comment 9 Frédéric Perrin 2014-03-11 22:34:28 UTC
I think I prefer your follow-up suggestion, it's more predictable than trying to dynamically detect collisions, and renders 1 and 2 moot. I think I'll have a look at 3 later.

There are two things to do, change gnc_commodity_get_nice_symbol() to check if the current currency is the locale one, and update iso-4217-currencies with non-duplicate symbols.

The first one is trivial (patch attached, smoke tested only, but we may want ot offer a way to overwrite the locale symbol), the second one is much harder as you need to look at each of the 200 currencies...
Comment 10 Frédéric Perrin 2014-03-11 22:35:40 UTC
Created attachment 271559 [details] [review]
Look at the locale symbol before anything
Comment 11 Geert Janssens 2014-03-18 08:48:16 UTC
Comment on attachment 271559 [details] [review]
Look at the locale symbol before anything

I agree mostly with this approach. However with your patch it is no longer possible to customize the currency symbol for the current locale. This may be confusing to the user.

So I would first check if there is a user symbol. In the absence of a user symbol. check if the currency is the current locale currency. If so use the locale symbol. As a last resort fall back to the iso-currencies file.

In addition to this patch the iso-currencies file should be disambiguated still. There should be no two currencies in that file with the same currency symbol.
Comment 12 Frédéric Perrin 2014-03-26 00:24:38 UTC
Created attachment 272930 [details] [review]
Look at the locale symbol before anything
Comment 13 Frédéric Perrin 2014-03-26 00:25:16 UTC
Created attachment 272931 [details] [review]
Warn when using the same currency symbol twice
Comment 14 Frédéric Perrin 2014-03-26 00:26:18 UTC
A small stab at removing ambiguities in teh iso-currencies file... Only printing a message so far.
Comment 15 Frédéric Perrin 2014-03-26 00:30:27 UTC
Review of attachment 272930 [details] [review]:

Ignore that, I think I reintroduced the issue where all commodities are dumped to the account file on save... I'll need to look at it again tomorrow.
Comment 16 Geert Janssens 2014-04-05 10:11:09 UTC
Comment on attachment 272930 [details] [review]
Look at the locale symbol before anything

I have looked at this patch but I don't see how this would reintroduce writing all symbols to the data file.

I applied it locally and my first superficial testing shows it does not.

I took some time to study your work so far and have some suggestions for improvement.

To start I think you have all the ingredients together and implemented. I can only propose some refinements to make the code easier to understand by other developers (including your future you in 5 years).

The core of your work revolves around two functions:
your newly introduces gnc_commodity_[gs]et_user_symbol and gnc_commodity_get_nice_symbol.

gnc_commodity_get_user_symbol originally only returned the symbol explicitly set by the user. Later it was changed to return the default symbol in the absence of a user symbol. IMO this addition needlessly complicates things.

For starters the function name doesn't line up anymore with what it really does. Instead of returning the user symbol or nothing it now returns the user symbol or the default symbol. Plus you now need a separate function to determine whether the user set a custom symbol or not, running the kvp query twice. This was not needed before.

I understand you have added this to hide the details  of where the symbol comes from for the commodity editor and commodity tree view. With the function as it is now you only need one call to get a symbol to show the user. 

However I believe it will be clearer if you treat the default symbol and the user symbol as two distinct public properties of a commodity. The default symbol can be a read-only property so it only needs a public getter function. You already have a private setter function (gnc_commodity_set_default_symbol) which is ok. You'd need to add a public getter function though (gnc_commodity_get_default_symbol).

With the default symbol as a public read-only property it would make sense if the commodity editor first tries gnc_commodity_get_user_symbol (the original version). When that returns no result call gnc_commodity_get_default_symbol and use that.

The same goes for the commodity tree view.

Furhter improvement: the current code will write the default symbol as user symbol to the data file if you open the commodity editor for a give commodity.

This can be prevented by refining the gnc_commodity_set_user_symbol function: if the user_symbol is actually the default symbol then unset the symbol instead of writing it to the kvp (meaning write NULL to the kvp).

Lastly if you keep separate getter functions for a user symbol and a default symbol the checks in gnc_commodity_get_nice_symbol can be simplified:
- test for a user symbol with gnc_commodity_get_user_symbol
- if there's no user symbol, check if the commodity is the current locale currency and use its symbol
- if not the locale currency check for a default symbol
- in the absence of a default symbol fall back to the mnemonic

I'm sorry for such a long explanation for a couple of relatively small code fixups.
Comment 17 Geert Janssens 2014-04-05 10:18:47 UTC
Comment on attachment 272931 [details] [review]
Warn when using the same currency symbol twice

Thank you for your patch. I'm trying to estimate it but I'm not sure if I am looking at it with the right context.

Is this code meant to inform developers of duplicate symbols in the iso currency file or to inform the user about this ?

I'm asking because I think these are two totally different audiences and need different warning criteria.
Comment 18 Geert Janssens 2014-04-05 10:26:45 UTC
(In reply to comment #9)
> I think I prefer your follow-up suggestion, it's more predictable than trying
> to dynamically detect collisions, and renders 1 and 2 moot. I think I'll have a
> look at 3 later.
> 
> There are two things to do, change gnc_commodity_get_nice_symbol() to check if
> the current currency is the locale one, and update iso-4217-currencies with
> non-duplicate symbols.
> 
> The first one is trivial (patch attached, smoke tested only, but we may want ot
> offer a way to overwrite the locale symbol), the second one is much harder as
> you need to look at each of the 200 currencies...

Reconsidering my own proposal I'm afraid it's not without its flaws either...

The ideal set up would be that a disambiguation symbol is only chosen when the preferred symbol is already in use. My proposal only covers the symbol preferred by the locale settings. 

But to take myself as an example: my locale defines € as the preferred symbol. This is unambiguous as no other currency uses that symbol. Now consider I am also working with USD and GBP currencies. With my proposal I would get disambiguated symbols for both currencies (say US$ and GB£). But in my situation there is no need for this because there are no other currencies in my book that could be confused with these currencies.

Just thinking out loud though... What you have contributed so far is already a nice feature.
Comment 19 Frédéric Perrin 2014-04-06 19:54:10 UTC
Hi Geert,

Firstly thank you for the detailed review and the effort that went into it.

I believe the attached patch implements what you're suggesting: we have a default_symbol property and [gs]etters, idem user_symbol; get_nice_symbol is the only caller of get_{default,user}_symbol and implements the dispatch between user/locale/default symbol (do we want to move that function to gnc-commodity? that would imply making that file aware of locales).

The name default_symbol is slightly misleading though, as it is the default symbol *for non-locale currencies*. Suggestion of a better name?

> Now consider I am
> also working with USD and GBP currencies. With my proposal I would get
> disambiguated symbols for both currencies (say US$ and GB£). But in my
> situation there is no need for this because there are no other currencies in my
> book that could be confused with these currencies.

While that would be cool, that means:
1- maintain for e.g. USD two symbols, $ and US$ (default and unambiguous default)
2- when displaying sums in USD, scan the commodities used in the book, if none use $ then use "default" for USD, otherwise use "unambiguous default" for USD and whatever other currency already uses $.

1- is a one-time (but boring) task of editing the currency.scm file. 2- introduces many smarts for simply choosing a symbol.

re: iso-currencies-to-c.in:
> Is this code meant to inform developers of duplicate symbols in the iso
> currency file or to inform the user about this ?

Devs, that file is only used at compile time.
Comment 20 Frédéric Perrin 2014-04-06 19:55:04 UTC
Created attachment 273671 [details] [review]
Look at the locale symbol before anything
Comment 21 Frédéric Perrin 2014-04-06 19:58:30 UTC
Created attachment 273672 [details] [review]
Look at the locale symbol before anything
Comment 22 Geert Janssens 2014-04-23 14:15:35 UTC
Comment on attachment 273672 [details] [review]
Look at the locale symbol before anything

Thank you for the update.

I stumbled upon one last detail while evaluating this patch: the function gnc_commodity_set_user_symbol checks for the default symbol but not for the locale supplied symbol. So it may potentially write the locale symbol as user symbol to the data file which as far as I can tell is equally unwanted as writing the default symbol.

I don't think there currently is a code path that would lead to that situation but preventing the possibility is more future safe.
Comment 23 Geert Janssens 2014-04-23 14:22:51 UTC
(In reply to comment #19)
> Hi Geert,
> 
> Firstly thank you for the detailed review and the effort that went into it.
> 
> I believe the attached patch implements what you're suggesting: we have a
> default_symbol property and [gs]etters, idem user_symbol; get_nice_symbol is
> the only caller of get_{default,user}_symbol and implements the dispatch
> between user/locale/default symbol (do we want to move that function to
> gnc-commodity? that would imply making that file aware of locales).
> 
The implementation looks good. I only added one suggestion in comment 22. As to whether this function belongs in gnc-commodity I don't know for sure. I don't know if the engine is locale neutral on purpose or not. If not, I would add this code in gnc-commodity indeed. Anyone have a better insight here ?

> The name default_symbol is slightly misleading though, as it is the default
> symbol *for non-locale currencies*. Suggestion of a better name?

Hmm, not immediately. Your comment already points this out. I think that will be sufficient.
> 
> > Now consider I am
> > also working with USD and GBP currencies. With my proposal I would get
> > disambiguated symbols for both currencies (say US$ and GB£). But in my
> > situation there is no need for this because there are no other currencies in my
> > book that could be confused with these currencies.
> 
> While that would be cool, that means:
> 1- maintain for e.g. USD two symbols, $ and US$ (default and unambiguous
> default)
> 2- when displaying sums in USD, scan the commodities used in the book, if none
> use $ then use "default" for USD, otherwise use "unambiguous default" for USD
> and whatever other currency already uses $.
> 
> 1- is a one-time (but boring) task of editing the currency.scm file. 2-
> introduces many smarts for simply choosing a symbol.
> 
Maybe. In any case this is only a nice to have. We can start with what you have done so far and wait for actual user requests before going further.

> re: iso-currencies-to-c.in:
> > Is this code meant to inform developers of duplicate symbols in the iso
> > currency file or to inform the user about this ?
> 
> Devs, that file is only used at compile time.

Ok.
Comment 24 Frédéric Perrin 2014-05-13 22:28:14 UTC
Hi Geert,

Besides guarding against installing the locale symbol as the user symbol, is there anything else to do on that bug?
That would require making Commodity aware of locales, do we want that? I think that would be necessary for a user that would change the locale currency's symbol to something else, then back to the locale symbol, and the locale symbol is different from the default_symbol in the iso-4217 file.
Comment 25 Geert Janssens 2014-05-14 16:39:12 UTC
Frédéric,

I don't see an issue in making Commodity locale aware. From the mental constructions we have been making earlier in this report it looks like it would only make sense. I think it's Commodity that should know which symbol to use. If it has to evaluate locale information for that then that's ok.

Also from a compilation pov this is ok. The gnc_localeconv function is defined in core-utils which is already a dependency for the engine code.

So after thinking this through some more, yes I think gnc_commodity_get_nice_symbol belongs in gnc-commodity.[ch]

(In reply to comment #23)
> The name default_symbol is slightly misleading though, as it is the default
> symbol *for non-locale currencies*. Suggestion of a better name?

Now I'm looking at this again: how about iso_symbol ? That way the name suggests the symbol is from the iso file.

(In reply to comment #23)
> (In reply to comment #19)
> > While that would be cool, that means:
> > 1- maintain for e.g. USD two symbols, $ and US$ (default and unambiguous
> > default)
> > 2- when displaying sums in USD, scan the commodities used in the book, if none
> > use $ then use "default" for USD, otherwise use "unambiguous default" for USD
> > and whatever other currency already uses $.
> > 
> > 1- is a one-time (but boring) task of editing the currency.scm file. 2-
> > introduces many smarts for simply choosing a symbol.
> > 
> Maybe. In any case this is only a nice to have. We can start with what you have
> done so far and wait for actual user requests before going further.
> 
Just to note I'm already seeing questions on this on the mailing lists... I won't block your patch because of this though.
Comment 26 Frédéric Perrin 2014-05-19 21:57:04 UTC
Created attachment 276807 [details] [review]
Look at the locale symbol before anything
Comment 27 Frédéric Perrin 2014-05-19 22:05:32 UTC
Hi Geert,

I believe the last patch covers what we discussed? We now have several commodity_get_*_symbol, I decided against making them non-externally visible but that would be just a matter of removing the prototype from the h file.

> Now I'm looking at this again: how about iso_symbol ? That way the name
> suggests the symbol is from the iso file.

Wouldn't that imply the symbol is defined by the ISO, which is not the case?
Comment 28 Geert Janssens 2014-05-20 14:24:54 UTC
Comment on attachment 276807 [details] [review]
Look at the locale symbol before anything

Committed to maint and master. Thank you very much for your work and patience :)
Comment 29 Frank H. Ellenberger 2015-04-04 18:48:00 UTC
I confess I am very late here again, but as it is still open...

In Bug 724738 - Value in "Display Symbol" field not saved the wish arises, to have a user defined display symbol for commodities too.

I do not really recall my intention to add the column locale symbol. I saw a field in wikipedia which might be of some use.

It is of use
* in the rare case the OS doesn't offer a currency symbol. Probably the file uses another default currency than the OS.
* if the user dislikes the default symbol. I remember an israeli user prefering "NIS", which led to some confusion, as whe had not implemented the new hebrew symbol.

The question is: should foreign currencies use the locale symbol?

In the current implemention, I am confused, having several symbols in foreign writing systems below cash:travel. at a flight stop over I bought a drink, paid with EUR and got local money back. Here I would prefer to see the ISO symbols. I am not shure how other users feel.

IMHO we should distinuish ISO symbol, locale symbol and display symbol.
I see 2 combinable approaches :
1. add a switch: Use by default locale symbols as display symbols.
2. change the control element so one can select locale or ISO symbol or overwrite it with a user defined symbol
Comment 30 yomlogs 2015-04-08 22:20:56 UTC
Hi all... as someone new to the GNUCash code base I have to admit to not fully understanding the discussion in this bug, so apologies if what I say here is naive or not valid, or belongs in a different ticket.

But the way I see it, when displaying amounts, a commodity's user display symbol (if defined) should simply override all other symbols, regardless of whether the commodity in question is a currency. If not defined, fall back to ISO/locale symbol for currencies, and the defined mnemonic for other commodities. This gives the user the power to display any symbol they like if they so wish, but will display something sensible by default.

The function xaccSPrintAmount() currently uses gnc_commodity_get_nice_symbol() to choose the display symbol for currencies, and gnc_commodity_get_mnemonic() for non-currency commodities. It would appear to make sense to use gnc_commodity_get_nice_symbol() in both cases; that function will already fall back to gnc_commodity_get_mnemonic() if a user symbol is not defined.

Combined with the patch proposed in Bug 724738, this seems to achieve that aim.
Comment 31 Geert Janssens 2015-10-10 17:17:26 UTC
Review of attachment 271559 [details] [review]:

Superseded by a more recent patch. I'm changing the state for this attachment to prevent it from showing up on bugzilla searches for attachments in state "reviewed".
Comment 32 Geert Janssens 2015-10-10 17:17:44 UTC
Review of attachment 273672 [details] [review]:

Superseded by a more recent patch. I'm changing the state for this attachment to prevent it from showing up on bugzilla searches for attachments in state "reviewed".
Comment 33 John Ralls 2018-06-29 23:25:30 UTC
GnuCash bug tracking has moved to a new Bugzilla host. The new URL for this bug is https://bugs.gnucash.org/show_bug.cgi?id=723145. Please continue processing the bug there and please update any external references or bookmarks.