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

ollydbg · « **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

From a65d48522278281a0106ce22b74d7c5c3a89dfb1 Mon Sep 17 00:00:00 2001
From: asmwarrior <asmwarrior@gmail.com>
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?

oBFusCATed · « **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.

ollydbg · « **Reply #2 on:** September 19, 2013, 02:28:11 pm »

Quote from: oBFusCATed 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.

No, I did not do that. Wrap mode is set once for the whole editor, no need for each lines.

oBFusCATed · « **Reply #3 on:** September 19, 2013, 02:34:06 pm »

Code

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:)

ollydbg · « **Reply #4 on:** September 19, 2013, 03:30:00 pm »

Quote from: oBFusCATed on September 19, 2013, 02:34:06 pm

Code

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".

oBFusCATed · « **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.

dmoore · « **Reply #6 on:** September 19, 2013, 07:29:51 pm »

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.

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 · « **Reply #7 on:** September 20, 2013, 05:05:43 pm »

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.

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);

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();
}

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();
}

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();
}

Comments?

EDIT: Check the m_isUpdatingUI in the GetSafeControl function?

oBFusCATed · « **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.

ollydbg · « **Reply #9 on:** September 27, 2013, 11:23:48 am »

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.

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
}

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();
}

Does this means you have some chance that

Code

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

will internally call

Code

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.

dmoore · « **Reply #10 on:** September 27, 2013, 06:06:31 pm »

Quote from: ollydbg on September 27, 2013, 11:23:48 am

Does this means you have some chance that
Code
submenu->Check(id_et_TabChar,ed->GetControl()->GetUseTabs());
will internally call
Code
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.

ollydbg · « **Reply #11 on:** September 28, 2013, 03:21:17 am »

Quote from: dmoore on September 27, 2013, 06:06:31 pm

Quote from: ollydbg on September 27, 2013, 11:23:48 am
Does this means you have some chance that
Code
submenu->Check(id_et_TabChar,ed->GetControl()->GetUseTabs());
will internally call
Code
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

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.

dmoore · « **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.

ollydbg · « **Reply #13 on:** September 29, 2013, 11:10:38 am »

Quote from: dmoore 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.

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.

ollydbg · « **Reply #14 on:** October 03, 2013, 05:54:23 pm »

Quote from: ollydbg on September 29, 2013, 11:10:38 am

Quote from: dmoore 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.
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.

dmoore · « **Reply #15 on:** October 04, 2013, 12:31:04 am »

where is the patch?

ollydbg · « **Reply #16 on:** October 04, 2013, 03:23:23 am »

Quote from: ollydbg on October 03, 2013, 05:54:23 pm

Quote from: ollydbg on September 29, 2013, 11:10:38 am
Quote from: dmoore 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.
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.

@dmoore
Testing this under wxGTK don't need any patches, you just open the EditorTweak source, and set a breakpoint in any menu item event handler. Then you need to open two editors, change one menu item status. Now, you can swith between two editors, and the menu item status will change after the swith, you need to see wether the event handler will be called after the swith. If yes, this means we need m_isUpdatingUI. This means programmatically change the menu item status will cause its event handler be called.

dmoore · « **Reply #17 on:** October 04, 2013, 06:22:58 pm »

Ok, seems to work fine without the m_isUpdatingUI check. Go ahead and commit your change.

ollydbg · « **Reply #18 on:** October 06, 2013, 12:48:29 pm »

Quote from: dmoore on October 04, 2013, 06:22:58 pm

Ok, seems to work fine without the m_isUpdatingUI check. Go ahead and commit your change.

Thank you for testing.
Committed in r9382, I also did some code clean up. Please adjust if I did something wrong.

ollydbg · « **Reply #19 on:** October 06, 2013, 02:36:48 pm »

BTW: I see the same EVT_UPDATE_UI issue in Editor Tweak Plugin. By search the forum, I found that it was discussed several years ago in Re: wxUpdateUIEvent performance issues.

My idea is the same as Jens said, if we bind same function to many menu items. The function will be called many times for each menu item.

Oh, here is new idea: can we set a timer in the update function(maybe a static member variable to remember the time stamp the function was called), then check if we have already called the function in 100ms, we can simply return from the function to avoid the redundant/unnecessary updates.

dmoore · « **Reply #20 on:** October 06, 2013, 03:00:26 pm »

