Author Topic: wrap char mode feature added for Editor Tweak plugin  (Read 23732 times)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5230
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
wrap char mode feature added for Editor Tweak plugin
« on: September 19, 2013, 12:06:48 pm »
When I try to edit a text file which contains some Chinese words, I see that the wrap word feature is ugly.
See the original view (no wrap enabled)

Now, when wrap word enabled, you can see the long Chinese sentence was treated as a single word, so there will be long empty spaces after the word "efg".


As I see the Scintilla component support "wrap char" feature, I just enabled it, and now it looks better.


The document said in http://www.scintilla.org/ScintillaDoc.html#LineWrapping
Quote
SCI_SETWRAPMODE(int wrapMode)
SCI_GETWRAPMODE
Set wrapMode to SC_WRAP_WORD (1) to enable wrapping on word boundaries, SC_WRAP_CHAR (2) to enable wrapping between any characters, and to SC_WRAP_NONE (0) to disable line wrapping. SC_WRAP_CHAR is preferred to SC_WRAP_WORD for Asian languages where there is no white space between words.

Patch is below:
Code: [Select]
From a65d48522278281a0106ce22b74d7c5c3a89dfb1 Mon Sep 17 00:00:00 2001
From: asmwarrior <[email protected]>
Date: Thu, 19 Sep 2013 18:11:27 +0800
Subject: [PATCH] * EditorTweaks: introduce wrap char mode

---
 src/plugins/contrib/EditorTweaks/EditorTweaks.cpp | 31 +++++++++++++++++++----
 src/plugins/contrib/EditorTweaks/EditorTweaks.h   |  1 +
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/plugins/contrib/EditorTweaks/EditorTweaks.cpp b/src/plugins/contrib/EditorTweaks/EditorTweaks.cpp
index cfa8062..88939af 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);
@@ -704,14 +713,26 @@ void EditorTweaks::OnWordWrap(wxCommandEvent &/*event*/)
     if (!ed || !ed->GetControl() || m_isUpdatingUI)
         return;
 
-    bool enabled=ed->GetControl()->GetWrapMode()>0;
+    bool enabled=ed->GetControl()->GetWrapMode()==wxSCI_WRAP_WORD;
 
     if (enabled)
         ed->GetControl()->SetWrapMode(wxSCI_WRAP_NONE);
     else
         ed->GetControl()->SetWrapMode(wxSCI_WRAP_WORD);
+}
+
+void EditorTweaks::OnCharWrap(wxCommandEvent &/*event*/)
+{
+    cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
+    if (!ed || !ed->GetControl() || m_isUpdatingUI)
+        return;
 
+    bool enabled=ed->GetControl()->GetWrapMode()==wxSCI_WRAP_CHAR;
 
+    if (enabled)
+        ed->GetControl()->SetWrapMode(wxSCI_WRAP_NONE);
+    else
+        ed->GetControl()->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);
--
1.8.4.msysgit.0


Comments?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12061
    • Travis build status
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #1 on: September 19, 2013, 12:23:12 pm »
Calling ed->GetControl() at every line is plain ugly. Please save it in a variable and use it instead.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5230
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #2 on: September 19, 2013, 02:28:11 pm »
Calling ed->GetControl() at every line is plain ugly. Please save it in a variable and use it instead.
No, I did not do that. Wrap mode is set once for the whole editor, no need for each lines.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12061
    • Travis build status
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #3 on: September 19, 2013, 02:34:06 pm »
Code: [Select]
void EditorTweaks::OnCharWrap(wxCommandEvent &/*event*/)
+{
+    cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
+    if (!ed || !ed->GetControl() || m_isUpdatingUI)
+        return;
 
+    bool enabled=ed->GetControl()->GetWrapMode()==wxSCI_WRAP_CHAR;
 
+    if (enabled)
+        ed->GetControl()->SetWrapMode(wxSCI_WRAP_NONE);
+    else
+        ed->GetControl()->SetWrapMode(wxSCI_WRAP_CHAR);
 }

Count the number of occurrences of ed->GetControl() and then count the number of lines of the function:)
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5230
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #4 on: September 19, 2013, 03:30:00 pm »
Code: [Select]
void EditorTweaks::OnCharWrap(wxCommandEvent &/*event*/)
+{
+    cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
+    if (!ed || !ed->GetControl() || m_isUpdatingUI)
+        return;
 
+    bool enabled=ed->GetControl()->GetWrapMode()==wxSCI_WRAP_CHAR;
 
+    if (enabled)
+        ed->GetControl()->SetWrapMode(wxSCI_WRAP_NONE);
+    else
+        ed->GetControl()->SetWrapMode(wxSCI_WRAP_CHAR);
 }

