Diagnostics Manager: Change to Auto-Apply, and related safety features
Bug Number: 5290
Release notes (y/n): Yes
For Release Nums: 6.3
Gnats 5290: Diagnostics are still being posted to the black command window.
Even with the enhancements introduced in addressing Gnats 5273, where the 'To Window' option is forced ON if Optional Diagnostics are enabled and 'To File' is off (in the course of the operation of the Diagnostics Manager dialog), there were still a couple ways in which this desirable state correction was not effected. And this causes diagnostics to be dumped to a Windows console -- a dangerous outcome, because dismissing such a console ends the RiverWare session! This undesirable state was still possible (with the Gnats 5273 changes) in these two ways:
(1) In an interactive session where the Diagnostics Manager dialog had not been used.
(2) Confoundingly ... even when that dialog is used, if the user dismissed the dialog with the 'Cancel' or the Window title-bar 'X' button instead of clicking 'Apply' or 'OK'.
The implemented solution to (#1) above is to force Window Output ON within the lower-level internals of the diagnostic mechanism when Optional Diagnostics is enabled and neither Window Output nor File Output had been enabled. This occurs after a model load -- ONLY for interactive (non-batch) RiverWare sessions. [See change in Sim/Diag.cpp].
For problem (#2) above, we had discussed replacing the OK/Apply/Cancel buttons with just a 'Close' (or 'OK') button, and implementing and relying upon an automatic 'Apply' when the dialog was closed. However, we realized that the automatic 'Apply' needed to occur also if the user refrained from closing the dialog (i.e. just kept it open) and started a run. Various approaches to accommodate this latter requirement were explored, and have fundamental problems -- See the 'NOT IMPLEMENTED' section below.
Even with Auto-Apply implemented (which is done -- on the closure of the dialog), without being able to satisfy the latter requirement, it's dangerous to hide from the user the fact that somehow an 'Apply' needs to occur -- just leaving the dialog there without dismissing it doesn't do the trick. For this reason, the current solution is to show 'OK' and 'Apply' buttons, but not a 'Cancel' button. Both buttons effect an 'Apply' -- and cause an error dialog to be shown if there is a problem with an enabled File Output path. The 'OK' button also closes the dialog IF there is no error.
This really is OK. The main problem was allowing the user to dismiss the dialog without having applied the changes -- and that has been addressed.
The following changes have been implemented:
(1) As described above ("#1"), the "To Window" state is forced on when it is needed -- after model loads in an interactive session.
(2) The visible settings are applied when the dialog is closed. The 'OK' and 'Apply' buttons remain, but the 'Cancel' button has been removed.
(3) Being that the 'Apply' operation can fail if File Output is enabled but the provided file path is invalid, the closing of the window is prevented -- and an error dialog is shown -- in that failure scenario. This applies to both clicking the 'OK' button AND the Window title-bar 'X' button. [Technical: In DiagMgrDlg, we conditionally reject QCloseEvents in an overridden implementation of QWidget::closeEvent()].
(4) FIX: When diagnostics are turned on in one of the five Configuration Settings dialog, and applied there, that forces the Diagnostic Manager Dialog's "Enable Information Diagnostics" checkbox ON. That was working, but when this happens, the conditional forcing-ON of the "To Window" checkbox (implemented for Gnats 5273) was not happening. That's been fixed.
(5) TECHNICAL FIX: There was a note in code about refraining from deleting this singleton dialog when it closed because of an unidentified crash. The source of that crash (had delete-on-close been re-enabled) was identified and fixed: needing to wipe out the singleton instance pointer in the destructor. However, there is a legitimate reason for not deleting this dialog when it closes: its relationship to its child "Settings" dialogs.
CONSIDERED CHANGES NOT IMPLEMENTED because of some unanticipated technical complexities.
(1) In order to prevent the user from starting a run without ever causing an 'Apply' of the diagnostic settings by LEAVING OPEN the Diagnostics Manager and clicking the Run button elsewhere -- we would like to force an 'Apply' (and validity check with error report popup) ideally when the user clicks outside of the dialog box -- or alternatively at least when the File Path editor QLineEdit loses focus. I couldn't get either approach to work without undesirable GUI side-effects. For one thing, using loss-of-focus of the QLineEdit (the latter approach) works quite badly when the user just tries to turn the "To File" toggle off (to bail from the bad file path) or fix the file path by pressing the file chooser button.
(2) In the failure case (File Output enabled, but bad file path), it's probably a mistake to automatically disable File Output (and turn off that checkbox). The user may miss the fact that that has happened (even though there is a message dialog saying so). It should really be up to the user to either disable File Output OR pick a valid, usable file path. UNFORTUNATELY, the error conditions are discerned IN THE PROCESS OF opening the output stream, and this change gets us into the situation of having to open the output stream which may already be open. Without a rewrite of the error-detection code, this works out really badly.
---