Resolve "Add app setting to change logging level at runtime"

mariusalexander commented 4 months ago

Description

Closes #1086.

Added setting to change logging level at runtime. It will remember selection, however, arguments like --debug or --dev will override this setting but only for the duration of the session. If the logging level is changed in the output dock the logging level is changed permanently to the overridden logging level.

grafik

How Has This Been Tested?

Manually.

Checklist:

[ ] A test for the new functionality was added (if applicable).
[ ] All tests run without failure.
[ ] The changelog has been extended, if this MR contains important changes from the users's point of view.
[ ] The new code complies with the GTlab's style guide.
[ ] New interface methods / functions are exported via EXPORT. Non-interface functions are NOT exported.
[ ] The number of code quality warnings is not increasing.

jensschmeink commented 4 months ago

I like the new function but I have some comments:

It is a bit missleading that the buttons to display messages in the output dock stay in the widget. I know they can be used to filter old existing messages but it feels a bit strange to see the buttons now even in non-dev mode.
The names of the logging levels are not so intuitive for me

mariusalexander commented 4 months ago

It is a bit missleading that the buttons to display messages in the output dock stay in the widget. I know they can be used to filter old existing messages but it feels a bit strange to see the buttons now even in non-dev mode.

Agree, it might be misleading. The reason I have not hidden these entries is because there is no easy way of updating the visibility of these buttons. For example, say you start GTlab in user mode (so only info and higher is logged), then there will be filter buttons for info, warning and error. If you now change the logging level in the settings to say Trace level, there is (currently) no way of telling the output dockwidget, that the logging level has changes, thus that the trace and debug filter buttons (which should appear) cannot be shown (and vice versa).

I did not want to add a signal, say to the GtCoreApplication, but maybe we have to? It is your call

And now that you mention it: What if trace messages are currently in the logging history, but now the logging level is changed to User-mode. What should happen to the trace filter button?

The names of the logging levels are not so intuitive for me

I could add a tooltip. What would you suggest? I think Trace-Level, Debug-Level are quite fitting. We could replace User-Level with Default or Normal...?

rainman110 commented 4 months ago

It is a bit missleading that the buttons to display messages in the output dock stay in the widget.

I agree, that was also the first thing that came up to me when testing it.

Also, I was wondering, what "user-level" is. So it seems to print the "information" messages. Why not rename it "Addition Info"

rainman110 commented 4 months ago

Maybe instead of adjusting the logging in the settings, it could be a combo box in the logging / output widget itself, e.g.:

Then, you can probably improve the communication between the combo box setting and muting the filter buttons.

mariusalexander commented 4 months ago

Maybe instead of adjusting the logging in the settings, it could be a combo box in the logging / output widget itself, e.g.:

Is the screenshot edited or how did you place it there? I like the idea to place, but I cannot imagine how I could achieve to place it there @rainman110. If we place it in the output dock I would like to make it take up less space, since the output dock is already quite noisy.

Also, I was wondering, what "user-level" is. So it seems to print the "information" messages. Why not rename it "Addition Info"

"Additional Info" does not sit right with me as well. The Info-Level is the main logging level to be used by the user. Maybe I'll go with Default or Informative

rainman110 commented 4 months ago

Is the screenshot edited or how did you place it there? I like the idea to place, but I cannot imagine how I could achieve to place it there @rainman110. If we place it in the output dock I would like to make it take up less space, since the output dock is already quite noisy.

It was just photoshopped. More like a general idea. It does not need to be exactly there though. I could imagine, that this is more user friendly. I could definitely take less space there.

mariusalexander commented 4 months ago

I could do it like this: grafik

grafik

rainman110 commented 4 months ago

Looks good to me 👍

mariusalexander commented 4 months ago

Okay wait... Would you look at that, I found a way! @rainman110

grafik

rainman110 commented 4 months ago

Okay wait... Would you look at that, I found a way! @rainman110

Wow cool 😄 I like it 🥳

rainman110 commented 4 months ago

How did you do it?

mariusalexander commented 4 months ago

So, obviously you can add a widget to a layout of the "main" widget. The layout will take care of resize events and general placement as far as I know. However, you can also simply set the parent of the child widget to the main widget. This will make it appear at (0, 0) which is the top left. Since I needed the widget to be on the far right side I could move it manually but then it would not handle resize events correctly. So I had a look online and found out that QGridLayout allows you to set the specific row and column to place a widget and that you can stack multiple widgets that way when using the same row and column. Thats exactly what I did.

mariusalexander commented 4 months ago

Sorry I miss-clicked and closed the PR 😂

mariusalexander commented 4 months ago

Oh I have noticed that I still need to make the buttons update when changing the logging level. What should happen e.g. if I have Trace messages in the output but then I switch the logging level to Debug? Should the trace messages be deleted and the trace filter button be hidden? @rainman110 @jensschmeink

jensschmeink commented 4 months ago

Oh I have noticed that I still need to make the buttons update when changing the logging level. What should happen e.g. if I have Trace messages in the output but then I switch the logging level to Debug? Should the trace messages be deleted and the trace filter button be hidden?

Good question. Is it an option to check the existance of those messages and hide/show the button depending on this?

mariusalexander commented 4 months ago

Sure, that is possible too

rainman110 commented 4 months ago

I would probably just hide the buttons but keep the state of the buttons. Thus, the old messages stay as they are but no new trace messages will appear.

But I also like you idea to decide based on the current history, whether to show the buttons or not.

mariusalexander commented 4 months ago

@rainman110 Finished this PR, please review.

I have implemented it so that each time the logmodel deletes a row it is checked if each filter button (only trace, debug and info are affected) are visible and should be hidden depending on the logging level. Only if the buttons are visible and if they should be hidden, it is checked whether the log model contains the corresponding logging level. If not, the button is hidden else it won't.

mariusalexander commented 4 months ago

@rainman110 I have integrated your suggestions. Both the constructor of the app settings page and the output dock are now a lot cleaner 👍🏽 But the diff is now a bit messy... Can you have another look?

rainman110 commented 4 months ago

@mariusalexander Please avoid merging, before the CI has passed. It is currently running for you two latest commits.

mariusalexander commented 4 months ago

@mariusalexander Please avoid merging, before the CI has passed. It is currently running for you two latest commits.

I see, sorry 😇

dlr-gtlab / gtlab-core