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 support for outputting json to a file #199

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

Conversation

jhf
Copy link

@jhf jhf commented Apr 11, 2019

This solves the problem of console messages, such as
Fetching package information from...
appearing in the json, that breaks json parsing tools.

I'd be happy to do it in another fashion if that's required.

@jhf
Copy link
Author

jhf commented Apr 11, 2019

I see some tests are failing, I'll look into that.

@stil4m
Copy link
Owner

stil4m commented Jul 10, 2019

Thanks @jhf for your PR.

If I understand correctly you are currently running elm-analyse --format=json, but the output of the process is not correct JSON. For the simplicity of the tool I would suggest to suppress the messages that corrupt the JSON output if the flag is set. This is already done for other messages ('file loading' and such).

Having to write to a file brings complexity with it, such as access, intermediate folders which have to be created, and in this case additional configuration in the process.

Would it suffice for you if the human readable messages are suppressed?

@jhf
Copy link
Author

jhf commented Sep 4, 2019

Thanks @jhf for your PR.

You are welcome.

If I understand correctly you are currently running elm-analyse --format=json, but the output of the process is not correct JSON. For the simplicity of the tool I would suggest to suppress the messages that corrupt the JSON output if the flag is set. This is already done for other messages ('file loading' and such).

Ok.

Having to write to a file brings complexity with it, such as access, intermediate folders which have to be created, and in this case additional configuration in the process.

Yes, good point.

Would it suffice for you if the human readable messages are suppressed?

Certainly, are you, @stil4m, taking a stab at removing Fetching package information from... or do you wan't an updated PR?

@stil4m
Copy link
Owner

stil4m commented Sep 5, 2019

If you can update the PR, that would be really cool. I do not have much time on my hands at this moment for this project :(.

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