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

Improve Javac error parsing #557

Merged
merged 2 commits into from Jul 5, 2018

Conversation

Projects
None yet
3 participants
@eed3si9n
Member

eed3si9n commented Jul 5, 2018

Ref #520

This is an improvement over the status quo of forked Javac error parsing,
but likely not a complete fix.

"Normal" Java error messages look like:

/path/a.java:100:1: error message
        if (x.isBar) {
              ^
  symbol: xxx
  location: xxx

However, under certain circumstances javac errors are reported as:

/path/a.java:100:1: error message
symbol: xxx
location: xxx
if (x.isBar) {

Current parsing assumes existence of ^ as well as indentation before "symbol" etc.

Ultimately a better way of handling forking probably is to define our own small commandline app, like forked tests are done.

eed3si9n added some commits Jun 27, 2018

Ref #520
This is an improvement over the status quo of forked Javac error parsing,
but likely not a complete fix.

"Normal" Java error messages look like:

```
/path/a.java💯1: error message
        if (x.isBar) {
              ^
  symbol: xxx
  location: xxx
```

However, under certain circumstances javac reports compiler errors are reported as:

```
/path/a.java💯1: error message
symbol: xxx
location: xxx
if (x.isBar) {
```

Current parsing assumes existence of `^` as well as indentation before "symbol" etc.

Ultimately a better way of handling forking probably is to define our own small commandline app, like forked tests are done.

@eed3si9n eed3si9n requested review from jvican and dwijnand Jul 5, 2018

@jvican

jvican approved these changes Jul 5, 2018

Looks good, but a little bit complicated. Thinking: does this approach complement with the complete fix? If it doesn't, maybe we should invest some time in implementing forked java compilation the same way it's done with forked tests (I have no idea how much this would cost, that's why I'm asking).

@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Jul 5, 2018

Member

If I can unblock the customer I am working with, I'd like to get this in soon.
It's difficult to estimate how complicated the commandline app approach would be, but I'm thinking it would be weeks away not days. I'll probably create a issue for it, and kick it till later.

Member

eed3si9n commented Jul 5, 2018

If I can unblock the customer I am working with, I'd like to get this in soon.
It's difficult to estimate how complicated the commandline app approach would be, but I'm thinking it would be weeks away not days. I'll probably create a issue for it, and kick it till later.

@jvican

This comment has been minimized.

Show comment
Hide comment
@jvican

jvican Jul 5, 2018

Member

Good, this is good to go on my side.

Member

jvican commented Jul 5, 2018

Good, this is good to go on my side.

@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Jul 5, 2018

Member

Thanks guys.

Member

eed3si9n commented Jul 5, 2018

Thanks guys.

@eed3si9n eed3si9n merged commit 4d3ed1d into sbt:1.x Jul 5, 2018

1 check passed

continuous-integration/drone/pr the build was successful
Details

@eed3si9n eed3si9n deleted the eed3si9n:wip/error branch Jul 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment