Having a few issues with 5.0.0 (custom group/decimal separators)

[btrautmann]

Hi there,

Sorry for the delay in testing 5.0.0. I finally got around to it, but I am having a few unit tests fail, and it's not obvious immediately whether it's my issue or the packages. All of these tests passed (with the code in the previous issue) on the prior version.

Here's my formatting function (no longer doing a temp replacement of group/decimal separators as described in #79)

String formatted(Option<CurrencyFormat> currencyFormat) {
  // TODO: This is a temporary workaround until we introduce a better way to
  // handle currrency formatting when the currency format is not yet available.
  if (currencyFormat case Some<CurrencyFormat>(:final some)) {
    // Example patterns are provided by the YNAB API without the currency
    // symbol.
    // - Replace all digits with `#`
    // - Use 0s to denote post-decimal precision
    // - Add symbol back in at the beginning or end of the pattern, as needed.
    String createPattern() {
      // e.g 123,456.789
      final sourceFormat = some.exampleFormat;
      // e.g ###,###.###
      final hashes = sourceFormat.replaceAll(RegExp(r'\d'), '#');
      final groupIndex = hashes.indexOf(some.groupSeparator);
      final decimalIndex = some.decimalDigits > 0 ? hashes.indexOf(some.decimalSeparator) : sourceFormat.length;
      final zeros = List.generate(some.decimalDigits, (i) => '0').join();
      // e.g ###,###.000
      final groupSubstring = hashes.substring(0, groupIndex);
      final decimalSubstring = hashes.substring(groupIndex + 1, decimalIndex);
      final pattern = '$groupSubstring${some.groupSeparator}$decimalSubstring${some.decimalSeparator}$zeros';
      // e.g S###,###.000
      final withSymbol = some.shouldDisplaySymbol
          ? some.isSymbolFirst
              ? 'S$pattern'
              : '${pattern}S'
          : pattern;
      print('BRANDON: withSymbol: $withSymbol');
      return withSymbol;
    }

    String symbol() => some.shouldDisplaySymbol ? some.currencySymbol : '';

    final pattern = createPattern();
    final currency = Currency.create(
      some.isoCode,
      some.decimalDigits,
      symbol: symbol(),
      pattern: pattern,
      groupSeparator: some.groupSeparator,
      decimalSeparator: some.decimalSeparator,
    );

    final minorUnits = absMinorUnitsForDecimalDigits(some.decimalDigits);
    final money = Money.fromIntWithCurrency(
      minorUnits,
      currency,
      decimalDigits: some.decimalDigits,
    );
    return _formatWithSign(money.toString());
  } else {
    // If no currency format is available, assume USD.
    final currency = Currency.create('USD', 2);
    final money = Money.fromIntWithCurrency(absMinorUnitsForDecimalDigits(2), currency);
    return _formatWithSign(money.toString());
  }
}

Here are just a few examples of failures:

test('group separator as space', () {
  const euroCurrency = CurrencyFormat(
    isoCode: 'EUR',
    decimalDigits: 2,
    decimalSeparator: ',',
    groupSeparator: ' ',
    currencySymbol: '€',
    isSymbolFirst: false,
    shouldDisplaySymbol: true,
    exampleFormat: '123 456,78',
  );

  const amount = 1234567890;
  final formatted = amount.formatted(const Some(euroCurrency));
  expect(formatted, '1 234 567,89€');
});

fails with:

BRANDON: withSymbol: ### ###,00S
Expected: '1 234 567,89€'
  Actual: '1 23 45 67 1 23 45 67€'
   Which: is different.
          Expected: 1 234 567,89€
            Actual: 1 23 45 67 1 2 ...
                        ^
           Differ at offset 4

test('standard with zeros', () {
  const euroCurrency = CurrencyFormat(
    isoCode: 'EUR',
    decimalDigits: 2,
    decimalSeparator: ',',
    groupSeparator: '.',
    currencySymbol: '€',
    isSymbolFirst: false,
    shouldDisplaySymbol: true,
    exampleFormat: '123.456,78',
  );

  const amount = 100000000;
  final formatted = amount.formatted(const Some(euroCurrency));
  expect(formatted, '100.000,00€');
});

fails with:

BRANDON: withSymbol: ###.###,00S
Expected: '100.000,00€'
  Actual: '100000,000000€'
   Which: is different.
          Expected: 100.000,00€
            Actual: 100000,000000 ...
                       ^
           Differ at offset 3

Using the same euroCurrency from the previous test:

test('zero', () {
  const amount = 0;
  final formatted = amount.formatted(const Some(euroCurrency));
  expect(formatted, '0,00€');
});

fails with

BRANDON: withSymbol: ###.###,00S
Expected: '0,00€'
  Actual: '0,000000€'
   Which: is different.
          Expected: 0,00€
            Actual: 0,000000€
                        ^
           Differ at offset 4

test('decimal separator as hyphen', () {
  const euroCurrency = CurrencyFormat(
    isoCode: 'EUR',
    decimalDigits: 2,
    decimalSeparator: '-',
    groupSeparator: ' ',
    currencySymbol: '€',
    isSymbolFirst: false,
    shouldDisplaySymbol: true,
    exampleFormat: '123 456-78',
  );

  const amount = 1234567890;
  final formatted = amount.formatted(const Some(euroCurrency));
  expect(formatted, '1 234 567-89€');
});

fails with (note: the same failure occurs for / as group separator) :

BRANDON: withSymbol: ### ###-00S
The pattern contains an unknown character: '-'
package:money2/src/pattern_encoder.dart 206:11  PatternEncoder._getMoneyPattern
package:money2/src/pattern_encoder.dart 78:26   PatternEncoder._formatMajorPart
package:money2/src/pattern_encoder.dart 53:17   PatternEncoder.encode
package:money2/src/money.dart 483:15            Money.encodedBy
package:money2/src/money.dart 445:24            Money.toString
package:lumy/ui/currency.dart 61:34             CurrencyIntX.formatted
test/ui/currency_test.dart 214:34               main.<fn>.<fn>.<fn>

[bsutton]

whilst I think its a bug, allowing a '-' as decimal separator seems highly problematic due to confusion with a -ve value.

I'm inclined to disallow - and + as any type of separator.

[btrautmann]

whilst I think its a bug, allowing a '-' as decimal separator seems highly problematic due to confusion with a -ve value.

I'm inclined to disallow - and + as any type of separator.

That's fair, I just don't really have a choice but to support it 😅