EditorTweaks: new option 'Convert Matching Braces' : little bug

[killerbot]

Code

int main()

select the opening '(' => press '[' ==>

Code

int main[]

select the opening '[' => press '(' ==>

Code

int main())

==> so we get 2 ')'

[dmoore]

I think this is a conflict with SmartIndent    (Disable smart indent and try again).

So I can
1/ Merge into smart indent.
2/ Change smart indent logic slightly (don't do smart indent's bracing if there is a selection) but keep the feature separate.
3/ Move the smart indent brace completion out of smart indent
4/ Do nothing / revert.

Obviously 4 isn't my preferred. The unappealing thing about 1 is having to do that logic 5+ times over for all of the SmartIndent languages. Not sure where these features really belong. It's not clear to me that a plugin named "Smart Indent" should include brace completions at all, but that's not a huge deal.

I was planning to add additional features:

* autoconversions for <, > (for html/xml), ' and "
* if user has a selection, typing a brace char (or quote char) puts the brace around the selection instead of deleting the selection.

[zabzonk]

What are the circumstances in which one would want to do this? I don't think I have ever wanted to convert () to [].

[dmoore]

What are the circumstances in which one would want to do this? I don't think I have ever wanted to convert () to [].

Converting the braces is mostly useful for python (in C/C++, probably much less commonly needed). Removing the matching brace is probably generally useful as would be enclosing a selection in braces instead of just deleting it. But fair point, I could just make the changes to the smart indent python code.

[Alpha]

* if user has a selection, typing a brace char (or quote char) puts the brace around the selection instead of deleting the selection.

This feature already exists.  Do you have Selection brace completion enabled?  (Although, the recent SmartIndent refactor has restricted it to C/C++ only; I am exploring how to make it general purpose again.)

[dmoore]

@alpha: I do have that option enabled. I was working in python when I decided to make the changes so that's probably why I missed it. We should coordinate on these changes and decide where they should go.

Which reminds me, the value of the brace conversion is it means less typing when porting code from another language. In my case it was Matlab to Python, but it could be useful in lots of other cases.

[oBFusCATed]

...decide where they should go.

Base class functions?

[dmoore]

Ok, I'm trying to reimplement this in the smart indent plugin. For now, just in the Cpp handler, which I will eventually move parts of to the base class. That patch is below and attached.

I am having a couple problems:
1. To do it in SmartIndent I need to handle wxEVT_SCI_KEY. But for some reason the plugin doesn't seem to receive it, despite receiving wsEVT_SCI_CHARADDED? I'm probably just doing something dumb, but any pointers appreciated
2. Many of the methods have been defined const. I would prefer to change them not to be const so I can add a member that holds the bracematch position (I could imagine all of the methods might need to be non-const for some languages). I have worked around this with a global variable for now. Is there a strong reason to make these functions const other than trying to be strict?

Code

Index: src/plugins/contrib/SmartIndent/CppSmartIndent.cpp
===================================================================
--- src/plugins/contrib/SmartIndent/CppSmartIndent.cpp	(revision 8464)
+++ src/plugins/contrib/SmartIndent/CppSmartIndent.cpp	(working copy)
@@ -19,8 +19,11 @@
 namespace
 {
     PluginRegistrant<CppSmartIndent> reg(_T("CppSmartIndent"));
-}
-void CppSmartIndent::OnEditorHook(cbEditor* ed, wxScintillaEvent& event) const
+}
+
+static int m_bracePos=-1;
+
+void CppSmartIndent::OnEditorHook(cbEditor* ed, wxScintillaEvent& event)const
 {
 
     // check if smart indent is enabled
@@ -32,18 +35,33 @@
 
     if ( !SmartIndentEnabled() )
         return;
-
-    wxEventType type = event.GetEventType();
-    if ( type != wxEVT_SCI_CHARADDED )
-        return;
-
+
     cbStyledTextCtrl* stc = ed->GetControl();
     if (!stc)
         return;
 
     wxString langname = Manager::Get()->GetEditorManager()->GetColourSet()->GetLanguageName(ed->GetLanguage());
     if ( langname != _T("C/C++") && langname != _T("D") && langname != _T("Java") ) return;
+
+    wxEventType type = event.GetEventType();
+    if ( type == wxEVT_SCI_KEY )
+    {
+        int p = stc->GetCurrentPos();
+        int a = stc->GetAnchor();
+        int m=p;
+        if(a>=0 && a<p)
+            m=a;
+        m_bracePos = stc->BraceMatch(m);
+        Manager::Get()->GetLogManager()->Log(wxString::Format(_T("Key Event Brace pos %i"),m_bracePos));
+        event.Skip(true);
+        return;
+    }
 
+    if ( type != wxEVT_SCI_CHARADDED )
+        return;
+
+
+
     ed->AutoIndentDone(); // we are responsible.
 
     const int pos = stc->GetCurrentPos();
@@ -440,13 +458,56 @@
     }
     return false;
 }
+
 void CppSmartIndent::DoSelectionBraceCompletion(cbStyledTextCtrl* control, const wxChar &ch)const
 {
     if (!control->GetLastSelectedText().IsEmpty())
     {
-
         const int pos = control->GetCurrentPos();
-        wxString selectedText = control->GetLastSelectedText();
+        wxString selectedText = control->GetLastSelectedText();
+        Manager::Get()->GetLogManager()->Log(_T("Last Selected Text ")+selectedText);
+        if(selectedText == _T("(") ||
+           selectedText == _T(")") ||
+           selectedText == _T("[") ||
+           selectedText == _T("]") ||
+           selectedText == _T("{") ||
+           selectedText == _T("}")
+           )
+        {
+            int p = pos;
+            wxString opch;
+            switch (ch)
+            {
+                case _T('('):
+                    opch = _T(")");
+                    break;
+                case _T(')'):
+                    opch = _T("(");
+                    break;
+                case _T('['):
+                    opch = _T("]");
+                    break;
+                case _T(']'):
+                    opch = _T("[");
+                    break;
+                case _T('{'):
+                    opch = _T("}");
+                    break;
+                case _T('}'):
+                    opch = _T("{");
+                    break;
+            }
+            //int m = control->BraceMatch(p-1);
+            Manager::Get()->GetLogManager()->Log(wxString::Format(_T("Brace pos %i"),m_bracePos));
+            if (m_bracePos == wxSCI_INVALID_POSITION)
+                return;
+            control->BeginUndoAction();
+            control->InsertText(m_bracePos, opch);
+            control->DeleteRange(m_bracePos+1, 1);
+            control->SetCurrentPos(p);
+            control->EndUndoAction();
+            return;
+        }
         switch (ch)
         {
             case _T('\''):
@@ -547,6 +608,7 @@
         }
     } // SelectionBraceCompletion
 }
+
 void CppSmartIndent::DoBraceCompletion(cbStyledTextCtrl* control, const wxChar& ch)const
 {
     int pos = control->GetCurrentPos();

[attachment deleted by admin]

[dmoore]

doh, RTFM. wxEVT_SCI_KEY only triggers for keystrokes that aren't processed by scintilla. So I have to do this the same way I did it in EditorTweaks and handle wxEVT_CHAR or wxEVT_KEY_DOWN in order to get matching brace positions before the brace is replaced. I think I can make this work, but I'm even less sure it belongs in SmartIndent.

[dmoore]

I did what I probably should have just done to start with and fixed the implementation in EditorTweaks so that it doesn't conflict with SmartIndent. This version actually has better undo behavior because a single undo will revert both brace changes. Unless someone feels strongly that this feature shouldn't be in editor tweaks, it can stay there for now.

@Alpha: would still like to have the brace selection behavior for other languages beside C/C++. As obfuscated suggests, just move that method to the base class and have more (all?) of the smart indent implementations call it would be one way to do it.

[Alpha]

@Alpha: would still like to have the brace selection behavior for other languages beside C/C++. As obfuscated suggests, just move that method to the base class and have more (all?) of the smart indent implementations call it would be one way to do it.

Something like that; what I am planning is to have more generic base function that is called as a fall-back, and smart indent plugins would then (have the option to) specialize the behavior (for example, when used on a double quote ("), it currently utilizes C/C++ escapes, which may not be applicable to all languages).

If you want to work on this, go ahead.  Otherwise, I should have a patch available in a few days.

[dmoore]

If you want to work on this, go ahead.  Otherwise, I should have a patch available in a few days.

Quite content for you to work on this Happy to test of course.

[Alpha]

Here is a candidate patch.

Code

Index: src/plugins/contrib/SmartIndent/PythonSmartIndent.cpp
===================================================================
--- src/plugins/contrib/SmartIndent/PythonSmartIndent.cpp	(revision 8478)
+++ src/plugins/contrib/SmartIndent/PythonSmartIndent.cpp	(working copy)
@@ -69,4 +69,7 @@
             stc->EndUndoAction();
         }
     }
+
+    if ( SelectionBraceCompletionEnabled() || stc->IsBraceShortcutActive() )
+        ed->DoSelectionBraceCompletion(stc, ch);
 }
