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

[Alpha]

Apologies for spacing and forgetting to give you credit in the log message, Alpha.

No worries.

[Alpha]

Fall-back for brace completion would have to be very conservative, because languages differ so greatly.

Here is a fall-back (plus a small fix for the previous patch - it had read from the wrong environment variable).  The language specific implementations will take some more time (and probably will require help from people who use those languages more frequently).

Code: [Select]

Index: src/sdk/cbeditor.cpp
===================================================================
--- src/sdk/cbeditor.cpp (revision 8496)
+++ src/sdk/cbeditor.cpp (working copy)
@@ -3076,9 +3076,9 @@
     {
         const wxChar ch = event.GetKey();
         cbStyledTextCtrl* control = GetControl();
+        const int pos = control->GetCurrentPos();
         if ( (ch == _T('\n')) || ( (control->GetEOLMode() == wxSCI_EOL_CR) && (ch == _T('\r')) ) )
         {
-            const int pos = control->GetCurrentPos();
             const int currLine = control->LineFromPosition(pos);
             const bool autoIndent = Manager::Get()->GetConfigManager(_T("editor"))->ReadBool(_T("/auto_indent"), true);
             if (autoIndent && currLine > 0)
@@ -3093,9 +3093,27 @@
                 control->EndUndoAction();
             }
         }
-        if(   Manager::Get()->GetConfigManager(_T("editor"))->ReadBool(_T("/brace_completion"), true)
+        if(   Manager::Get()->GetConfigManager(wxT("editor"))->ReadBool(wxT("/selection_brace_completion"), false)
            || control->IsBraceShortcutActive())
             DoSelectionBraceCompletion(control, ch);
+        // brace completion
+        const wxString braces(wxT("([{)]}"));
+        const int braceIdx = braces.Find(ch);
+        if (   braceIdx != wxNOT_FOUND && pos == control->GetCurrentPos() // pos != curPos if selection brace completion succeeded
+            && Manager::Get()->GetConfigManager(wxT("editor"))->ReadBool(wxT("/brace_completion"), true))
+        {
+            if (control->GetCharAt(pos) == ch)
+            {
+                control->DeleteBack();
+                control->CharRight();
+            }
+            else if (braceIdx < (braces.Length() / 2))
+            {
+                const int closeIdx = braceIdx + (braces.Length() / 2);
+                if (control->GetCharAt(pos) != braces[closeIdx] || control->BraceMatch(pos) != wxNOT_FOUND)
+                    control->InsertText(pos, braces[closeIdx]);
+            }
+        }
     }
 }
 
I did not make this a separate function because most languages will have very different requirements, so it would therefore remain relatively unused; do you agree?

[dmoore]

I did not make this a separate function because most languages will have very different requirements, so it would therefore remain relatively unused; do you agree?

Busy ATM but will take a look in the next few days. In terms of different languages, I would think it's rare to find a language that doesn't expect brace chars to be closed. So that seems like something that should always happen when this feature is turned on, which it appears is what your patch does. I would say same with quote chars, but scintilla doesn't offer a nice bracematch function for quotes so would require more work.

I guess the tricky thing is that you probably don't want auto brace completion inside a string or comment, and different lexers have different style codes for their strings, but this would be hard to do right. (Doesn't look like your patch deals with this?)

[danselmi]

I guess the tricky thing is that you probably don't want auto brace completion inside a string or comment, and different lexers have different style codes for their strings, but this would be hard to do right. (Doesn't look like your patch deals with this?)

Code: [Select]

cbStyledTextCtrl::IsComment() and

Code: [Select]

cbStyledTextCtrl::IsString() should help

[Alpha]

Thanks.

Attached is another patch.
It handles quotes (with guess work that is often, but not always, close enough) and attempts to act more intelligently during brace completion.  I have also now separated it into a function, and called the function from the SmartIndent plugins that have not yet implemented their own brace completion.

I still want to write some specialized functions; the fall-back function attempts to be generalized, so it misses many language specifics (for example triple quoted python strings).

[Alpha]

Here is python triple quote completion (it requires the patch from my last post be applied to everything but PythonSmartIndent.cpp).

Code: [Select]

