search

tomwanzek / d3-v4-definitelytyped

[DEPRECATED] This repo was intended as a staging area for typescript definitions supporting the latest major release of D3js (i.e. version 4.1.x) by Mike Bostock. It has been migrated to DefinitelyTyped.
MIT License
53 stars 14 forks source link

Allow arbitrary group and thousands string separator #128

Closed vetvicka closed 7 years ago

vetvicka commented 7 years ago

Separators should not be limited to '.' | ','

Most of Europe uses space separator. I would expect there are countries with completely different separators.

tomwanzek commented 7 years ago

@vetvicka Thanks for catching an omission and for preparing this PR.

I am closing it with the following provisio as this repo is no longer actively maintained. All current definitions previously developed here are now in their most current versions in the DefinitelyTyped/types-2.0 branch. This branch feeds the definition available through the npm @types/d3-format --save acquisition mechanism.

(1) I agree that the non-breaking space \u00a0 should be added to the intersection type for thousands (2) I think widening it to string is too permissive without a specific use-case warranting it. There are a wide variety of locale definitions already 'pre-built' in d3-format locale and none of them is not covered by '.' | ',' | '\u00a0' (3) Widening the type for decimal seems inadvisable, unless you have a specific example where it is required.

I have created PR 12272 to the DefinitelyTyped/types-2.0 branch based on the above consideration and @-mentioned you in it for reference.

If you find any other improvements, please use DefinitelyTyped to create an issue and @-mention me there, so I can respond/review there.

Thanks again!!!