Skip to content

Add an option to show errors view from the toolbar#5511

Merged
vadi2 merged 7 commits intoMudlet:developmentfrom
vadi2:show-errors-in-top-toolbar
Oct 15, 2021
Merged

Add an option to show errors view from the toolbar#5511
vadi2 merged 7 commits intoMudlet:developmentfrom
vadi2:show-errors-in-top-toolbar

Conversation

@vadi2
Copy link
Copy Markdown
Member

@vadi2 vadi2 commented Oct 13, 2021

Brief overview of PR changes/additions

Add an option to show errors

Motivation for adding to Mudlet

People miss the 'errors' window very often - possibly because on smaller displays you have to expand the bar in order to see it. This PR adds another point of discoverability.

Other info (issues closed, discussion etc)

Continues on #5293

@vadi2 vadi2 requested a review from a team as a code owner October 13, 2021 16:38
@vadi2 vadi2 requested a review from a team October 13, 2021 16:38
@add-deployment-links
Copy link
Copy Markdown

add-deployment-links bot commented Oct 13, 2021

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 13, 2021

Messages
✔️

PR type: Addition

Generated by 🚫 dangerJS against 6e82753

@github-actions
Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Copy Markdown
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

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

As well as a Seg. Fault opportunity (:bomb: ) that needs to be defused, the sub-module updates should be done in one (or more) separate PR(s) I think...

</property>
</action>
<action name="dactionInputLine">
<property name="text">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This hunk is just noise (Qt Designer plug-in/utility randomly reordering XML elements) and doesn't really need to be included.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is, but it's not worth the time and effort to be re-arranging XML files that'll have no visible effect for the user for me

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we should use QtDesigner to rearrange them all and forgot about this issue :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is that tool that is the bloody problem. And, no, you can't fix the ordering with it.... 🙄

FTR it isn't even sensitive to correct Qt Versions - it will quite happy insert new *attributes^ that come in the Qt Version that it was built for (there is no "Qt Version(s) to build the .ui file for" option) - which causes the uic from older versions to died screaming about stuff it cannot grok - which is horrible if your project (like, say, Mudlet) tries to be compatible with a range of Qt versions.

We last ran into this nightmare with the addition of a color role for PlaceholderText which clobbered something that Vadi was working on for Mudlet 4.4.0 not only once in c701ab6 but again in 53941a7 a few days later. This element was introduced in Qt 5.12 but since we (still) support building on Qt 5.11 (I'm still using that for system Qt builds on my Devuan 3 "Buster" - comparable to Debian 10 "Bullseye" systems) one must be careful not to tickle Qt Designer to cause it to regenerate colour palettes in a .ui file otherwise it stuffs that bit back in and breaks the Qt 5.11 build for our CI and anyone compiling on a less than bleeding edge distribution...

}
host->mpEditorDialog->slot_show_current();
host->mpEditorDialog->raise();
host->mpEditorDialog->showNormal();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤔 If a user has the editor window opened full screen/maximised on a second (or higher) monitor than this will undo either of those choices - whilst that might be the case for only a very small set of users doing this might not be the optimum design...

Copy link
Copy Markdown
Member

@SlySven SlySven Oct 15, 2021

Choose a reason for hiding this comment

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

Instead we just need to clear the Qt::WindowMinimized flag if it is set - to de-iconify the window to whichever of normal, maximised or full-screen was previous set (and not try to set the Qt::WindowActive as that is not something that we should do according to the Qt Documentation):

Suggested change
host->mpEditorDialog->showNormal();
host->mpEditorDialog->setWindowState(host->mpEditorDialog->windowState() &~(Qt::WindowMinimized|Qt::WindowActive));

Humm, that almost works - except that the editor window does not get shown when this is called upon to create it in the first place.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That doesn't sound like an improvement, then!

if (!host) {
return;
}
host->mpEditorDialog->slot_show_current();
Copy link
Copy Markdown
Member

@SlySven SlySven Oct 14, 2021

Choose a reason for hiding this comment

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

Off-topic for this PR but:

I've just checked and dlgTriggerEditor::slot_show_current() is not connect(...)ed up to any signals (this is only one of two usages) so it can loss the slot_ prefix and it's declaration be moved in the header file...

@vadi2 vadi2 merged commit 4166db0 into Mudlet:development Oct 15, 2021
@vadi2 vadi2 deleted the show-errors-in-top-toolbar branch October 15, 2021 05:55
@vadi2 vadi2 changed the title Add an option to show errors Add an option to show errors view from the toolbar Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants