Implementation of global messages

[pgScorpio]

Short description of changes New implementation of the messageboxes part of #2583

CHANGELOG: Messageboxes will show up related to the instance instead of at system default position and they will have 'clientname' in the window title. In headless mode the messages will appear on the terminal.

Context: Fixes an issue?

General improvement and preparation for sound-redesign.

Does this change need documentation? What needs to be documented and how?

No documentation changes

Status of this Pull Request

Waiting on review

What is missing until this pull request can be merged?

• [ ] Review

Checklist

• [x] I've verified that this Pull Request follows the general code principles
• [x] I tested my code and it does what I want
• [x] My code follows the style guide
• [x] I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
• [x] I've filled all the content above

[ann0see]

Great! Thank you!

[ann0see]

Could you please add a CHANGELOG entry in your PR description (and a short reasoning why you need this + advantages e.g uniform way to show error messages in future)

[pgScorpio]

Could you please add a CHANGELOG entry in your PR description (and a short reasoning why you need this + advantages e.g uniform way to show error messages in future)

Done. Please let me know if it's ok this way.

[pgScorpio]

Questions:

Do we want a "Jamulus [clientname]" prefix to appear on the debug steams too? (Personally I think this might be usefull.)

Would it be useful to be able to set the button text from the "ShowXxxx" calls (optional parameter defaulting to "Ok")? (Personally I think we shouldn't for consistency, since different text's might give the expectation of different behavior.)

[ann0see]

My main concern is that if we replaced all qWarning/qInfo calls with the new abstraction, we might start showing solely informational messages as dialogs accidentially?

I mean you could also add a boolean parameter to only show it via CLI if needed.

[ann0see]

Would it be useful to be able to set the button text from the "ShowXxxx" calls (optional parameter defaulting to "Ok")?

Overload the methods to add the ability to do that?

[pgScorpio]

I'm less sure about the headless stuff. My gut feeling is that there are not that many instances where there's duplicated error handling for UI/CLI stuff, but I might be wrong.

There are several places places in the code with ifndef HEADLESS QMessage #else qStream #endif

The QMessageBox class would make those ifdefs unnecessary. Also there are some places with only ifndef HEADLESS QMessage #endif where a message in headless mode would be useful too. So my idea is that only if you explicitly don't want the message on the terminal in headless mode you should now use an ifdef, since I think there are very few occasions where you would not want to show errors/warnings/info in headless mode if there is a good reason to show them in ui mode.

My main concern is that if we replaced all qWarning/qInfo calls with the new abstraction, we might start showing solely informational messages as dialogs accidentially?

I'm not sure if I really understand this remark. But we have Error, Warning and Info messageboxes. We would just replace all QMessagebox::xxxx calls with the matching CMessageBox::ShowXxxx call.

PR note: It seems like this PR adds and includes the infrastructure (src/messages) and updates one caller place. If we decide to add it, it would make sense to update all callers in the same go (separate commit, but same PR).

Sure !, but I did that in the other PR and I was requested NOT to do that again this PR but in a separate PR after this one is merged....

(Maybe there was some confusion about that in the other PRs regarding what to include. A PR should do one thing. If it aims to improve the message box implementation, it should include all of that, even if that touches lots of different files. It's just that all those changes should be related to the same specific goal.)

So I think there is still some difference in opinion on that. Just let me know what we agree upon, new commit or new PR doesn't make that much of a difference to me.

[pgScorpio]

Would it be useful to be able to set the button text from the "ShowXxxx" calls (optional parameter defaulting to "Ok")?

Overload the methods to add the ability to do that?

There are several ways to do it, but as said another button text would only seem useful to me if we take specific actions after the message is shown, so that would need another kind of (modal) message box that waits for the button to be pressed. So I would rather implement those kind of message boxes too and those would then have a (optional?) button text parameter. But AFAIK there are just one or two places in the current code where this would be applicable. (and one occurrence would be disappearing after the sound redesign anyway.)

[ann0see]

I think the idea is still valuable. However to keep the PR view clean, I'll close this for now.