wrap char mode feature added for Editor Tweak plugin

Developer forums (C::B DEVELOPMENT STRICTLY!) > Plugins development

<< < (2/7) > >>

oBFusCATed:
Hm, it seems that I'm not clear enough.
In the snippet I've posted you have 4 calls to ed->GetControl(), instead of just doing "Control *control=ed->GetControl();" and then just use control.

dmoore:

--- Quote from: oBFusCATed on September 19, 2013, 03:36:58 pm ---Hm, it seems that I'm not clear enough.
In the snippet I've posted you have 4 calls to ed->GetControl(), instead of just doing "Control *control=ed->GetControl();" and then just use control.

--- End quote ---

This is sort of my fault, not just his patch, because the whole plugin is full of ed->GetControl()s. I say "sort of" because ed->GetControl() is a reasonably frequent occurrence throughout the C::B codebase, though in this particularly plugin it's grown to be egregious due to the sheer number of options.

ollydbg:

--- Quote from: oBFusCATed on September 19, 2013, 03:36:58 pm ---Hm, it seems that I'm not clear enough.
In the snippet I've posted you have 4 calls to ed->GetControl(), instead of just doing "Control *control=ed->GetControl();" and then just use control.

--- End quote ---
What about this:

--- Code: --- src/plugins/contrib/EditorTweaks/EditorTweaks.cpp | 43 ++++++++++++++++++-----
src/plugins/contrib/EditorTweaks/EditorTweaks.h | 1 +
2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/src/plugins/contrib/EditorTweaks/EditorTweaks.cpp b/src/plugins/contrib/EditorTweaks/EditorTweaks.cpp
index cfa8062..fa1c2ad 100644
--- a/src/plugins/contrib/EditorTweaks/EditorTweaks.cpp
+++ b/src/plugins/contrib/EditorTweaks/EditorTweaks.cpp
@@ -42,6 +42,7 @@ namespace

int id_et = wxNewId();
int id_et_WordWrap = wxNewId();
+int id_et_CharWrap = wxNewId();
int id_et_ShowLineNumbers = wxNewId();
int id_et_TabChar = wxNewId();
int id_et_TabIndent = wxNewId();
@@ -75,6 +76,7 @@ int id_et_ScrollTimer = wxNewId();
// events handling
BEGIN_EVENT_TABLE(EditorTweaks, cbPlugin)
   EVT_UPDATE_UI(id_et_WordWrap, EditorTweaks::OnUpdateUI)
+ EVT_UPDATE_UI(id_et_CharWrap, EditorTweaks::OnUpdateUI)
   EVT_UPDATE_UI(id_et_ShowLineNumbers, EditorTweaks::OnUpdateUI)
   EVT_UPDATE_UI(id_et_TabChar, EditorTweaks::OnUpdateUI)
   EVT_UPDATE_UI(id_et_TabIndent, EditorTweaks::OnUpdateUI)
@@ -91,6 +93,7 @@ BEGIN_EVENT_TABLE(EditorTweaks, cbPlugin)

   EVT_MENU(id_et_WordWrap, EditorTweaks::OnWordWrap)
+ EVT_MENU(id_et_CharWrap, EditorTweaks::OnCharWrap)
   EVT_MENU(id_et_ShowLineNumbers, EditorTweaks::OnShowLineNumbers)
   EVT_MENU(id_et_TabChar, EditorTweaks::OnTabChar)
   EVT_MENU(id_et_TabIndent, EditorTweaks::OnTabIndent)
@@ -276,7 +279,8 @@ void EditorTweaks::BuildMenu(wxMenuBar* menuBar)

   wxMenu *submenu=m_tweakmenu; //_("Editor Tweaks")

- submenu->AppendCheckItem( id_et_WordWrap, _( "Word wrap" ), _( "Wrap text" ) );
+ submenu->AppendCheckItem( id_et_WordWrap, _( "Word wrap" ), _( "Wrap word" ) );
+ submenu->AppendCheckItem( id_et_CharWrap, _( "Char wrap" ), _( "Wrap char" ) );
   submenu->AppendCheckItem( id_et_ShowLineNumbers, _( "Show Line Numbers" ), _( "Show Line Numbers" ) );
   submenu->AppendSeparator();
   submenu->AppendCheckItem( id_et_TabChar, _( "Use Tab Character" ), _( "Use Tab Character" ) );
@@ -359,7 +363,8 @@ void EditorTweaks::UpdateUI()
   wxMenu *submenu = m_tweakmenu; //_("Editor Tweaks") TODO: Retrieve actual menu

   m_isUpdatingUI = true; // ignore events the following can trigger
- submenu->Check(id_et_WordWrap,ed->GetControl()->GetWrapMode()>0);
+ submenu->Check(id_et_WordWrap,ed->GetControl()->GetWrapMode()==wxSCI_WRAP_WORD);
+ submenu->Check(id_et_CharWrap,ed->GetControl()->GetWrapMode()==wxSCI_WRAP_CHAR);
   submenu->Check(id_et_ShowLineNumbers,ed->GetControl()->GetMarginWidth(0)>0);
   submenu->Check(id_et_TabChar,ed->GetControl()->GetUseTabs());
   submenu->Check(id_et_TabIndent,ed->GetControl()->GetTabIndents());
@@ -639,10 +644,14 @@ void EditorTweaks::BuildModuleMenu(const ModuleType type, wxMenu* menu, const Fi

   menu->Append(id_et,_("Editor Tweaks"),submenu);

- submenu->AppendCheckItem( id_et_WordWrap, _( "Word wrap" ), _( "Wrap text" ) );
- if (ed->GetControl()->GetWrapMode()>0)
+ submenu->AppendCheckItem( id_et_WordWrap, _( "Word wrap" ), _( "Wrap word" ) );
+ if (ed->GetControl()->GetWrapMode()==wxSCI_WRAP_WORD)
   submenu->Check(id_et_WordWrap,true);

+ submenu->AppendCheckItem( id_et_CharWrap, _( "Char wrap" ), _( "Wrap char" ) );
+ if (ed->GetControl()->GetWrapMode()==wxSCI_WRAP_CHAR)
+ submenu->Check(id_et_CharWrap,true);
+
   submenu->AppendCheckItem( id_et_ShowLineNumbers, _( "Show Line Numbers" ), _( "Show Line Numbers" ) );
   if (ed->GetControl()->GetMarginWidth(0)>0)
   submenu->Check(id_et_ShowLineNumbers,true);
@@ -701,17 +710,35 @@ void EditorTweaks::BuildModuleMenu(const ModuleType type, wxMenu* menu, const Fi
void EditorTweaks::OnWordWrap(wxCommandEvent &/*event*/)
{
   cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
- if (!ed || !ed->GetControl() || m_isUpdatingUI)
+ if (!ed || m_isUpdatingUI)
+ return;
+ cbStyledTextCtrl* control = ed->GetControl();
+ if (!control)
   return;

- bool enabled=ed->GetControl()->GetWrapMode()>0;
+ bool enabled = control->GetWrapMode() == wxSCI_WRAP_WORD;

   if (enabled)
- ed->GetControl()->SetWrapMode(wxSCI_WRAP_NONE);
+ control->SetWrapMode(wxSCI_WRAP_NONE);
   else
- ed->GetControl()->SetWrapMode(wxSCI_WRAP_WORD);
+ control->SetWrapMode(wxSCI_WRAP_WORD);
+}

+void EditorTweaks::OnCharWrap(wxCommandEvent &/*event*/)
+{
+ cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
+ if (!ed || m_isUpdatingUI)
+ return;
+ cbStyledTextCtrl* control = ed->GetControl();
+ if (!control)
+ return;

+ bool enabled = control->GetWrapMode() == wxSCI_WRAP_CHAR;
+
+ if (enabled)
+ control->SetWrapMode(wxSCI_WRAP_NONE);
+ else
+ control->SetWrapMode(wxSCI_WRAP_CHAR);
}

void EditorTweaks::OnShowLineNumbers(wxCommandEvent &/*event*/)
diff --git a/src/plugins/contrib/EditorTweaks/EditorTweaks.h b/src/plugins/contrib/EditorTweaks/EditorTweaks.h
index aa15f9a..86c3074 100644
--- a/src/plugins/contrib/EditorTweaks/EditorTweaks.h
+++ b/src/plugins/contrib/EditorTweaks/EditorTweaks.h
@@ -106,6 +106,7 @@ class EditorTweaks : public cbPlugin
   void OnKeyPress(wxKeyEvent& event);
   void OnChar(wxKeyEvent& event);
   void OnWordWrap(wxCommandEvent &event);
+ void OnCharWrap(wxCommandEvent &event);
   void OnShowLineNumbers(wxCommandEvent &event);
   void OnTabChar(wxCommandEvent &event);
   void OnTabIndent(wxCommandEvent &event);

--- End code ---

As dmoore said, there are a lot of code like:

--- Code: ---void EditorTweaks::OnXXXXXXX(wxCommandEvent &/*event*/)
{
   cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
   if (!ed || !ed->GetControl() || m_isUpdatingUI)
   return;

   ed->GetControl()->XXXXXX();
   ....
   ed->GetControl()->YYYYYY();
}

--- End code ---
Can you give a much better way?
I think we can extract a common code to a member function, like:

--- Code: ---cbStyledTextCtrl* EditorTweaks::GetSafeControl()
{
   cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
   if (!ed || m_isUpdatingUI)
   return nullptr;
   return ed->GetControl();
}

--- End code ---

Then in the function body, we can have:

--- Code: ---void EditorTweaks::OnXXXXXXX(wxCommandEvent &/*event*/)
{
   cbStyledTextCtrl* control = GetSafeControl();
   if (!control || m_isUpdatingUI)
   return;

   control->XXXXXX();
   ....
   control->YYYYYY();
}

--- End code ---
Comments?

EDIT: Check the m_isUpdatingUI in the GetSafeControl function?

oBFusCATed:
You can go the EditorTweaks::GetSafeControl() path if you know that you'll use the function a lot.

ollydbg:

--- Quote from: oBFusCATed on September 20, 2013, 05:53:30 pm ---You can go the EditorTweaks::GetSafeControl() path if you know that you'll use the function a lot.

--- End quote ---
I'm on the way, thanks.

@dmoore
I have a question:
What does the member variable "m_isUpdatingUI" used for?
I see the only time it was set to true is in the function:

--- Code: ---void EditorTweaks::UpdateUI()
{
if (!m_tweakmenu)
return;

cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
if (!ed || !ed->GetControl())
{
m_tweakmenuitem->Enable(false);
return;
}
m_tweakmenuitem->Enable(true);

wxMenu *submenu = m_tweakmenu; //_("Editor Tweaks") TODO: Retrieve actual menu

m_isUpdatingUI = true; // ignore events the following can trigger
submenu->Check(id_et_WordWrap,ed->GetControl()->GetWrapMode()==wxSCI_WRAP_WORD); // FIXME (ollydbg#1#09/26/13): Dose this Check function cause an menu event handler call?
submenu->Check(id_et_CharWrap,ed->GetControl()->GetWrapMode()==wxSCI_WRAP_CHAR);
submenu->Check(id_et_ShowLineNumbers,ed->GetControl()->GetMarginWidth(0)>0);
submenu->Check(id_et_TabChar,ed->GetControl()->GetUseTabs());
submenu->Check(id_et_TabIndent,ed->GetControl()->GetTabIndents());
submenu->Check(id_et_TabSize2,ed->GetControl()->GetTabWidth()==2);
submenu->Check(id_et_TabSize4,ed->GetControl()->GetTabWidth()==4);
submenu->Check(id_et_TabSize6,ed->GetControl()->GetTabWidth()==6);
submenu->Check(id_et_TabSize8,ed->GetControl()->GetTabWidth()==8);
submenu->Check(id_et_EOLCRLF,ed->GetControl()->GetEOLMode()==wxSCI_EOL_CRLF);
submenu->Check(id_et_EOLCR,ed->GetControl()->GetEOLMode()==wxSCI_EOL_CR);
submenu->Check(id_et_EOLLF,ed->GetControl()->GetEOLMode()==wxSCI_EOL_LF);
submenu->Check(id_et_ShowEOL,ed->GetControl()->GetViewEOL());
submenu->Check(id_et_SuppressInsertKey, m_suppress_insert);
submenu->Check(id_et_ConvertBraces, m_convert_braces);
m_isUpdatingUI = false; // done
}

--- End code ---

But you have many checks in the event function handler, like: ( I have wrap the check in function GetSafeControl())

--- Code: ---void EditorTweaks::OnCharWrap(wxCommandEvent &/*event*/)
{
cbStyledTextCtrl* control = GetSafeControl();
if (!control)
return;

bool enabled = control->GetWrapMode() == wxSCI_WRAP_CHAR;

if (enabled)
control->SetWrapMode(wxSCI_WRAP_NONE);
else
control->SetWrapMode(wxSCI_WRAP_CHAR);
}
....
cbStyledTextCtrl* EditorTweaks::GetSafeControl()
{
cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
if (!ed || m_isUpdatingUI)
return nullptr;
return ed->GetControl();
}

--- End code ---

Does this means you have some chance that

--- Code: ---submenu->Check(id_et_TabChar,ed->GetControl()->GetUseTabs());

--- End code ---
will internally call

--- Code: ---void EditorTweaks::OnCharWrap(wxCommandEvent &/*event*/)

--- End code ---
?

In case this does change the status of id_et_TabChar (from check to unchecked or inverse), does this cause some error?
Thanks.

Navigation

[0] Message Index

[#] Next page

[*] Previous page

Go to full version