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

Conversation

Projects
None yet
2 participants
@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 some commits Jan 21, 2013

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 22, 2013

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.

Member

s-ludwig commented Jan 22, 2013

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

This comment has been minimized.

Show comment
Hide comment
@jniehus

jniehus 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.

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

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 22, 2013

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)
Member

s-ludwig commented Jan 22, 2013

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

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 22, 2013

Member

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

Member

s-ludwig commented Jan 22, 2013

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

Merge pull request #166 from jniehus/master
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

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 22, 2013

Member

Okay the renamed functions are in 2948051 now.

Member

s-ludwig commented Jan 22, 2013

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