Index: src/plugins/contrib/SmartIndent/PythonSmartIndent.cpp
===================================================================
--- src/plugins/contrib/SmartIndent/PythonSmartIndent.cpp (revision 8500)
+++ src/plugins/contrib/SmartIndent/PythonSmartIndent.cpp (working copy)
@@ -71,6 +71,18 @@
         }
     }
 
+    bool braceCompleted = false;
     if ( SelectionBraceCompletionEnabled() || stc->IsBraceShortcutActive() )
-        ed->DoSelectionBraceCompletion(stc, ch);
+        braceCompleted = ed->DoSelectionBraceCompletion(stc, ch);
+    if (!braceCompleted && BraceCompletionEnabled())
+    {
+        ed->DoBraceCompletion(stc, ch);
+        if (  !(stc->IsComment(stc->GetStyleAt(pos)) || stc->IsComment(stc->GetStyleAt(pos - 2)))
+            && (ch == wxT('"') || ch == wxT('\'')) )
+        {
+            const wxString tripleQuote(3, ch);
+            if (stc->GetTextRange(pos - 3, pos) == tripleQuote && !stc->IsString(stc->GetStyleAt(pos - 4)))
+                stc->InsertText(pos, tripleQuote);
+        }
+    }
 }

I also wanted to add term -> end term "brace" completion (similar to how C++ matches PP directives, and XML matches tags), but I realized that I do not use/understand those languages enough to be able to create something that functions as expected.

[dmoore]

I tried your last patch last night and it seemed to be working well on python files. So should I take this new patch, test a little more then commit over the weekend?

One thing I wondered about was whether you it is better add the sdk functions to cbEditor (as you do now) or to cbStyledTextCtrl. The advantage of the latter is that you don't need to pass around the redundant cbStyleTextCtrl pointer as the first arg.

[Alpha]

Assuming it passes your testing, I believe the patch is ready for commit.

Attached I have refactored to cbStyledTextCtrl (plus a slight improvement to brace completion).

[dmoore]

Tested and works well enough for me.

Unless someone has objections to putting the Do*BraceCompletion methods in cbStyleTextCtrl or finds some other major problem, I will commit within the next 24 hours.  (The alternative would be to put the methods in cbEditor but I think this is stuff that doesn't need to be exposed in the SDK to anything other than smart indent plugins, so hiding them away makes some sense.)

[dmoore]

aaargh! The commit would be a big mess of line ending changes on Linux because I had to dos2unix before applying the patch. (Some of the line ending changes might be unavoidable due to the changes to xml lexer files)

@Morten: please do the propset magic if you have time. (I would do it myself, but only have linux access ATM which will commit a huge batch of unwanted line ending changes if I do the propset)

EDIT: In case I wasn't clear, I am going to hold off on the commit until I resolve the line endings.

Never mind. I figured this out and made the eol propset changes.

[MortenMacFly]

Tested and works well enough for me.

It doesn't compile under wx2.9.x. this line:
if (GetCharAt(nextPos) != braces[closeIdx] || BraceMatch(nextPos) != wxNOT_FOUND)

...raises:
cbstyledtextctrl.cpp:356:54: error: ambiguous overload for 'operator!=' in '((cbStyledTextCtrl*)this)->cbStyledTextCtrl::<anonymous>.wxScintilla::GetCharAt(nextPos) != braces.wxString::operator[](((int)closeIdx))'


This should be fixed before committing...

[Alpha]

I do not currently have wx2.9 available for testing; does a cast fix this?

if ((wxChar)GetCharAt(nextPos) != braces[closeIdx] || BraceMatch(nextPos) != wxNOT_FOUND)

[MortenMacFly]

if ((wxChar)GetCharAt(nextPos) != braces[closeIdx] || BraceMatch(nextPos) != wxNOT_FOUND)

Yes. Not nice and I don't know if it would work for all Unicode characters... but yes. :-)

[MortenMacFly]

Attached I have re-factored to cbStyledTextCtrl (plus a slight improvement to brace completion).

Oh dear... I'm afraid I just screwed the compatibility with the patch after the last commits of mine (renaming of SmartIndent). Maybe you can provide an updated version? Sorry - I didn't have that in mind.

[dmoore]

Ok, I'm leaving this one to you guys. (Let me know if you want me to look at anything specific.)