purescript
Add logging options to facilitate json errors and general command-line use
Description of the change
Spago has a lot of functionality. One of its primary responsibilities, though, is to make using the purs executable easier -- that is, Spago calls purs directly, provides some output guidance, and interleaves logging metadata but otherwise stays out of the compiler's way. Because end users employ Spago during development to interact with purs, there have been a few issues (e.g., #154, #170, #256, #316) asking for greater separation between Spago logging and purs output. PR #475 does a good job of this by having Spago log to stderr. However, as @hdgarrood mentions in #256, this approach doesn't fully isolate Spago metadata from purs data, since the compiler prints json errors to the same stderr stream. For common Spago use cases, this mingling of streams isn't much of a problem in itself, but it does make pipelining on the command-line difficult, since Spago's logging messages then affect downstream processing. This pull request offers a possible remedy by giving users greater control over Spago logging, including the option of dedicating a file handle exclusively to Spago output and the option of suppressing logging entirely.
Here's an example of the former CLI option. In the bash command below, logging by purs on stdout is suppressed and logging by Spago on Spago's own file handle 3 is redirected to stderr. Because compilation errors, originally on stderr, are redirected to stdout, they can be piped for additional processing.
$ spago --output-stream 3 build -n -u --json-errors 3>&2 2>&1 1>/dev/null | ./format.sh
{
"warnings": [
{
"warningCode": "UnusedImport",
"loc": "start: 4, end: 4, file: src/Main.purs",
"msg": " The import of Data.Either is redundant "
}
],
"errors": [
{
"errorCode": "TypesDoNotUnify",
"loc": "start: 10, end: 10, file: src/Main.purs",
"msg": " Could not match type Effect with type Function Int while trying to match type Effect Unit with type Int -> t0 while checking that expression (log \"\\127837\") 3 has type Effect Unit in value declaration main where t0 is an unknown type "
}
]
}
[error] Failed to build.
src/Main.purs
module Main where
import Prelude (Unit)
import Data.Either
import Effect (Effect)
import Effect.Console (log)
main :: Effect Unit
main = do
log "🍝" 3
format.sh
#!/bin/sh
sed --unbuffered 's/\\n/ /g; s/\s\+/ /g' \
| jq --unbuffered \
' { warnings:
.warnings
| [.[]
| { warningCode: .errorCode
, loc: "start: \(.position.startLine), end: \(.position.endLine), file: \(.filename)"
, msg: .message
}
]
, errors:
.errors
| [.[]
| { errorCode: .errorCode
, loc: "start: \(.position.startLine), end: \(.position.endLine), file: \(.filename)"
, msg: .message
}
]
}
'
For comparison, and to show the usefulness of Spago pipelining, here's the same compilation output but without --json-errors and without formatting.
$ spago --output-stream 3 build -n -u --json-errors 3>&2 2>&1 1>/dev/null
{"warnings":[{"position":{"startLine":4,"startColumn":1,"endLine":4,"endColumn":19},"message":" The import of Data.Either is redundant\n","errorCode":"UnusedImport","errorLink":"https://github.com/purescript/documentation/blob/master/errors/UnusedImport.md","filename":"src/Main.purs","moduleName":"Main","suggestion":{"replacement":"","replaceRange":{"startLine":4,"startColumn":1,"endLine":4,"endColumn":19}},"allSpans":[{"start":[4,1],"name":"src/Main.purs","end":[4,19]}]}],"errors":[{"position":{"startLine":10,"startColumn":3,"endLine":10,"endColumn":12},"message":" Could not match type\n\n Effect\n\n with type\n\n Function Int\n\n\nwhile trying to match type Effect Unit\n with type Int -> t0\nwhile checking that expression (log \"\\127837\") 3\n has type Effect Unit\nin value declaration main\n\nwhere t0 is an unknown type\n","errorCode":"TypesDoNotUnify","errorLink":"https://github.com/purescript/documentation/blob/master/errors/TypesDoNotUnify.md","filename":"src/Main.purs","moduleName":"Main","suggestion":null,"allSpans":[{"start":[10,3],"name":"src/Main.purs","end":[10,12]}]}]}
[error] Failed to build.
$ spago -o 3 build -n 3>&2 1>/dev/null
Warning found:
in module Main
at src/Main.purs:4:1 - 4:19 (line 4, column 1 - line 4, column 19)
The import of Data.Either is redundant
See https://github.com/purescript/documentation/blob/master/errors/UnusedImport.md for more information,
or to contribute content related to this warning.
Error found:
in module Main
at src/Main.purs:10:3 - 10:12 (line 10, column 3 - line 10, column 12)
Could not match type
Effect
with type
Function Int
while trying to match type Effect Unit
with type Int -> t0
while checking that expression (log "\127837") 3
has type Effect Unit
in value declaration main
where t0 is an unknown type
See https://github.com/purescript/documentation/blob/master/errors/TypesDoNotUnify.md for more information,
or to contribute content related to this error.
[error] Failed to build.
Checklist:
- [ ] Added the change to the "Unreleased" section of the changelog
- [X] Added some example of the new feature to the
README - [ ] Added a test for the contribution (if applicable)
P.S.: the above checks are not compulsory to get a change merged, so you may skip them. However, taking care of them will result in less work for the maintainers and will be much appreciated 😊
Is Spago now part of Purescript's official tooling? It looks like ownership of the project has changed. I'm wondering, because I was going to suggest that this PR has broader merit than ease of json-errors use alone, that Spago at times acts more like a pair (or more) of semi-independent applications than a single application, and that this two-program conceptual model perhaps warrants a non-traditional multi-"typed" streaming interface. However, now I'm not so sure.
I suppose there could still be a case made for granular output streams. Granularity would seem to make command-line use easier in general (users could project a tuple of streams onto the desired component rather than filter a single merged stream) and to better isolate Spago and compiler functionality. But if Spago and purs are now to be developed in concert, chances of contention between the two, present or future, are reduced and, at worst, any such conflict likely resolved within a release cycle. Component insularity, then, from among the long list of design goals, becomes less important and perhaps not worth the effort or complexity costs.
That's a reasonable argument to make, and if that's the case, does this pull request have any parts that could still be of use? Are there featurelets that I can strip out and resubmit?
A related question about Spago's watch mode: What's the best channel for this interactive feature? Right now, watch mode's messaging is directed to stderr. This makes sense -- it's not compiler output; it's something else entirely. But at the same time, it doesn't seem like the diagnostic metadata of Jesse Storimer either; rather, it looks like the console interface of a running program. stderr is probably the better choice between it and stdout, but I wonder if, for command-line interactivity, this design decision shouldn't be second-guessed.
@matthew-hilty
chances of contention between the two, present or future, are reduced and, at worst, any such conflict likely resolved within a release cycle
That's the hope, and in this case it looks like this should be fixed in 0.14
does this pull request have any parts that could still be of use? Are there featurelets that I can strip out and resubmit?
Yes! The --quiet and --no-color flags are more than welcome and I'd like to merge them in. Could you open a new PR just with them so we can keep the discussion about the output-stream here?
A related question about Spago's watch mode: What's the best channel for this interactive feature?
If I understand correctly: you'd like to move the interactive prompts for the watch mode to stdout, right? I think this is the right thing to do (for the reasons you mentioned) and I'd welcome a PR for this change :slightly_smiling_face:
Awesome! Thanks, @f-f. I'll get right on opening a new PR for the --quiet and --no-color flags.
As for the watch mode...
If I understand correctly: you'd like to move the interactive prompts for the watch mode to
stdout, right? I think this is the right thing to do (for the reasons you mentioned)
Actually, I wasn't too sure. Ideally, the interactive prompts should be moved to stdout, I think. However, once json errors are sent to stdout in 0.14, if the interactive prompts are also on stdout, processing json errors downstream while in watch mode would be harder.
Leaving the prompts on stderr seems counter to stderr's purpose, and users wouldn't have the option of suppressing logging, but maybe it's the better choice. I just wanted to bring it up as something to consider.
@matthew-hilty it looks to me that there's consensus on fixing the issue in the compiler being the way forward for now - should we close this PR?
I think your points about the logging messages while in watch mode are good, so feel free to move those to a new issue so we can track that separately