Fix userwindows not going away on profile close#2959
Fix userwindows not going away on profile close#2959vadi2 merged 1 commit intoMudlet:developmentfrom
Conversation
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
Kebap
left a comment
There was a problem hiding this comment.
Thanks for fixing the crash!
| auto hostName(pHost->getName()); | ||
| auto pC = pHost->mpConsole->mSubConsoleMap.value(name); | ||
| auto pD = pHost->mpConsole->mDockWidgetMap.value(name); | ||
| auto console = pHost->mpConsole->mSubConsoleMap.value(name); |
There was a problem hiding this comment.
Can I make the observations that:
- the loss of the initial
pis disguising (as far as I am concerned) that this and the next variable are pointers - the noise from the variable renaming is hiding any changes in the functionality - hell, it isn't even obvious that anything has changed in this method...
There was a problem hiding this comment.
Valid points, but - a good Git client shows that only the variable was renamed in every line. Github's doesn't unfortunately, and if you confuse that the value is not a pointer, the compiler will tell you very fast otherwise, so no mistake can be made there.
| connect(dactionShowMap, &QAction::triggered, this, &mudlet::slot_mapper); | ||
| connect(dactionOptions, &QAction::triggered, this, &mudlet::slot_show_options_dialog); | ||
| connect(dactionAbout, &QAction::triggered, this, &mudlet::slot_show_about_dialog); | ||
| // PLACEMARKER: Save for later restoration (2 of 2) (by adding a "Close" (profile) option to first menu on menu bar: |
There was a problem hiding this comment.
👎 I do want to put that close option (back?) in - IMHO it logically make sense, though it will need some checking that the Application does return to the state prior to the initial "Connect Dialog" appearance on start-up. Remember when running a single profile there is not method currently to close the profile without terminating the application (perhaps we might want to use <CTRL>+<F4>) even though that is not an unreasonable way of operating.
OTOH for consistency I do not see you deleting the other // PLACEMARKER: Save for later restoration... for this, even though I made it clear that there is two of them! 😮
There was a problem hiding this comment.
It can be re-added when needed later on, it's always available in Git history!
| dockWindowMap.remove(consoleName); | ||
| } | ||
|
|
||
| hostConsoleMap[consoleName]->close(); |
There was a problem hiding this comment.
Surely calling QDockWidget::close() when the Qt::WA_DeleteOnClose property is set on it will automagically destroy the TConsole instance it contains {and if not, why not?} - so shouldn't the hostConsoleMap[consoleName]->close(); be in an else to the if - though the hostConsoleMap.remove(consoleName); should remain outside of the if ? 🤓
There was a problem hiding this comment.
The entire reason for this bug is because the TConsole is not destroyed automatically when the QDockWidget is, so this solution works well.
* Initial commit. Adds ctrl+tab shortcut and logic for moving from pane to pane in the trigger editor. * Shortcut works without menu action. Forward tabbing logic seems to be right. Just missing reverse shortcut and logic. * Added CTRL+SHIFT+Tab shortcut to tab through sections in reverse order. * Removed unnecessary debug code. * Factor out drawing exits in the map into a separate method (#2941) * WIP * Const the variables that we can * Format * Reorganize windows sdk (#2958) * Move declaration of install functions This allows us to use them singularly in a different context. * Use our own edbee with updated QsLog (#2914) Intention is to send all improvements upstream but not be delayed by them getting merged. * Fix userwindows not going away on profile close (#2959) * Use correct URL to download the coverity software * Don't show SSL certificate info until we have one in Profile Preferences (#2953) * Don't show SSL certificate info until we have one & streamline UI * Remove colorroles * Add a workaround for mapper creation breaking getMainWindowSize() (#2945) * Add a workaround for mapper creation breaking getMainWindowSize() * Revert "Fix errors when creating new triggers (#2868)" This reverts commit 0578c61. * Fix typo * Don't include TLS code code when it's not available (#2965) * Raise minimum to Qt 5.11 (#2944) * Raise minimum to Qt 5.11 * Update Travis minimum Qt build * Remove < Qt 5.11 workarounds (#2969) * Replace a couple of cases of std::tie with auto (#2967) * Replace a couple of cases of std::tie with auto * A few more * Fix Coverity TTimer mRepeating initialiser warning (#2972) * Fix updateDialog uninitialised in constructor (#2973) * Fix nullptr warning around curVar (#2974) * Fix a possible nullptr dereference if the room wasn't found for some reason (#2975) * getRoom() will return nullptr if the room id is negative (#2976) * Add autocomplete toggle (#2679) * Update tooltip, more than IRE now provide maps (#2668) * Add autocomplete toggle to profile * Update to a version of edbee with autocomplete toggle * Wire in the autocomplete toggle * Use correct host variable * Add missing toggle call * Formatting changes up simplification of some code. * Added closing bracket that I must have accidentally deleted while resolving conflicts.
Brief overview of PR changes/additions
Fix userwindows not going away on profile close - the console they were belonging to was getting removed, but not the dock widget the console was in.
Motivation for adding to Mudlet
Bugfix.
Other info (issues closed, discussion etc)
Fix #2916.