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 indentation to json dump method #748

Merged
merged 1 commit into from
Mar 10, 2024
Merged

Conversation

sina-rostami
Copy link
Contributor

The problem is specified in issue #747 .

This is a simple solution to improve the dump method without breaking any APIs and is fully backward-compatible.

include/crow/json.h Outdated Show resolved Hide resolved
Copy link
Member

@gittiver gittiver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a look at my comment. As a general comment I tend to leave dump and dump internal as it is and put the indentation into a pretty_print function or something like this.

@sina-rostami
Copy link
Contributor Author

Have a look at my comment. As a general comment I tend to leave dump and dump internal as it is and put the indentation into a pretty_print function or something like this.

Thank you for the review. Leaving those functions as they are, I should duplicate a bunch of code in pretty_print as they have much in common. Moreover, dump and dump_internal can still be used with and for the non-pretty style purpose without losing any performance in comparison to the current version. However, if my reasons didn't convince you, please let me know and I'll separate them.

@sina-rostami sina-rostami requested a review from gittiver January 23, 2024 20:29
@gittiver gittiver added the feature Code based project improvement label Jan 30, 2024
@mrozigor
Copy link
Member

mrozigor commented Feb 2, 2024

I don't know if formatting output should be a part of the library. But that's up too you . Code looks ok I guess ;)

@gittiver gittiver merged commit 106aeb0 into CrowCpp:master Mar 10, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Code based project improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants