From 2e3f0d95ddc1355318e6deac850db4a0a1a29d24 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 20 Oct 2017 02:36:01 +0200 Subject: [PATCH 1/3] Open debugger at assert location from the GUI assert handler too This should have been part of 55fd62c1e370bef69657f220ddd591743e842be8 which only updated the default assert handler, but not the one used by default in all GUI applications, for some reason, see there for more explanations. Do this now to ensure that after pressing "Yes" in the assert failure dialog, the debugger opens at the assert location and not deep inside wxWidgets code. See #11184. --- src/common/appcmn.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/common/appcmn.cpp b/src/common/appcmn.cpp index 0379985eba..16c79481e7 100644 --- a/src/common/appcmn.cpp +++ b/src/common/appcmn.cpp @@ -489,7 +489,9 @@ bool wxGUIAppTraitsBase::ShowAssertDialog(const wxString& msg) wxYES_NO | wxCANCEL | wxICON_STOP ) ) { case wxYES: - wxTrap(); + // See the comment about using the same variable in + // DoShowAssertDialog(). + wxTrapInAssert = true; break; case wxCANCEL: From c52ed1aff3301c396f5a597adf66bc7944a76330 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 20 Oct 2017 02:38:14 +0200 Subject: [PATCH 2/3] Make "No" button default in the assert dialog This makes more sense than the default default (sic) "Yes" button, pressing which accidentally could kill the program if not running under the debugger. --- src/common/appbase.cpp | 2 +- src/common/appcmn.cpp | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/common/appbase.cpp b/src/common/appbase.cpp index 63512ccd38..b885f7d992 100644 --- a/src/common/appbase.cpp +++ b/src/common/appbase.cpp @@ -1307,7 +1307,7 @@ bool DoShowAssertDialog(const wxString& msg) wxT("further warnings."); switch ( ::MessageBox(NULL, msgDlg.t_str(), wxT("wxWidgets Debug Alert"), - MB_YESNOCANCEL | MB_ICONSTOP ) ) + MB_YESNOCANCEL | MB_DEFBUTTON2 | MB_ICONSTOP ) ) { case IDYES: // If we called wxTrap() directly from here, the programmer would diff --git a/src/common/appcmn.cpp b/src/common/appcmn.cpp index 16c79481e7..4c93db8584 100644 --- a/src/common/appcmn.cpp +++ b/src/common/appcmn.cpp @@ -485,8 +485,11 @@ bool wxGUIAppTraitsBase::ShowAssertDialog(const wxString& msg) wxT("You can also choose [Cancel] to suppress ") wxT("further warnings."); + // "No" button means to continue execution, so it should be the default + // action as leaving the "Yes" button the default one would mean that + // accidentally pressing Space or Enter would trap and kill the program. switch ( wxMessageBox(msgDlg, wxT("wxWidgets Debug Alert"), - wxYES_NO | wxCANCEL | wxICON_STOP ) ) + wxYES_NO | wxCANCEL | wxNO_DEFAULT | wxICON_STOP ) ) { case wxYES: // See the comment about using the same variable in From a1a3efe03b42f3fee993d8eef1319f23e6dd0e94 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 20 Oct 2017 02:40:17 +0200 Subject: [PATCH 3/3] Use wxRichMessageDialog for showing assertion failures This allows to hide the long (and possibly not fitting on the screen) call stack by default to avoid intimidating people not used to it and provides a much more clear way to ignore the subsequent asserts, by clicking a dedicated checkbox instead of having to choose the "Cancel" button which didn't make much sense. See #15430. --- src/common/appcmn.cpp | 91 ++++++++++++++++++++++++++++++------------- 1 file changed, 65 insertions(+), 26 deletions(-) diff --git a/src/common/appcmn.cpp b/src/common/appcmn.cpp index 4c93db8584..022b5e6fcf 100644 --- a/src/common/appcmn.cpp +++ b/src/common/appcmn.cpp @@ -37,6 +37,7 @@ #include "wx/apptrait.h" #include "wx/cmdline.h" #include "wx/msgout.h" +#include "wx/richmsgdlg.h" #include "wx/thread.h" #include "wx/vidmode.h" #include "wx/evtloop.h" @@ -457,56 +458,94 @@ wxRendererNative *wxGUIAppTraitsBase::CreateRenderer() bool wxGUIAppTraitsBase::ShowAssertDialog(const wxString& msg) { #if wxDEBUG_LEVEL - // under MSW we prefer to use the base class version using ::MessageBox() - // even if wxMessageBox() is available because it has less chances to - // double fault our app than our wxMessageBox() + // If possible, show the assert using a dialog allowing to hide the stack + // trace by default to avoid frightening people unnecessarily. // - // under DFB the message dialog is not always functional right now + // Otherwise, show the assert using a basic message box, but under MSW + // we prefer to use the base class version using ::MessageBox() even if + // wxMessageBox() is available because it has less chances to double + // fault our app than our wxMessageBox() // - // and finally we can't use wxMessageBox() if it wasn't compiled in, of - // course -#if !defined(__WXMSW__) && !defined(__WXDFB__) && wxUSE_MSGDLG + // Notice that under DFB the message dialog is not always functional right + // now and, finally, we can't use wxMessageBox() if it wasn't compiled in. +#if wxUSE_RICHMSGDLG || \ + (wxUSE_MSGDLG && !defined(__WXMSW__) && !defined(__WXDFB__)) // we can't (safely) show the GUI dialog from another thread, only do it // for the asserts in the main thread if ( wxIsMainThread() ) { - wxString msgDlg = msg; + // Note that this and the other messages here are intentionally not + // translated -- they are for developpers only. + static const wxStringCharType* caption = wxS("wxWidgets Debug Alert"); -#if wxUSE_STACKWALKER - const wxString stackTrace = GetAssertStackTrace(); - if ( !stackTrace.empty() ) - msgDlg << wxT("\n\nCall stack:\n") << stackTrace; -#endif // wxUSE_STACKWALKER - - // this message is intentionally not translated -- it is for - // developpers only - msgDlg += wxT("\nDo you want to stop the program?\n") - wxT("You can also choose [Cancel] to suppress ") - wxT("further warnings."); + wxString msgDlg = wxS("A debugging check in this application ") + wxS("has failed.\n\n") + msg; // "No" button means to continue execution, so it should be the default // action as leaving the "Yes" button the default one would mean that // accidentally pressing Space or Enter would trap and kill the program. - switch ( wxMessageBox(msgDlg, wxT("wxWidgets Debug Alert"), - wxYES_NO | wxCANCEL | wxNO_DEFAULT | wxICON_STOP ) ) + const int flags = wxYES_NO | wxNO_DEFAULT | wxICON_STOP; + +#if wxUSE_STACKWALKER + const wxString stackTrace = GetAssertStackTrace(); +#endif // wxUSE_STACKWALKER + +#if wxUSE_RICHMSGDLG + wxRichMessageDialog dlg(NULL, msgDlg, caption, flags); + + dlg.SetYesNoLabels("Stop", "Continue"); + + dlg.ShowCheckBox("Don't show this dialog again"); + +#if wxUSE_STACKWALKER + if ( !stackTrace.empty() ) + dlg.ShowDetailedText(stackTrace); +#endif // wxUSE_STACKWALKER +#else // !wxUSE_RICHMSGDLG +#if wxUSE_STACKWALKER + if ( !stackTrace.empty() ) + msgDlg << wxT("\n\nCall stack:\n") << stackTrace; +#endif // wxUSE_STACKWALKER + + msgDlg += wxT("\nDo you want to stop the program?\n") + wxT("You can also choose [Cancel] to suppress ") + wxT("further warnings."); + + wxMessageDialog dlg(NULL, msg, caption, flags); +#endif // wxUSE_RICHMSGDLG/!wxUSE_RICHMSGDLG + + switch ( dlg.ShowModal() ) { - case wxYES: + case wxID_YES: // See the comment about using the same variable in // DoShowAssertDialog(). wxTrapInAssert = true; break; - case wxCANCEL: - // no more asserts + case wxID_CANCEL: + // This button is used with the plain message dialog only to + // indicate that no more assert dialogs should be shown, as + // there is no other way to do it with it. return true; - //case wxNO: nothing to do + case wxID_NO: +#if wxUSE_RICHMSGDLG + if ( dlg.IsCheckBoxChecked() ) + { + // With this dialog, the checkbox is used to indicate that + // the subsequent asserts should be skipped. + return true; + } +#endif // wxUSE_RICHMSGDLG + + // Nothing to do otherwise. + break; } return false; } -#endif // wxUSE_MSGDLG +#endif // wxUSE_RICHMSGDLG || wxUSE_MSGDLG #endif // wxDEBUG_LEVEL return wxAppTraitsBase::ShowAssertDialog(msg);