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

Filter debug console outputs by severity #6486

Merged
merged 1 commit into from
Nov 5, 2019
Merged

Filter debug console outputs by severity #6486

merged 1 commit into from
Nov 5, 2019

Conversation

tolusha
Copy link
Contributor

@tolusha tolusha commented Nov 4, 2019

Signed-off-by: Anatoliy Bazko abazko@redhat.com

Reference issue

#6067

What it does

It supports filtering in debug console by severity.

How to test

  1. Add some sample.
  2. Add debug configuration and launch it
  3. Go into debug console and try to filter output.
  4. Close/Open debug console, recheck #3

Review checklist

Reminder for reviewers

screencast-localhost_3000-2019 11 04-11_09_12

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works nicely for me, I am able to filter down the possible message types in the debug console :)

@@ -114,6 +123,50 @@ export class DebugConsoleContribution extends AbstractViewContribution<ConsoleWi
}));
}

protected readonly SEVERITIES = ['Errors', 'Warnings', 'Info', 'Log'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tolusha I'm wondering, why not use the MessageType directly? I believe it would also remove the need to have
messageType + 1 and messageType - 1 in order places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vince-fugnitto
What about MessageType.Progress severity? Should we use it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vince-fugnitto
What about MessageType.Progress severity? Should we use it?

I believe progress is mainly used for notifications, I don't believe it is useful in the context of debugging types. @AlexTugarev am I correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing, but at least odd.
I'd rather derive the type of a message from a common severity than the other way around.

@tolusha and @akosyakov don't you think, this is a good chance to clean it up, i.e. to introduce Severity type in core?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vince-fugnitto vince-fugnitto added console issues related to the console debug issues that related to debug functionality enhancement issues that are enhancements to current functionality - nice to haves labels Nov 4, 2019
@tsmaeder tsmaeder mentioned this pull request Nov 5, 2019
26 tasks
@tolusha
Copy link
Contributor Author

tolusha commented Nov 5, 2019

@vince-fugnitto @AlexTugarev
PR is updated

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code-wise looks good, except mino remarks, testing

packages/console/src/browser/console-session.ts Outdated Show resolved Hide resolved
packages/console/src/browser/console-session.ts Outdated Show resolved Hide resolved
packages/core/src/common/severity.ts Outdated Show resolved Hide resolved
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works pretty well, thank you ❤️

please take care of comments and git history before meting

@tolusha
Copy link
Contributor Author

tolusha commented Nov 5, 2019

All remarks are fixed. I am going to squash commits and merge.

Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
@tolusha tolusha merged commit f2b3556 into master Nov 5, 2019
@tolusha tolusha deleted the ab/severity branch November 5, 2019 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console issues related to the console debug issues that related to debug functionality enhancement issues that are enhancements to current functionality - nice to haves
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants