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

Adding support for duc json #294

Merged
merged 1 commit into from
Jul 23, 2022
Merged

Adding support for duc json #294

merged 1 commit into from
Jul 23, 2022

Conversation

MYDIH
Copy link

@MYDIH MYDIH commented Jul 19, 2022

Hi !

I would like to have an interactive sunburst graph on one of my websites. I was going to translate XML to JSON (way easier to manipulate IMHO), and then I saw this Feature Request. I didn't add any tests though, I'm not really sure how to test this properly and I didn't find a test of the XML version to take inspiration from ... The tests harness is failing on my side too, seems like the size and md5 changed maybe ?

Anyway, cool project to work on !

while(*s) {
switch(*s) {
case '"': printf("\""); break;
case '\t': putchar('\t'); break;
Copy link
Author

Choose a reason for hiding this comment

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

I wasn't really sure this was mandatory ...

if(e->type == DUC_FILE_TYPE_DIR && size>= min_size) {
duc_dir *dir_child = duc_dir_openent(dir, e);
if(dir_child) {
indent(depth);
Copy link
Author

Choose a reason for hiding this comment

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

At first, I tried to add a new method to merge the root print and the children prints. It was really painful due to commas, in the end I added a boolean to handle those. Thoughts welcome !

@zevv
Copy link
Owner

zevv commented Jul 23, 2022

This is nice, no particular comments on the code or your comments (the bool comma handling is idiomatic, just fine).

I'mma merge this, thanks!

@zevv zevv merged commit a11f367 into zevv:master Jul 23, 2022
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