Index: src/plugins/contrib/SmartIndent/LuaSmartIndent.cpp
===================================================================
--- src/plugins/contrib/SmartIndent/LuaSmartIndent.cpp	(revision 8478)
+++ src/plugins/contrib/SmartIndent/LuaSmartIndent.cpp	(working copy)
@@ -67,6 +67,9 @@
             stc->EndUndoAction();
         }
     }
+
+    if ( SelectionBraceCompletionEnabled() || stc->IsBraceShortcutActive() )
+        ed->DoSelectionBraceCompletion(stc, ch);
 }
 
 bool LuaSmartIndent::BraceIndent(cbStyledTextCtrl *stc, wxString &indent)const
Index: src/plugins/contrib/SmartIndent/XMLSmartIndent.cpp
===================================================================
--- src/plugins/contrib/SmartIndent/XMLSmartIndent.cpp	(revision 8478)
+++ src/plugins/contrib/SmartIndent/XMLSmartIndent.cpp	(working copy)
@@ -47,16 +47,28 @@
 
     ed->AutoIndentDone(); // we are responsible
 
-    const int pos = stc->GetCurrentPos();
+    int pos = stc->GetCurrentPos();
     int currLine = stc->LineFromPosition(pos);
 
     const wxChar ch = event.GetKey();
     wxRegEx reTag(wxT("***:<[ \t]*?(|/)[ \t]*?([a-zA-Z][a-zA-Z0-9_-]*).*?(|/)[ \t]*?>"));
 
+    bool complQuote = true;
+    if ( SelectionBraceCompletionEnabled() || stc->IsBraceShortcutActive() )
+    {
+        ed->DoSelectionBraceCompletion(stc, ch);
+        if (pos != stc->GetCurrentPos())
+        {
+            complQuote = false;
+            pos = stc->GetCurrentPos();
+            currLine = stc->LineFromPosition(pos);
+        }
+    }
+
     if (BraceCompletionEnabled())
     {
         // finish tag
-        if (ch == wxT('>'))
+        if ( ch == wxT('>') && !stc->IsString(stc->GetStyleAt(pos)) )
         {
             wxString tag;
             for (int i = pos - 2; i > 0; --i)
@@ -72,7 +84,7 @@
                 stc->InsertText(pos, wxT("</") + reTag.GetMatch(tag, 2) + wxT(">"));
         }
         // close string
-        else if (ch == wxT('"') || ch == wxT('\''))
+        else if (complQuote && (ch == wxT('"') || ch == wxT('\'')))
         {
             if (stc->GetCharAt(pos) == ch)
             {
@@ -113,14 +125,14 @@
         }
     }
     // indent
-    if ( (ch == wxT('\n')) || ( (stc->GetEOLMode() == wxSCI_EOL_CR) && (ch == wxT('\r')) ) )
+    if (   AutoIndentEnabled()
+        && ( (ch == wxT('\n')) || ((stc->GetEOLMode() == wxSCI_EOL_CR) && (ch == wxT('\r'))) ) )
     {
-        if (AutoIndentEnabled())
+        wxString indent = ed->GetLineIndentString(currLine - 1);
+        stc->BeginUndoAction();
+        if (SmartIndentEnabled()) // smart indent
         {
-            wxString indent = ed->GetLineIndentString(currLine - 1);
             int idx = stc->GetLine(currLine - 1).Find(wxT('>'), true);
-
-            stc->BeginUndoAction();
             if (idx != wxNOT_FOUND)
             {
                 wxString tag;
@@ -167,10 +179,10 @@
                     }
                 }
             }
-            stc->InsertText(pos, indent);
-            stc->GotoPos(pos + indent.Length());
-            stc->ChooseCaretX();
-            stc->EndUndoAction();
         }
+        stc->InsertText(pos, indent);
+        stc->GotoPos(pos + indent.Length());
+        stc->ChooseCaretX();
+        stc->EndUndoAction();
     }
 }