I think (but may be misremembering) we fixed a pretty major bug in wxScintilla that was generating a lot of UpdateUI messages. So while it's probably inefficient to have so many handlers for the same event I don't think the performance penalty is all that bad because there is an order of magnitude fewer calls. Fine if you want to combine the handlers, but I personally would seek out some other solution than a timer. (Adding latency as a workaround sucks!)

oBFusCATed · « **Reply #21 on:** October 06, 2013, 03:25:57 pm »

But is it a performance problem? You know first rule of optimisations, right? Measure before doing anything...

gd_on · « **Reply #22 on:** October 06, 2013, 06:41:49 pm »

SVN 9382 has a problem for me on Windows.
C::B starts, but is not able to load a project: it hangs and I have to kill C::B.
If I disable Editor Tweaks, my project can be loaded.
This problem does not happen with svn 9380 (I have not tried svn 9381)

gd_on

oBFusCATed · « **Reply #23 on:** October 06, 2013, 07:30:03 pm »

Are you sure you've rebuild it correctly?

This is the main discussion about this commit: http://forums.codeblocks.org/index.php/topic,18358.msg125852/topicseen.html#msg125852

@admins: Please move these posts there.

MortenMacFly · « **Reply #24 on:** October 06, 2013, 07:40:33 pm »

Quote from: oBFusCATed on October 06, 2013, 07:30:03 pm

@admins: Please move these posts there.

Done.

MortenMacFly · « **Reply #25 on:** October 06, 2013, 08:20:16 pm »

Quote from: gd_on on October 06, 2013, 06:41:49 pm

C::B starts, but is not able to load a project: it hangs and I have to kill C::B.
If I disable Editor Tweaks, my project can be loaded.

Even worse for me: C::B crashes immediately on startup not leaving any debug log if I leave the editortweaks plugin enabled. It works fine when disabling (removing) this plugin.

oBFusCATed · « **Reply #26 on:** October 06, 2013, 08:23:54 pm »

Should be fixed in trunk.

@devs: Please add -Werror=return-type in your global compiler settings, it makes life a lot easier

ollydbg · « **Reply #27 on:** October 06, 2013, 09:01:18 pm »

Quote from: oBFusCATed on October 06, 2013, 08:23:54 pm

Should be fixed in trunk.

@devs: Please add -Werror=return-type in your global compiler settings, it makes life a lot easier

Thank you for fixing this.
I am sorry about this error commit.

MortenMacFly · « **Reply #28 on:** October 06, 2013, 09:53:51 pm »

Quote from: oBFusCATed on October 06, 2013, 08:23:54 pm

Should be fixed in trunk.

I'm afraid not - its still crashing for me.

MortenMacFly · « **Reply #29 on:** October 06, 2013, 10:05:24 pm »

Quote from: MortenMacFly on October 06, 2013, 09:53:51 pm

I'm afraid not - its still crashing for me.

OK - now it should be REALLY fixed.

The reason was a exception diue to the call of UpdateUI which calls the SDK function to obtain an editor before the plugin is actually attached.

ollydbg · « **Reply #30 on:** October 07, 2013, 10:03:24 am »

The reason why I check in a wrong function in rev 9382 is that originally I have code like:

Code

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

After the testing by dmoore, I quickly blindly remove the m_isUpdatingUI check, thus error happens:

Code

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

MortenMacFly · « **Reply #31 on:** October 07, 2013, 11:08:42 am »

It is generally not safe to call SDK methods if the plugin is not attached (that's what the method IsAttached() is for). So the change in the two places should be fine.

ollydbg · « **Reply #32 on:** October 07, 2013, 11:26:09 am »

Quote from: MortenMacFly on October 07, 2013, 11:08:42 am

It is generally not safe to call SDK methods if the plugin is not attached (that's what the method IsAttached() is for). So the change in the two places should be fine.

I'm curious that this bug should be already in trunk before rev 9382, since I don't change the control sequence in the rev 9382. Do you have a crash call stack? I'm curious which function cause this crash issue.

EDIT: I just test rev 9385 (before your fix), I have no crash here.

MortenMacFly · « **Reply #33 on:** October 08, 2013, 07:16:47 am »

Quote from: ollydbg on October 07, 2013, 11:26:09 am

Do you have a crash call stack? I'm curious which function cause this crash issue.

Yes I had one but deleted it already. It was from PluginManager -> RegisterPlugin calling the plugins initialisation routine which caused an updateui event. this caused the illegal method call I had mentioned. Just trust me. :-)

News:

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