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

Add convenience method to Json struct to output pretty strings #166

Merged
merged 4 commits into from
Jan 22, 2013
Merged

Add convenience method to Json struct to output pretty strings #166

merged 4 commits into from
Jan 22, 2013

Conversation

jniehus
Copy link

@jniehus jniehus commented Jan 21, 2013

added convenience method to Json struct to output pretty strings and also added level (default == 0) parameter to "toPrettyJson()" overload

Joshua Niehus added 4 commits January 21, 2013 15:14
@s-ludwig
Copy link
Member

I get the feeling that all the Json->string conversion requires a bit of a clean up. Maybe toJson/``toPrettyJsonshould getwriteJsonString`and `writePrettyJsonString` with your `Json.toPrettyString` added and the old names kept as a deprecated alias...

Btw. do you see/have an actual use for the level parameter? My original indention was more of an internal nature - it would have been cleaner to hide it in an inner function. But of course it doesn't hurt as a public option either.

@jniehus
Copy link
Author

jniehus commented Jan 22, 2013

Yea that sounds good.

Btw. do you see/have an actual use for the level parameter?

I could live without it. I used it when dumping test results and debugging info back to the console. Once everything thing starts working, i just git rid of it and dump things to mongodb. For my 1 use case, 0 indention is good enough.

@s-ludwig
Copy link
Member

Hm at least it is a use case. I'm really struggling with a decision here, because

  1. Technically it's perfectly fine to add it and also doesn't really complicate the interface
  2. It might lead to the wish to customize the output even more (indentation style, line breaks etc) and then either more parameters get added or the parameter needs to get changed to a struct (breaking backwards compatibility)

@s-ludwig
Copy link
Member

I'll just merge it in with the parameter for now, though, and defer the final decision a bit...

s-ludwig added a commit that referenced this pull request Jan 22, 2013
Add convenience method to Json struct to output pretty strings
@s-ludwig s-ludwig merged commit f35e4f1 into vibe-d:master Jan 22, 2013
@s-ludwig
Copy link
Member

Okay the renamed functions are in 2948051 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants