-
Notifications
You must be signed in to change notification settings - Fork 31.9k
Make channel log level settable from output view #205159
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
Conversation
@gjsjohnmurray Thanks for contributing the PR. I liked the new behaviour. I have one feedback
|
@sandy081 as a (sub)menu I don't think we can achieve both functions of the quickpick, in which you can set the log level for the channel until the next restart and the default level that will apply after restarts (the double checkmark on the right-hand end of a level). Assuming the (sub)menu item picked will get a single checkmark and set the level only for the current session, maybe the initial button / overflow menu item should be captioned "Set Current Log Level..." rather than how they appear in my previous screenshots ("Configure Log Level..."). |
That's right, we would need two actions
I would make first action that can be able to made a primary action and second action just a secondary action. |
@sandy081 this is ready for your review. It now uses a submenu, but instead of adding a second submenu for setting the channel's default level I added an option at the end of the submenu which updates the default to match the current choice. This option is only enabled when the default differs from the current level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If that's the case, you would need to find all occurrences in the code where createInstance is called with the channelId and remove that argument. If the channelId was providing necessary context for the SetLogLevelAction, you'll need to find a new way to provide that context or reconsider the removal of the channelId parameter.
This PR closes #162262
Choosing the option takes you directly to the second quickpick presented by the existing
Developer: Set Log Level...
command:Like all entries on this overflow menu the user can promote the new option to be a button:
For channels that don't support log levels the option gets disabled rather than hidden, so it matches the behaviour of existing overflow options, and so when shown as a button the layout doesn't change as you scroll through channels: