1. ADRet is quite confusing, something longer should be used here. Being verbose is no such problem.
[...]
5. Some comments are needed in regard to the formatting of the string. BeforeLast and After
Will do.
2. AD_SAVE_CHOICE should be a constant and it should not be in the header
It is in the header so other files can use it:
AnnoyingDialog dlg(_("XYZ question?"), msg, wxART_INFORMATION,
AnnoyingDialog::YES_NO, AD_SAVE_CHOICE);
3. Mixing eol changes with other changes is not good!
Oops...
I see, I format. Bad habit.
4. Converting wxID_????? to string is dangerous, you should define your own constants.
Probably an enum, which is stable with different versions of wx.
The problem with defining my own enums, is that the parameter for default return type could *technically* be any number (not just the specific ones I registered). How should I handle an unknown return type? Currently, I always save the default type when AD_SAVE_CHOICE is not specified. This is so it will show up in the dismissed dialogues box with the choice it will use.
6. What will happen if you use older C::B with newconfig? Probably you should use another config path.
Don't. Unless you want to be annoyed, because all the annoying dialogues will pop up again.
2. What is the purpose of this param then? Doxy comment please.
No documentation available yet; here is the constructor's signature:
AnnoyingDialog(const wxString& caption, const wxString& message, const wxArtID icon = wxART_INFORMATION,
dStyle style = YES_NO, int defaultReturn = wxID_YES, bool separate = true,
const wxString& b1 = wxEmptyString, const wxString& b2 = wxEmptyString, const wxString& b3 = wxEmptyString);
Static here is inappropriate, you want external linkage here.
Hmm... looks like I have some reading to do. (My C++ knowledge appears to be rather incomplete.)
If I were you, I'd made it enum in the AnnoyingDialog class. So people will have to use something like AnnoyingDialog::SaveChoice
4. Probably add AnnoyingDialogChoice::Unknown
My preference would be to switch int defaultReturn = wxID_YES to an enum (for example: dReturnType defaultReturn = YES). However, I had been attempting to leave the API unchanged (so plugins would not need fixing).
6. People will do this and they will get annoyed, you have to think of a way to handle this (Me included).
This can be solved by reading from both the old and a new settings path, but only writing to the new path.
Hmm... I just discovered a rather major design flaw in my plan. If I set the "Layout changed" dialogue to rtSAVE_CHOICE, the user could potentially save rtCANCEL, which would cause strange errors of sometimes not being able to switch layouts. If I do the following, the behavior will be inconsistent:
Index: src/src/main.cpp
===================================================================
--- src/src/main.cpp (revision 9113)
+++ src/src/main.cpp (working copy)
@@ -1488,7 +1488,9 @@
AnnoyingDialog dlg(_("Layout changed"),
wxString::Format(_("The perspective '%s' has changed. Do you want to save it?"), m_LastLayoutName.wx_str()),
wxART_QUESTION,
- canCancel ? AnnoyingDialog::YES_NO_CANCEL : AnnoyingDialog::YES_NO);
+ canCancel ? AnnoyingDialog::YES_NO_CANCEL : AnnoyingDialog::YES_NO,
+ // do not let the user save 'rtCANCEL' otherwise we will have problems...
+ canCancel ? AnnoyingDialog::rtYES : AnnoyingDialog::rtSAVE_CHOICE);
switch (dlg.ShowModal())
{
case AnnoyingDialog::rtYES:
Ideas?