Skip to content

Conversation

@cgkantidis
Copy link
Contributor

JSON has wider support than XML across programming languages, so I believe having the option to write the findings in a JSON format make Duplo more versatile, since its output can be consumed by many more tools.

I chose to support JSON by adding a dependancy to the nlohmann/json header-only library, which handles escaping of the lines correctly.

This has the drawback that we keep all the findings in memory (in a json object) instead of writing them directly to the output file.

If this is considered a deal-breaker, we can re-implement all the escaping of the lines ourselves, as is done for XML, and write directly to the output file.

@cgkantidis
Copy link
Contributor Author

I closed the previous pull request because it was causing a huge diff, due to changes in line endings.
Some files in the repo have dos line endings and some of the have unix line endings (CRLF vs LF).
Consider using a single format for EOL.

@dlidstrom
Copy link
Owner

Hi @cgkantidis ! Thank you for your contribution. I've looked at your PR briefly, and it looks nice. I didn't know cmake has package support. I'll look into it a bit more over the weekend. Generally I think we can include libraries now that the build is done using GitHub actions, so no issue. It would be great if you can make the Windows build green too (probably lacks a header include only).

@cgkantidis
Copy link
Contributor Author

Thanks @dlidstrom ! Indeed I had to include the iterator header file for the MSVC to find the std::back_inserter.

There seems to be an issue with CI now, because it can't create a new tag. It seems like some kind of permission issue? Not sure. Please let me know if there's something I can do from my side.

@cgkantidis cgkantidis force-pushed the main branch 2 times, most recently from dd62a51 to 16c2c83 Compare March 22, 2025 19:06
JSON has wider support than XML across programming languages, so I believe
having the option to write the findings in a JSON format make Duplo more
versatile, since its output can be consumed by many more tools.

I chose to support JSON by adding a dependancy to the nlohmann/json header-only
library, which handles escaping of the lines correctly.

This has the drawback that we keep all the findings in memory (in a `json`
object) instead of writing them directly to the output file.

If this is considered a deal-breaker, we can re-implement all the escaping of
the lines ourselves, as is done for XML, and write directly to the output file.
@cgkantidis cgkantidis force-pushed the main branch 2 times, most recently from 6cfdefe to b8d8c43 Compare March 22, 2025 19:59
@cgkantidis
Copy link
Contributor Author

Hi @dlidstrom, I resolved the conflict with the other pull requests, and I also added a test for the json output.
Sorry for the multitude of force pushes. I was trying to figure out how the "bats" test system works.
Feel free to merge the PR if you're OK with it.
Cheers!

@dlidstrom
Copy link
Owner

This is looking great to me. A big thanks for taking the time to add a test too!

@dlidstrom dlidstrom merged commit 4818619 into dlidstrom:main Mar 23, 2025
6 of 7 checks passed
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