Count the number of occurrences of ed->GetControl() and then count the number of lines of the function:)

OK, the function name cause confusion. :)
This function is a menu click handler. There was a function named "OnWordWrap", so I add this one named "OnCharWrap".
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12061
    • Travis build status
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #5 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.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #6 on: September 19, 2013, 07:29:51 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.

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.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5230
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #7 on: September 20, 2013, 05:05:43 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.
What about this:
Code: [Select]
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);


As dmoore said, there are a lot of code like:
Code: [Select]
void EditorTweaks::OnXXXXXXX(wxCommandEvent &/*event*/)
{
    cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
    if (!ed || !ed->GetControl() || m_isUpdatingUI)
        return;

    ed->GetControl()->XXXXXX();
    ....
    ed->GetControl()->YYYYYY();
}
Can you give a much better way?
I think we can extract a common code to a member function, like:
Code: [Select]
cbStyledTextCtrl* EditorTweaks::GetSafeControl()
{
    cbEditor* ed = Manager::Get()->GetEditorManager()->GetBuiltinActiveEditor();
    if (!ed || m_isUpdatingUI)
        return nullptr;
    return ed->GetControl();
}

Then in the function body, we can have:
Code: [Select]
void EditorTweaks::OnXXXXXXX(wxCommandEvent &/*event*/)
{
    cbStyledTextCtrl* control = GetSafeControl();
    if (!control || m_isUpdatingUI)
        return;

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

EDIT: Check the m_isUpdatingUI in the GetSafeControl function?

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12061
    • Travis build status
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #8 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.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5230
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #9 on: September 27, 2013, 11:23:48 am »
You can go the EditorTweaks::GetSafeControl() path if you know that you'll use the function a lot.
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: [Select]
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
}

But you have many checks in the event function handler, like: ( I have wrap the check in function GetSafeControl())
Code: [Select]
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();
}

Does this means you have some chance that
Code: [Select]
submenu->Check(id_et_TabChar,ed->GetControl()->GetUseTabs());
will internally call
Code: [Select]
void EditorTweaks::OnCharWrap(wxCommandEvent &/*event*/)
?

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

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #10 on: September 27, 2013, 06:06:31 pm »
Does this means you have some chance that
Code: [Select]
submenu->Check(id_et_TabChar,ed->GetControl()->GetUseTabs());
will internally call
Code: [Select]
void EditorTweaks::OnCharWrap(wxCommandEvent &/*event*/)
?

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

I think that was my reasoning for including that check. Presumably there are nicer ways of handling this.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5230
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #11 on: September 28, 2013, 03:21:17 am »
Does this means you have some chance that
Code: [Select]
submenu->Check(id_et_TabChar,ed->GetControl()->GetUseTabs());
will internally call
Code: [Select]
void EditorTweaks::OnCharWrap(wxCommandEvent &/*event*/)
?

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

I think that was my reasoning for including that check. Presumably there are nicer ways of handling this.
From my research of wx's source, I think that manually runing the functions
Code: [Select]
void wxMenuBase::Check( int id, bool enable )does not cause event handler to be called, so I simply think the member variable m_isUpdatingUI is not needed here.

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #12 on: September 28, 2013, 09:32:41 pm »
This is often a problem with GTK so I assumed it would also be a problem with wxGTK. If you are sure it isn't then go ahead and remove it.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5230
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #13 on: September 29, 2013, 11:10:38 am »
This is often a problem with GTK so I assumed it would also be a problem with wxGTK. If you are sure it isn't then go ahead and remove it.
Ok, I asked this question on wx forums, and some one give me the reply, Re: wxMenu Check() cause event handler to be called?, now I need some one help to give me the result on Linux. BTW: I do have have a Linux system.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5230
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: wrap char mode feature added for Editor Tweak plugin
« Reply #14 on: October 03, 2013, 05:54:23 pm »
This is often a problem with GTK so I assumed it would also be a problem with wxGTK. If you are sure it isn't then go ahead and remove it.
Ok, I asked this question on wx forums, and some one give me the reply, Re: wxMenu Check() cause event handler to be called?, now I need some one help to give me the result on Linux. BTW: I do have have a Linux system.
Ping, any one can test this on Linux, thanks.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.