Handle german number formatting

tbfuerst commented 1 year ago

This pull requests adds support for German locale when a number format is provided on a MergeField

Description

A locale check for german locale is performed while parsing the number formatting string. If the german local ist set for numeric values, the thousands and decimal separators are handled differently.

Motivation and Context

A Program was needed for german users, where users can use Mailmerge. They need to format number data into typical german fomatting which looks like this: N.NNN,NN where dots are thousands separators and commas are decimal separators. To use this, the locale needs to be set like this:

import locale
locale.setlocale(locale.LC_NUMERIC, 'de_DE')

Maybe you want to add this to your documentation

How Has This Been Tested?

Change has been tested on a personal project. No unit test has been written for this particular problem.

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[x] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[ ] I have added tests to cover my changes.
[ ] All new and existing tests passed.

iulica commented 1 year ago

Hi Tobias, thanks for the pull request. Your first commit is rather large, and removes some necessary code and functionality. Maybe it was made on an older version of the code and the merge failed? Can you make a smaller pull request with just the changes related to the german number formatting ? Maybe start with the clean version and then copy/paste the relevant parts without reformatting the whole source code.

tbfuerst commented 1 year ago

Hi Iulian

ah yes you are right. I used the one from the pypi package. I will fix it. Thanks for info!

On Thu, 15 Dec 2022, 12:15 Iulian Ciorăscu, @.***> wrote:

Hi Tobias, thanks for the pull request. Your first commit is rather large, and removes some necessary code and functionality. Maybe it was made on an older version of the code and the merge failed? Can you make a smaller pull request with just the changes related to the german number formatting ? Maybe start with the clean version and then copy/paste the relevant parts without reformatting the whole source code.

— Reply to this email directly, view it on GitHub https://github.com/iulica/docx-mailmerge/pull/10#issuecomment-1352911642, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHSX2S6DI5AGGVRVT26JYQDWNL4UXANCNFSM6AAAAAAS7PM2HM . You are receiving this because you authored the thread.Message ID: @.***>

iulica commented 1 year ago

Also, could you try a more generic solution ? That works with all locale that have different decimal and thousand characters ? locale.localeconv() there you have the keys: 'decimal_point': '.', 'thousands_sep': ','

You can use those instead of having an IF for the specific locale.

iulica commented 1 year ago

Also, please add a test for the new feature. You can have a look in the tests/test_formatting.py and add some tests with locale de_DE. For the test_date() function it is already there.

tbfuerst commented 1 year ago

I'm on it. I plan to do a generic approach with tests. But it will take some time as im doing it on the side.

On Fri, 16 Dec 2022, 19:56 Iulian Ciorăscu, @.***> wrote:

Also, please add a test for the new feature. You can have a look in the tests/test_formatting.py and add some tests with locale de_DE. For the test_date() function it is already there.

— Reply to this email directly, view it on GitHub https://github.com/iulica/docx-mailmerge/pull/10#issuecomment-1355424503, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHSX2SY5VTJAK5TAIYZ4U73WNS3MBANCNFSM6AAAAAAS7PM2HM . You are receiving this because you modified the open/close state.Message ID: @.***>

iulica / docx-mailmerge