I had some no-quality spare time sitting in a train and looked over one of the classes in the plugin.
only refactoring, functionality should be identical.
changes to addtododlg.h :
- changed the constructor: now takes const& instead of copying arguments
- made the destructor virtual
- removed two not necessary members
- some clean up
changes to addtododlg.cpp :
- changed the constructor: now takes const& instead of copying arguments
- moved empty destructor to header file
- removed two not necessary members
- removed some loops to add entries to wxWidgets gui elements which support adding an array of entries
- removed some loops getting the entries from gui elements, when they support retrieving an array
- some clean up
regards
First, thanks for the contribution.
I just review the patch, and I have two questions.
1, I see you have such change:
- if (cmb->GetCount()>0)
- cmb->SetSelection(0);
+ cmb->SetSelection(0);
It is the same thing?
2, I see that you have removed the two member variables:
- wxArrayString m_Users;
- wxArrayString m_Types;
std::bitset<(int)tdctError+1> m_supportedTdcts;
DECLARE_EVENT_TABLE()
When I read the code, I see that inside the function:
void AddTodoDlg::EndModal(int retVal)
{
if (retVal == wxID_OK)
{
- // "save" users
wxChoice* cmb = XRCCTRL(*this, "chcUser", wxChoice);
- m_Users.Clear();
- if (cmb->FindString(cmb->GetStringSelection(), true) == wxNOT_FOUND)
- m_Users.Add(cmb->GetStringSelection());
- for (unsigned int i = 0; i < cmb->GetCount(); ++i)
- m_Users.Add(cmb->GetString(i));
- Manager::Get()->GetConfigManager(_T("todo_list"))->Write(_T("users"), m_Users);
+ Manager::Get()->GetConfigManager(_T("todo_list"))->Write(_T("users"), cmb->GetStrings());
Manager::Get()->GetConfigManager(_T("todo_list"))->Write(_T("last_used_user"), cmb->GetStringSelection());
- // "save" types
cmb = XRCCTRL(*this, "chcType", wxChoice);
- m_Types.Clear();
- if (cmb->FindString(cmb->GetStringSelection(), true) == wxNOT_FOUND)
- m_Types.Add(cmb->GetStringSelection());
- for (unsigned int i = 0; i < cmb->GetCount(); ++i)
- m_Types.Add(cmb->GetString(i));
- Manager::Get()->GetConfigManager(_T("todo_list"))->Write(_T("types"), m_Types);
+ Manager::Get()->GetConfigManager(_T("todo_list"))->Write(_T("types"), cmb->GetStrings());
Manager::Get()->GetConfigManager(_T("todo_list"))->Write(_T("last_used_type"), cmb->GetStringSelection());
cmb = XRCCTRL(*this, "chcStyle", wxChoice);
Manager::Get()->GetConfigManager(_T("todo_list"))->Write(_T("last_used_style"), cmb->GetStringSelection());
Here, you just save the types and users from the GUI (through cmb->GetStrings()) to the configure files.
But when I looked at the usage of the dialog:
void ToDoList::OnAddItem(cb_unused wxCommandEvent& event)
{
cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
if (!ed)
return;
EditorColourSet* colour_set = Manager::Get()->GetEditorManager()->GetColourSet();
if (!colour_set)
return;
HighlightLanguage hlang = colour_set->GetLanguageName(ed->GetLanguage());
bool edIsCCpp = (hlang == _T("C/C++"));
CommentToken token = colour_set->GetCommentToken(ed->GetLanguage());
bool hasStreamComments = not token.streamCommentStart.IsEmpty();
bool hasLineComments = not token.lineComment.IsEmpty();
if (!(edIsCCpp||hasLineComments||hasStreamComments))
return;
std::bitset<(int)tdctError+1> supportedTdcts;
supportedTdcts[tdctLine] = !(token.lineComment.IsEmpty());
supportedTdcts[tdctStream] = !(token.streamCommentStart.IsEmpty());
supportedTdcts[tdctDoxygenLine] = !(token.doxygenLineComment.IsEmpty());
supportedTdcts[tdctDoxygenStream] = !(token.doxygenStreamCommentStart.IsEmpty());
supportedTdcts[tdctWarning] = edIsCCpp;
supportedTdcts[tdctError] = edIsCCpp;
// display todo dialog
AddTodoDlg dlg(Manager::Get()->GetAppWindow(), m_Users, m_Types, supportedTdcts );
PlaceWindow(&dlg);
if (dlg.ShowModal() != wxID_OK)
return;
// Re-load users and types as they might have changed by the user via AddTodoDlg
LoadUsers();
LoadTypes();
Here, in the LoadUsers(); and LoadTypes(); function, it was saved again in the configure file.
My question is: why do we need to pass the "m_Users, m_Types" as an argument to the AddTodoDlg's constructor, it looks like in the ToDoList class, we don't need the two "m_Users, m_Types" member variables either.
Inside the constructor of AddTodoDlg, it also contains some code like:
if (users.empty())
cmb->Append(wxGetUserId());