Skip to content
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

Issue with French culture #179

Closed
OhSoGood opened this issue Jul 7, 2021 · 3 comments
Closed

Issue with French culture #179

OhSoGood opened this issue Jul 7, 2021 · 3 comments

Comments

@OhSoGood
Copy link

OhSoGood commented Jul 7, 2021

Hi,

I've been happily using SmartFormat for a while (thank you very much for it!), just to realize I was always on English culture (I'm using SmartFormat v2.7 on .NET 4.7).

Switching to French, I realized that the implementation of PluralRules.DualFromZeroToTwo (used in French) did not take into account the 'c' parameter like PluralRules.DualOneOther (used in English).

Ex:
Smart.Format("{0:|One item|Two items}", 1") returns 'One item' as expected, but
Smart.Format(new CultureInfo("fr"), "{0:|Un objet|Deux objets}", 1") returns an empty string instead of 'Un objet'.

This is because DualFromZeroToTwo is too simplistic and should be written like DualOneOther.

// This is DualOneOther original code
private static PluralRuleDelegate DualOneOther => (n, c) =>
        {
            if (c == 2) return n == 1 ? 0 : 1;
            if (c == 3) return n == 0 ? 0 : n == 1 ? 1 : 2;
            if (c == 4) return n < 0 ? 0 : n == 0 ? 1 : n == 1 ? 2 : 3;
            return -1;
        };// Dual: one (n == 1), other

// This is my proposal for DualFromZeroToTwo 
private static PluralRuleDelegate DualFromZeroToTwo => (n, c) => 
        {
            if (c == 2) return n < 2 ? 0 : 1;
            if (c == 3) return n == 0 ? 0 : n < 2 ? 1 : 2;
            if (c == 4) return n < 0 ? 0 : n == 0 ? 1 : n < 2 ? 2 : 3;
            return -1;
        };// DualFromZeroToTwo: one (n == 0..2 fractionate and n != 2), other

In the meantime, I'll stick to English as the current lib's code is erroneous for French and other 'DualFromZeroToTwo' cultures.

@axunonb
Copy link
Member

axunonb commented Jul 7, 2021

Thanks for pointing this out.
As a matter of fact, the PluralRules logic is as much as untested. The high test coverage is misleading here. Although you wrote "issue with French culture", isn't it an issue with the DualFromZeroToTwo delegate, unless French required a different rule?
We'd be more than happy about a PR on that.

@OhSoGood
Copy link
Author

OhSoGood commented Jul 7, 2021

Yes, you are right, as I wrote, the code is erroneous for French and other 'DualFromZeroToTwo' cultures, the issue is about the DualFromZeroToTwo delegate. I mentioned the French culture as I found the issue using that culture actually.

For the PR, my github environment is broken these days, so it may be faster and easier if you commit & push directly.

axunonb added a commit that referenced this issue Jul 23, 2021
@axunonb axunonb mentioned this issue Jul 23, 2021
axunonb added a commit that referenced this issue Jul 23, 2021
@axunonb
Copy link
Member

axunonb commented Jul 23, 2021

Fix will go into v3.0

@axunonb axunonb closed this as completed Jul 23, 2021
axunonb added a commit to axunonb/SmartFormat that referenced this issue Oct 21, 2021
Backported bug fixes from v3.0.0-alpha:
axuno#179 - Corrected DualFromZeroToTwo plural rule
axuno#211 . Illegal placeholder characters that are not 8-bit, will no more throw unexpected ThrowByteOverflowException
axunonb added a commit to axunonb/SmartFormat that referenced this issue Oct 21, 2021
axuno#179 - Corrected DualFromZeroToTwo plural rule
axuno#211 . Illegal placeholder characters that are not 8-bit, will no more throw unexpected ThrowByteOverflowException
axunonb added a commit that referenced this issue Oct 21, 2021
Bumped version to 2.7.1

Back-ported bug fixes from v3.0.0-alpha:
#179 - Corrected DualFromZeroToTwo plural rule
#211 . Illegal placeholder characters that are not 8-bit, will no more throw unexpected ThrowByteOverflowException
axunonb added a commit to axunonb/SmartFormat that referenced this issue Oct 21, 2021
commit ebb820c
Merge: 365bbd0 ca28b30
Author: axunonb <[email protected]>
Date:   Thu Oct 21 10:06:49 2021 +0200

    Merge branch 'axuno:main' into main

commit ca28b30
Author: axunonb <[email protected]>
Date:   Thu Oct 21 10:05:03 2021 +0200

    Backport fixes from v3 (axuno#213)

    Bumped version to 2.7.1

    Back-ported bug fixes from v3.0.0-alpha:
    axuno#179 - Corrected DualFromZeroToTwo plural rule
    axuno#211 . Illegal placeholder characters that are not 8-bit, will no more throw unexpected ThrowByteOverflowException

commit 365bbd0
Author: axunonb <[email protected]>
Date:   Thu Oct 21 09:16:12 2021 +0200

    Bumped version to 2.7.1

    Backported bug fixes from v3.0.0-alpha:
    axuno#179 - Corrected DualFromZeroToTwo plural rule
    axuno#211 . Illegal placeholder characters that are not 8-bit, will no more throw unexpected ThrowByteOverflowException

commit 3165fc7
Author: axunonb <[email protected]>
Date:   Thu Oct 21 08:03:32 2021 +0200

    Updated NuGet secure key

commit 8169585
Author: axunonb <[email protected]>
Date:   Sun Sep 5 23:09:38 2021 +0200

    Update README.md

commit 1e09b6a
Author: axunonb <[email protected]>
Date:   Mon Aug 30 01:52:50 2021 +0200

    Update README.md

commit 8954f3e
Author: axunonb <[email protected]>
Date:   Mon Aug 30 01:51:32 2021 +0200

    Update README.md

commit a16b01c
Author: axunonb <[email protected]>
Date:   Tue Jun 15 00:06:03 2021 +0200

    Update README.md

commit d9462f5
Author: axunonb <[email protected]>
Date:   Tue Jun 15 00:05:17 2021 +0200

    Update README.md
axunonb added a commit to axunonb/SmartFormat that referenced this issue Nov 2, 2021
commit a8dec64
Merge: ebb820c c4f0115
Author: axunonb <[email protected]>
Date:   Tue Nov 2 21:55:35 2021 +0100

    Merge branch 'axuno:main' into main

commit c4f0115
Author: axunonb <[email protected]>
Date:   Tue Nov 2 21:54:46 2021 +0100

    Delete .github/workflows directory

commit 3283ba9
Author: axunonb <[email protected]>
Date:   Tue Nov 2 17:05:09 2021 +0100

    Remove demo projects

commit 0ebf3f4
Author: axunonb <[email protected]>
Date:   Tue Nov 2 16:30:42 2021 +0100

    Corrected path to test project

commit a9a79d1
Author: axunonb <[email protected]>
Date:   Tue Nov 2 16:22:48 2021 +0100

    Add AltCover to test

commit 608533f
Author: axunonb <[email protected]>
Date:   Tue Nov 2 16:07:01 2021 +0100

    Update build.yml

commit 3005692
Author: axunonb <[email protected]>
Date:   Tue Nov 2 15:00:06 2021 +0100

    Update build.yml

commit 4683d59
Author: axunonb <[email protected]>
Date:   Tue Nov 2 14:23:29 2021 +0100

    Update build.yml

commit d3f9b68
Author: axunonb <[email protected]>
Date:   Tue Nov 2 14:19:05 2021 +0100

    Update build.yml

commit 4ac19ea
Author: axunonb <[email protected]>
Date:   Tue Nov 2 14:11:42 2021 +0100

    Update build.yml

commit ef76dba
Author: axunonb <[email protected]>
Date:   Tue Nov 2 14:08:13 2021 +0100

    Update build.yml

commit e2406b4
Author: axunonb <[email protected]>
Date:   Tue Nov 2 14:01:07 2021 +0100

    Update build.yml

commit adee45a
Author: axunonb <[email protected]>
Date:   Tue Nov 2 14:00:40 2021 +0100

    Update build.yml

commit 5464bb9
Author: axunonb <[email protected]>
Date:   Tue Nov 2 13:53:50 2021 +0100

    Create build.yml

commit ebb820c
Merge: 365bbd0 ca28b30
Author: axunonb <[email protected]>
Date:   Thu Oct 21 10:06:49 2021 +0200

    Merge branch 'axuno:main' into main

commit ca28b30
Author: axunonb <[email protected]>
Date:   Thu Oct 21 10:05:03 2021 +0200

    Backport fixes from v3 (axuno#213)

    Bumped version to 2.7.1

    Back-ported bug fixes from v3.0.0-alpha:
    axuno#179 - Corrected DualFromZeroToTwo plural rule
    axuno#211 . Illegal placeholder characters that are not 8-bit, will no more throw unexpected ThrowByteOverflowException

commit 365bbd0
Author: axunonb <[email protected]>
Date:   Thu Oct 21 09:16:12 2021 +0200

    Bumped version to 2.7.1

    Backported bug fixes from v3.0.0-alpha:
    axuno#179 - Corrected DualFromZeroToTwo plural rule
    axuno#211 . Illegal placeholder characters that are not 8-bit, will no more throw unexpected ThrowByteOverflowException

commit 3165fc7
Author: axunonb <[email protected]>
Date:   Thu Oct 21 08:03:32 2021 +0200

    Updated NuGet secure key

commit 8169585
Author: axunonb <[email protected]>
Date:   Sun Sep 5 23:09:38 2021 +0200

    Update README.md

commit 1e09b6a
Author: axunonb <[email protected]>
Date:   Mon Aug 30 01:52:50 2021 +0200

    Update README.md

commit 8954f3e
Author: axunonb <[email protected]>
Date:   Mon Aug 30 01:51:32 2021 +0200

    Update README.md

commit a16b01c
Author: axunonb <[email protected]>
Date:   Tue Jun 15 00:06:03 2021 +0200

    Update README.md

commit d9462f5
Author: axunonb <[email protected]>
Date:   Tue Jun 15 00:05:17 2021 +0200

    Update README.md
axunonb added a commit to axunonb/SmartFormat that referenced this issue Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants