From 2a9909c252ba6da0c1a0df4c5a4a71330b7dc5ff Mon Sep 17 00:00:00 2001 From: SomberNight Date: Tue, 10 Jan 2023 14:45:35 +0000 Subject: [PATCH] locale amounts: consistently use "." as dec point, and " " as thou sep Always use "." as decimal point, and " " as thousands separator. Previously, - for decimal point, we were using - "." in some places (e.g. AmountEdit, most fiat amounts), and - `locale.localeconv()['decimal_point']` in others. - for thousands separator, we were using - "," in some places (most fiat amounts), and - " " in others (format_satoshis) I think it is better to be consistent even if whatever we pick differs from the locale. Using whitespace for thousands separator (vs comma) is probably less confusing for people whose locale would user "." for ts and "," for dp (as in e.g. German). The alternative option would be to always use the locale. Even if we decide to do that later, this refactoring should be useful. closes https://github.com/spesmilo/electrum/issues/2629 --- electrum/commands.py | 4 ++-- electrum/exchange_rate.py | 19 +++++++++++++------ electrum/gui/qml/qefx.py | 4 ++-- electrum/gui/qt/amountedit.py | 18 +++++++++++------- electrum/gui/qt/main_window.py | 2 +- electrum/tests/test_wallet.py | 8 ++++---- electrum/util.py | 12 ++++++++---- electrum/wallet.py | 4 ++-- 8 files changed, 43 insertions(+), 28 deletions(-) diff --git a/electrum/commands.py b/electrum/commands.py index 4cf99b205..0ca35feff 100644 --- a/electrum/commands.py +++ b/electrum/commands.py @@ -1339,8 +1339,8 @@ class Commands: except InvalidOperation: raise Exception("from_amount is not a number") return { - "from_amount": self.daemon.fx.ccy_amount_str(from_amount, False, from_ccy), - "to_amount": self.daemon.fx.ccy_amount_str(to_amount, False, to_ccy), + "from_amount": self.daemon.fx.ccy_amount_str(from_amount, add_thousands_sep=False, ccy=from_ccy), + "to_amount": self.daemon.fx.ccy_amount_str(to_amount, add_thousands_sep=False, ccy=to_ccy), "from_ccy": from_ccy, "to_ccy": to_ccy, "source": self.daemon.fx.exchange.name(), diff --git a/electrum/exchange_rate.py b/electrum/exchange_rate.py index 260e9770b..480b5030e 100644 --- a/electrum/exchange_rate.py +++ b/electrum/exchange_rate.py @@ -556,17 +556,24 @@ class FxThread(ThreadJob, EventListener): return d.get(ccy, []) @staticmethod - def remove_thousands_separator(text): - return text.replace(',', '') # FIXME use THOUSAND_SEPARATOR in util + def remove_thousands_separator(text: str) -> str: + return text.replace(util.THOUSANDS_SEP, "") - def ccy_amount_str(self, amount, commas, ccy=None): + def ccy_amount_str(self, amount, *, add_thousands_sep: bool = False, ccy=None) -> str: prec = CCY_PRECISIONS.get(self.ccy if ccy is None else ccy, 2) - fmt_str = "{:%s.%df}" % ("," if commas else "", max(0, prec)) # FIXME use util.THOUSAND_SEPARATOR and util.DECIMAL_POINT + fmt_str = "{:%s.%df}" % ("," if add_thousands_sep else "", max(0, prec)) try: rounded_amount = round(amount, prec) except decimal.InvalidOperation: rounded_amount = amount - return fmt_str.format(rounded_amount) + text = fmt_str.format(rounded_amount) + # replace "," -> THOUSANDS_SEP + # replace "." -> DECIMAL_POINT + dp_loc = text.find(".") + text = text.replace(",", util.THOUSANDS_SEP) + if dp_loc == -1: + return text + return text[:dp_loc] + util.DECIMAL_POINT + text[dp_loc+1:] async def run(self): while True: @@ -683,7 +690,7 @@ class FxThread(ThreadJob, EventListener): def format_fiat(self, value: Decimal) -> str: if value.is_nan(): return _("No data") - return "%s" % (self.ccy_amount_str(value, True)) + return self.ccy_amount_str(value, add_thousands_sep=True) def history_rate(self, d_t: Optional[datetime]) -> Decimal: if d_t is None: diff --git a/electrum/gui/qml/qefx.py b/electrum/gui/qml/qefx.py index 79d833aae..3dd783b44 100644 --- a/electrum/gui/qml/qefx.py +++ b/electrum/gui/qml/qefx.py @@ -108,7 +108,7 @@ class QEFX(QObject, QtEventListener): except: return '' if plain: - return self.fx.ccy_amount_str(self.fx.fiat_value(satoshis, rate), False) + return self.fx.ccy_amount_str(self.fx.fiat_value(satoshis, rate), add_thousands_sep=False) else: return self.fx.value_str(satoshis, rate) @@ -133,7 +133,7 @@ class QEFX(QObject, QtEventListener): return '' dt = datetime.fromtimestamp(int(td)) if plain: - return self.fx.ccy_amount_str(self.fx.historical_value(satoshis, dt), False) + return self.fx.ccy_amount_str(self.fx.historical_value(satoshis, dt), add_thousands_sep=False) else: return self.fx.historical_value_str(satoshis, dt) diff --git a/electrum/gui/qt/amountedit.py b/electrum/gui/qt/amountedit.py index 0dad2f7a9..c6b55c81c 100644 --- a/electrum/gui/qt/amountedit.py +++ b/electrum/gui/qt/amountedit.py @@ -10,7 +10,7 @@ from PyQt5.QtWidgets import (QLineEdit, QStyle, QStyleOptionFrame, QSizePolicy) from .util import char_width_in_lineedit, ColorScheme from electrum.util import (format_satoshis_plain, decimal_point_to_base_unit_name, - FEERATE_PRECISION, quantize_feerate) + FEERATE_PRECISION, quantize_feerate, DECIMAL_POINT) from electrum.bitcoin import COIN, TOTAL_COIN_SUPPLY_LIMIT_IN_BTC @@ -66,13 +66,13 @@ class AmountEdit(SizedFreezableLineEdit): return pos = self.cursorPosition() chars = '0123456789' - if not self.is_int: chars +='.' + if not self.is_int: chars += DECIMAL_POINT s = ''.join([i for i in text if i in chars]) if not self.is_int: - if '.' in s: - p = s.find('.') - s = s.replace('.','') - s = s[:p] + '.' + s[p:p+self.max_precision()] + if DECIMAL_POINT in s: + p = s.find(DECIMAL_POINT) + s = s.replace(DECIMAL_POINT, '') + s = s[:p] + DECIMAL_POINT + s[p:p+self.max_precision()] if self.max_amount: if (amt := self._get_amount_from_text(s)) and amt >= self.max_amount: s = self._get_text_from_amount(self.max_amount) @@ -95,6 +95,7 @@ class AmountEdit(SizedFreezableLineEdit): def _get_amount_from_text(self, text: str) -> Union[None, Decimal, int]: try: + text = text.replace(DECIMAL_POINT, '.') return (int if self.is_int else Decimal)(text) except: return None @@ -127,6 +128,7 @@ class BTCAmountEdit(AmountEdit): def _get_amount_from_text(self, text): # returns amt in satoshis try: + text = text.replace(DECIMAL_POINT, '.') x = Decimal(text) except: return None @@ -141,7 +143,9 @@ class BTCAmountEdit(AmountEdit): return Decimal(amount) if not self.is_int else int(amount) def _get_text_from_amount(self, amount_sat): - return format_satoshis_plain(amount_sat, decimal_point=self.decimal_point()) + text = format_satoshis_plain(amount_sat, decimal_point=self.decimal_point()) + text = text.replace('.', DECIMAL_POINT) + return text def setAmount(self, amount_sat): if amount_sat is None: diff --git a/electrum/gui/qt/main_window.py b/electrum/gui/qt/main_window.py index d9da81327..fa81f236f 100644 --- a/electrum/gui/qt/main_window.py +++ b/electrum/gui/qt/main_window.py @@ -926,7 +926,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): else: fiat_e.follows = True fiat_e.setText(self.fx.ccy_amount_str( - amount * Decimal(rate) / COIN, False)) + amount * Decimal(rate) / COIN, add_thousands_sep=False)) fiat_e.setStyleSheet(ColorScheme.BLUE.as_stylesheet()) fiat_e.follows = False diff --git a/electrum/tests/test_wallet.py b/electrum/tests/test_wallet.py index d32e63fed..dfba11e1b 100644 --- a/electrum/tests/test_wallet.py +++ b/electrum/tests/test_wallet.py @@ -133,19 +133,19 @@ class TestFiat(ElectrumTestCase): self.fx = FakeFxThread(FakeExchange(Decimal('1000.001'))) default_fiat = Abstract_Wallet.default_fiat_value(self.wallet, txid, self.fx, self.value_sat) self.assertEqual(Decimal('1000.001'), default_fiat) - self.assertEqual('1,000.00', self.fx.ccy_amount_str(default_fiat, commas=True)) + self.assertEqual('1 000.00', self.fx.ccy_amount_str(default_fiat, add_thousands_sep=True)) def test_save_fiat_and_reset(self): self.assertEqual(False, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '1000.01', self.fx, self.value_sat)) saved = self.fiat_value[ccy][txid] - self.assertEqual('1,000.01', self.fx.ccy_amount_str(Decimal(saved), commas=True)) + self.assertEqual('1 000.01', self.fx.ccy_amount_str(Decimal(saved), add_thousands_sep=True)) self.assertEqual(True, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '', self.fx, self.value_sat)) self.assertNotIn(txid, self.fiat_value[ccy]) # even though we are not setting it to the exact fiat value according to the exchange rate, precision is truncated away - self.assertEqual(True, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '1,000.002', self.fx, self.value_sat)) + self.assertEqual(True, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '1 000.002', self.fx, self.value_sat)) def test_too_high_precision_value_resets_with_no_saved_value(self): - self.assertEqual(True, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '1,000.001', self.fx, self.value_sat)) + self.assertEqual(True, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '1 000.001', self.fx, self.value_sat)) def test_empty_resets(self): self.assertEqual(True, Abstract_Wallet.set_fiat_value(self.wallet, txid, ccy, '', self.fx, self.value_sat)) diff --git a/electrum/util.py b/electrum/util.py index e2e5cdffe..12b4b1295 100644 --- a/electrum/util.py +++ b/electrum/util.py @@ -33,7 +33,7 @@ import urllib import threading import hmac import stat -from locale import localeconv +import locale import asyncio import urllib.request, urllib.parse, urllib.error import builtins @@ -698,7 +698,11 @@ def format_satoshis_plain( # We enforce that we have at least that available. assert decimal.getcontext().prec >= 28, f"PyDecimal precision too low: {decimal.getcontext().prec}" -DECIMAL_POINT = localeconv()['decimal_point'] # type: str +# DECIMAL_POINT = locale.localeconv()['decimal_point'] # type: str +DECIMAL_POINT = "." +THOUSANDS_SEP = " " +assert len(DECIMAL_POINT) == 1, f"DECIMAL_POINT has unexpected len. {DECIMAL_POINT!r}" +assert len(THOUSANDS_SEP) == 1, f"THOUSANDS_SEP has unexpected len. {THOUSANDS_SEP!r}" def format_satoshis( @@ -737,9 +741,9 @@ def format_satoshis( sign = integer_part[0] if integer_part[0] in ("+", "-") else "" if sign == "-": integer_part = integer_part[1:] - integer_part = "{:,}".format(int(integer_part)).replace(',', " ") + integer_part = "{:,}".format(int(integer_part)).replace(',', THOUSANDS_SEP) integer_part = sign + integer_part - fract_part = " ".join(fract_part[i:i+3] for i in range(0, len(fract_part), 3)) + fract_part = THOUSANDS_SEP.join(fract_part[i:i+3] for i in range(0, len(fract_part), 3)) result = integer_part + DECIMAL_POINT + fract_part # add leading/trailing whitespaces so that numbers can be aligned in a column if whitespaces: diff --git a/electrum/wallet.py b/electrum/wallet.py index 35dce435b..0ea87f743 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -612,13 +612,13 @@ class Abstract_Wallet(ABC, Logger, EventListener): # and not util, also have fx remove it text = fx.remove_thousands_separator(text) def_fiat = self.default_fiat_value(txid, fx, value_sat) - formatted = fx.ccy_amount_str(def_fiat, commas=False) + formatted = fx.ccy_amount_str(def_fiat, add_thousands_sep=False) def_fiat_rounded = Decimal(formatted) reset = not text if not reset: try: text_dec = Decimal(text) - text_dec_rounded = Decimal(fx.ccy_amount_str(text_dec, commas=False)) + text_dec_rounded = Decimal(fx.ccy_amount_str(text_dec, add_thousands_sep=False)) reset = text_dec_rounded == def_fiat_rounded except: # garbage. not resetting, but not saving either