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

Use Fluttered structured error events (behind preview flag) #1870

Merged
merged 8 commits into from
Jul 18, 2019

Conversation

DanTup
Copy link
Member

@DanTup DanTup commented Jul 17, 2019

@jacob314 @InMatrix @devoncarew I've made some progress on this, trying to make them look like IntelliJ's (so please let me know if they change - for ex the yellow ascii banner - so I can copy).

Two of the examples in @InMatrix's repo don't look too bad:

Screenshot 2019-07-17 at 12 58 56 pm

Screenshot 2019-07-17 at 12 59 42 pm

However the Material one looks really bad:

Screenshot 2019-07-17 at 1 00 34 pm
Screenshot 2019-07-17 at 1 00 42 pm

I think probably I'm rendering more nodes than I should (though it's not clear how I should decide which to show), but also some of it's just in the source (like the top line is like this in one big string). How should that be handled?

  • Also needs tests

@DanTup DanTup added is enhancement in flutter Relates to running Flutter apps in debugger Relates to the debug adapter or process of launching a debug session labels Jul 17, 2019
@DanTup DanTup added this to the v3.3.0 milestone Jul 17, 2019
@DanTup
Copy link
Member Author

DanTup commented Jul 17, 2019

Also, there seems to be a race at startup - if an error occurs in startup code, it may be before structuredErrors has been toggled on. As far as I can tell, the service extension doesn't load until after I resumed the isolate, so I don't know if there's a way to avoid this?

@devoncarew
Copy link
Contributor

Some drive-by-comments:

  • we'll need to adjust the ◢◤◢◤◢◤◢◤◢◤◢◤ chars (used by IntelliJ as well) to some other ansi chars; doesn't have to be for this PRR, but those chars are used by Flutter to signify layout overflow
  • after a summary line, you'll want to output a single blank line
  • IntelliJ is currently not printing out nodes where type == ErrorSpacer
  • we don't recurse into child nodes if style == shallow (we still show local properties)

The impl. for IntelliJ is all at https://github.com/flutter/flutter-intellij/blob/master/src/io/flutter/logging/FlutterConsoleLogManager.java#L135.

@devoncarew
Copy link
Contributor

As far as I can tell, the service extension doesn't load until after I resumed the isolate, so I don't know if there's a way to avoid this?

I've seen this race as well, but don't have a good solution.

@DanTup
Copy link
Member Author

DanTup commented Jul 17, 2019

Thanks for the notes! I had been following the IntelliJ implementation but I couldn't figure out all the differences (I suck at reading Java :-)).

I'll make those changes - and I'll update the banner to match whatever you do in IntelliJ once you've made a decision about what to do there.

@DanTup DanTup merged commit 8c2383f into master Jul 18, 2019
@DanTup DanTup deleted the structured-errors branch July 18, 2019 15:54
@FrobtheBuilder
Copy link

Oh my god, thank you, it was so utterly aggravating to have the console filled up with literally hundreds of lines of calls you don't care about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in debugger Relates to the debug adapter or process of launching a debug session in flutter Relates to running Flutter apps is enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants