-
Notifications
You must be signed in to change notification settings - Fork 42
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
stringify adds underscore as integer decimal separator #34
Comments
I'm honestly surprised that you think it would be more editable/readable without separators. That said, using thousands separators versus ten thousand separators betrays my biases. I'm hesitant to add configuration to the stringifier, but I have to admit that this is probably the place for it. |
I'm modifying config files for another program using this library, and I'd ideally want to stay true to their formatting, and they don't use underscores. Also, I can see a few cases where underscores might be confusing, such as with port numbers. It's a great feature for readability, but I don't think toggling it would be a bad idea. |
@iarna This package is being used in |
(whoa it sure was a while ago I was active here 馃槄) @iarna I'm looking through the code to see if I could create a simple PR for adding options to the stringify function, but seeing as that part of the library consists of 25 different stringifier functions calling each other recursively, it would result in a lot of passing around of that options object. One idea I have to solve this would be to have a What are your thoughts? Keep it fully functional and pass around config? Convert to class? Maybe even skip implementing options altogether? |
I suppose a good question would be: what's the benefit of injecting the underscore as a thousands separator? Simply for improved readability? Seems like altering a value during conversion wouldn't be a desired (nor expected) effect in the first place. Certainly was not expected in my case. That said, sounds like @scriptcoded has a reasonable suggestion with the class approach. |
@dsolanorush Yup, I agree that having no separator by default would make a lot of sense. In a perfect would I'd argue formatting should be kept upon parsing and stringification, but unfortunately that's practically impossible if you want the parsed output to be a simple object representing only the data contained. I think #42 has some relevant information regarding that topic. All that being said, removing the separator from the formatted output could mess with other dependents, just as it does in reverse for |
@scriptcoded - All fair points. I went ahead and forked the |
Hello 馃憢,
I am a bit intrigued with the fact that integers are added an underscore as a decimal separatorssuch as:
repro: https://codesandbox.io/s/iarnatoml-stringify-45080?file=/src/index.js
This is being used for both integers and floats
iarna-toml/stringify.js
Lines 188 to 191 in 1374bf2
But it would great allowing users to disable it in case they prefer it without separators in order to improve editability?
The text was updated successfully, but these errors were encountered: