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

Removing jsbeautifier from the project using the pure json lib which … #6

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pmithrandir
Copy link
Collaborator

…is more efficient.

Please check if the presentation of the data is still ok for you.
Main change is that list value are now listed one per line, where before you could get many per line.

File will be longer, but values will be easier to search.

@pmithrandir pmithrandir requested a review from jones139 January 12, 2024 21:59
@pmithrandir pmithrandir linked an issue Jan 12, 2024 that may be closed by this pull request
@jones139
Copy link
Member

I'll run this and see what the output looks like tomorrow, thank you!

@jones139
Copy link
Member

I have run it, and have remembered that this is how I did the JSON output initially, but thought the files were very long and difficult to scan down to look for interesting things - I introduced jsbeautifier to save having to write some code to output the arrays without newlines after each character.
So I do prefer the jsbeautifier output - how about we add a command line option to use it rather than making it the default if you are concerned about the run speet?

@pmithrandir
Copy link
Collaborator Author

Hi,

I think it's a matter of choice. If speed is relevant for thet output generation, moving to json lib make sense... but I'm not sure it is currently.

What I could suggest is to keep your system and to add a comment explaining what is the expected gain and why we do not choose to use json lib in the end.

If performance are requested, the user can also disable pretty export.

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.

Json beautifier is very consuming in resources
2 participants