feat: stringifyWithSpace -> add method similar to elm encode

[srghma]

What does this pull request do?

it adds function similar to https://package.elm-lang.org/packages/elm/json/latest/Json-Encode#encode

this pr consumes #29 and #23

the difference is:

• from add stringifyWithSpace, stringifyWithIndent #29 - no ...space alias, and not ...indent, but ...indentation, because I prefer full name
• from stringifyWithSpace #23 - again not ...space

it adds stringifyWithIndentation function

[srghma]

@thomashoneyman reopened the #40

[srghma]

well, maybe the space suffix is better

because it's whitespace and not something else

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify

[srghma]

I have changed my decision and added the new func

[srghma]

@thomashoneyman there is an error in tests I don't know how to fix, could you help

[srghma]

@thomashoneyman I think this pr is ready to be merged

this pr is superior to other 2 prs, because it defines stringifyWith func also

maybe if we rerun tests it will pass now

[thomashoneyman]

@srghma Sorry for such a long response time. I've had some time to think about what we should support in this library. I'm happy to merge a version of this PR which:

• adds a stringifyWithIndent function
• which accepts an Int argument that is passed through to the underlying stringify function
• which has a documentation comment which notes that indentation is restricted to values between 0 and 10 (a value under 0 will be treated as 0, and a value over 10 will be treated as 10).

I'm not sure that stringifyWithSpace is a good idea to support, even though it exists in the JavaScript version. That's because it's essentially allowing a format string that can have anything in it, though only the first 10 characters will be used. The only thing I've personally seen this used for is to provide a '\t' character, but it's much more common to simply provide 2 or 4 spaces instead.

If you feel strongly that there should also be a stringifyWithSpace function, please open an issue and we can discuss it there. As far as merging something ASAP, I'm happy to do so with the function I described above.

Thanks!

[thomashoneyman]

By the way -- we've switched to GitHub Actions, so the next commit to this branch should re-run CI.

[srghma]

@thomashoneyman pushed with comments

but I don't understand - you want me to rename the stringifyWithSpace to stringifyWithIndent?

If you feel strongly that there should also be a stringifyWithSpace function

no, I dont think that there should be an alias, either stringifyWithSpace or stringifyWithIndent is sufficient

[thomashoneyman]

I’m just saying there should only be 1 function, and it should take an int representing a number of spaces to indent. There would not be a function that takes a string as an argument. This would also mean we can remove the Foreign dependency added in this PR (which is why CI is failing at the moment).

[srghma]

removed foreign dep

The only thing I've personally seen this used for is to provide a '\t' character, but it's much more common to simply provide 2 or 4 spaces instead.

yes, but maybe someone anyway find it useful

can remove if you want

[thomashoneyman]

I would rather remove it, for the sake of merging this PR, and if demand seems high for that specific variation on stringify then we can perhaps add it. But if stringifyWithIndent covers the common case then I would rather only have that function, and I believe that it does cover the most common case.