-
Notifications
You must be signed in to change notification settings - Fork 18
Verbose option and always log Elm errors to console #60
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
base: master
Are you sure you want to change the base?
Conversation
|
Thank you, this looks good. My only comment is that there was already an implicit See this call to That said changing the behavior in As a consequence, the flag description might need to be slightly adapted too. |
|
After using this for a bit, I realized there wasn't a clear distinction when it succeeded again (I couldn't make run-pty go green again). So I also added an info log when the build succeeds:
|
|
Actually, what if we always showed documentation error details in the console output, regardless of mode? The verbose flag makes more sense to me for things like stubbing ports. But I don't necessarily want to see stubbing details even when I want to see the errors. I'll try it and see how it goes. |
Example: corrupt file.
| const json = JSON.parse(errorString); | ||
| elmErrors(json); | ||
| } | ||
| catch (err) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this exception handler (14f0e05) after I discovered an issue during our CI builds. We were running elm-doc-preview in parallel with elm-test and elm-review.
It would fail sometimes with a vague error about parsing:
SyntaxError: Unexpected token '+', "+---------"... is not valid JSON
I found it was trying to parse this:
+-------------------------------------------------------------------------------
| Corrupt File: /root/.elm/0.19.1/packages/elm-explorations/test/2.2.0/artifacts.dat
| Byte Offset: 101307
| Message: Could not map value 17 to Bool
|
| Please report this to https://github.com/elm/compiler/issues
| Trying to continue anyway.
+-------------------------------------------------------------------------------
Now, we handle and log it! Elm will gracefully recover, and elm-doc-preview will exit successfully in output mode. 🎉
| } | ||
| } | ||
| else { | ||
| info("✅ Documentation build succeeded!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we change how we determine build success? Currently, if it encounters the corrupt file stderr, "✅ Documentation build succeeded!" won't be logged. Maybe this should be moved to after the docs are successfully parsed.




Thanks for making elm-doc-preview! I love it and use it all the time at work for application documentation. 💌
I use it in server mode locally, and run
elm-doc-preview --output /dev/nullin CI. However, when running locally, if I don't open the docs, I don't know if there's an error. I use Simon Lydell's run-pty to run all of our tools in parallel. With elm-doc-preview, sometimes I don't get the feedback until CI runs. I'd like to have the errors on the console, so I could set the status in run-pty according to the output.Without logging that there's an error, it's always green:

I might be able to add a separate process that runs
elm-doc-preview --output /dev/null, but that seems wasteful and redundant.Instead, I added a
console.errorif there's an error, and a--verboseoption that will display the error details to the console when in server mode.Without

--verboseWith

--verboseI also considered that some of the port stubbing details might make sense to put behind the
--verboseflag, too. We have a lot of them at work, and it floods the output.Please let me know what you think.