Skip to content

Commit

Permalink
Remove odd line in compile errors (#488)
Browse files Browse the repository at this point in the history
Before:

```
❯ ../bin/elm-test
-- UNFINISHED DEFINITION - /Users/lydell/forks/node-test-runner/example-application/tests/TestsPassing.elm

I got stuck while parsing the `x` definition:

3| x
    ^
I was expecting to see an argument or an equals sign next.

Here is a valid definition (with a type annotation) for reference:

    greet : String -> String
    greet name =
      "Hello " ++ name ++ "!"

The top line (called a "type annotation") is optional. You can leave it off if
you want. As you get more comfortable with Elm and as your project grows, it
becomes more and more valuable to add them though! They work great as
compiler-verified documentation, and they often improve error messages!

Compiling >
`elm make` failed with exit code 1.
```

After:

```
❯ ../bin/elm-test
-- UNFINISHED DEFINITION - /Users/lydell/forks/node-test-runner/example-application/tests/TestsPassing.elm

I got stuck while parsing the `x` definition:

3| x
    ^
I was expecting to see an argument or an equals sign next.

Here is a valid definition (with a type annotation) for reference:

    greet : String -> String
    greet name =
      "Hello " ++ name ++ "!"

The top line (called a "type annotation") is optional. You can leave it off if
you want. As you get more comfortable with Elm and as your project grows, it
becomes more and more valuable to add them though! They work great as
compiler-verified documentation, and they often improve error messages!


`elm make` failed with exit code 1.
```

Notice the `Compile >` line near the bottom in the “Before” output. That’s not supposed to be there.

I fixed this by reverting the change here: #480 (comment)

This is pretty tricky. I’ll try to explain.

So we print this `Thing > Other thing > Last thing` progress line, where each segment appears over time. For this reason, the terminal cursor never moves to the next line, so we can overwrite the line over and over. Once done, we move to the next line so the test output can print below the (finished) progress line.

But what happens if there’s an error? We don’t want to end up with something like:

```
Thing > Other thingError: Can’t do this and that for:
more lines of error
Maybe try this and that?
```

So we want to move to the next line when printing an error as well. `progressLogger.logLine('')` worked for this case most of the time: It would reprint `Thing > Other` (which technically was unnecessary but couldn’t be seen) and then moved to the new line. Another thing I noticed while writing this: The empty string would actually result in `This > Other > ` (an unwanted trailing `>` separator).

But the thing is that we can’t actually know that the cursor is still on the cursor line. Why? Because `elm make` might have printed to stderr (in case of compilation errors in the user’s code). We don’t know if it has either – we can’t capture its stderr because then we would lose colors.

So a solution that works in both cases is to _not_ reprint the progress line and instead only move to the next line. (That results in an extra blank line for the compiler errors but I don’t think it matters.)
  • Loading branch information
lydell authored Jan 22, 2021
1 parent 995b916 commit a211d34
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions lib/RunTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ function makeProgressLogger(report /*: typeof Report.Report */) {
process.stdout.write(`${items.join(' > ')}\r`);
}
},
logLine(message) {
this.log(message);
newLine() {
items.length = 0;
if (!Report.isMachineReadable(report)) {
process.stdout.write('\n');
Expand Down Expand Up @@ -240,7 +239,8 @@ function runTests(

Generate.prepareCompiledJsFile(pipeFilename, dest);

progressLogger.logLine('Starting tests');
progressLogger.log('Starting tests');
progressLogger.newLine();

return await Supervisor.run(
packageInfo.version,
Expand All @@ -251,7 +251,7 @@ function runTests(
watch
);
} catch (err) {
progressLogger.logLine('');
progressLogger.newLine();
console.error(err.message);
return 1;
}
Expand Down

0 comments on commit a211d34

Please sign in to comment.