Index: src/plugins/contrib/SmartIndent/PascalSmartIndent.cpp
===================================================================
--- src/plugins/contrib/SmartIndent/PascalSmartIndent.cpp	(revision 8478)
+++ src/plugins/contrib/SmartIndent/PascalSmartIndent.cpp	(working copy)
@@ -44,6 +44,9 @@
         DoIndent(ed, langname);   // indent because \n added
     else if ( ch != wxT(' ') )
         DoUnIndent(ed, langname); // un-indent because not a newline added
+
+    if ( SelectionBraceCompletionEnabled() || stc->IsBraceShortcutActive() )
+        ed->DoSelectionBraceCompletion(stc, ch);
 }
 
 void PascalSmartIndent::DoIndent(cbEditor* ed, const wxString& WXUNUSED(langname)) const
Index: src/plugins/contrib/SmartIndent/HDLSmartIndent.cpp
===================================================================
--- src/plugins/contrib/SmartIndent/HDLSmartIndent.cpp	(revision 8478)
+++ src/plugins/contrib/SmartIndent/HDLSmartIndent.cpp	(working copy)
@@ -49,6 +49,9 @@
         DoIndent(ed, langname);   // indent because \n added
     else if ( ch != wxT(' ') )
         DoUnIndent(ed, langname); // un-indent because not a newline added
+
+    if ( SelectionBraceCompletionEnabled() || stc->IsBraceShortcutActive() )
+        ed->DoSelectionBraceCompletion(stc, ch);
 }
 
 int HDLSmartIndent::FindBlockStartVHDL(cbEditor* ed, int position, wxString block) const
Index: src/plugins/contrib/SmartIndent/FortranSmartIndent.cpp
===================================================================
--- src/plugins/contrib/SmartIndent/FortranSmartIndent.cpp	(revision 8478)
+++ src/plugins/contrib/SmartIndent/FortranSmartIndent.cpp	(working copy)
@@ -103,4 +103,7 @@
 
         stc->EndUndoAction();
     }
+
+    if ( SelectionBraceCompletionEnabled() || stc->IsBraceShortcutActive() )
+        ed->DoSelectionBraceCompletion(stc, ch);
 }
Index: src/include/cbeditor.h
===================================================================
--- src/include/cbeditor.h	(revision 8478)
+++ src/include/cbeditor.h	(working copy)
@@ -301,6 +301,7 @@
         static void ApplyStyles(cbStyledTextCtrl* control);
 
         void AutoIndentDone();
+        void DoSelectionBraceCompletion(cbStyledTextCtrl* control, const wxChar& ch) const;
     private:
         cbEditor(const cbEditor& /*rhs*/); // prevent copy construction
 
Index: src/sdk/cbeditor.cpp
===================================================================
--- src/sdk/cbeditor.cpp	(revision 8478)
+++ src/sdk/cbeditor.cpp	(working copy)
@@ -3095,12 +3095,33 @@
                 control->EndUndoAction();
             }
         }
+        if(   Manager::Get()->GetConfigManager(_T("editor"))->ReadBool(_T("/brace_completion"), true)
+           || control->IsBraceShortcutActive())
+            DoSelectionBraceCompletion(control, ch);
     }
 }
 void cbEditor::AutoIndentDone()
 {
     m_autoIndentDone = true;
 }
+void cbEditor::DoSelectionBraceCompletion(cbStyledTextCtrl* control, const wxChar& ch) const
+{
+    if (control->GetLastSelectedText().IsEmpty())
+        return;
+    const wxString braces(wxT("([{<'\")]}>'\""));
+    const int braceAIdx = braces.Find(ch, true); // from end (so caret ends after quotes)
+    if (braceAIdx == wxNOT_FOUND)
+        return;
+    const int braceBIdx = (braceAIdx + (braces.Length() / 2)) % braces.Length();
+    control->BeginUndoAction();
+    control->DeleteBack();
+    if (braceAIdx < braceBIdx)
+        control->InsertText(control->GetCurrentPos(),
+                            braces[braceAIdx] + control->GetLastSelectedText() + braces[braceBIdx]);
+    else
+        control->AddText(braces[braceBIdx] + control->GetLastSelectedText() + braces[braceAIdx]);
+    control->EndUndoAction();
+}
 
 void cbEditor::OnEditorDwellStart(wxScintillaEvent& event)
 {

[MortenMacFly]

Here is a candidate patch.

dmoore: Do you take action?

[dmoore]

Here is a candidate patch.

dmoore: Do you take action?

I haven't but I can. Might take a few days so if you get to it sooner be my guest. Anyone else tested it?