Issue about wxScintilla

[Loaden]

Hi, all!
I encountered a problem about scintilla.

void CodeRefactoring::Find(cbStyledTextCtrl* control, const wxString& file, const wxString& target)
{
   const int end = control->GetLength();
   int start = 0;
   int pos = 0;

   for (;
   {
       int lengthFound;
       pos = control->FindText(start, end, target, wxSCI_FIND_WHOLEWORD | wxSCI_FIND_MATCHCASE, &lengthFound);
       if (pos != wxSCI_INVALID_POSITION)
       {
           start = pos + lengthFound;

           // TODO (Loaden) not work?
           const int style = control->GetStyleAt(pos); // always been zero?
           if (control->IsString(style) || control->IsComment(style))
               continue;

           int line = control->LineFromPosition(pos);
           wxString text = control->GetLine(line).Trim(true).Trim(false);
           m_SearchDataMap[file].push_back(crSearchData(pos, line, text));
       }
       else
           break;
   }
}

Please see the bold style code, why the return value always been zero.
Any comment are welcome!

Example code:

Code Select Expand

int test()
{
   // test
   printf("test");
}
When call control->FindText, we will get three result, include the comment(test) and string (test).

Here are "find references" result:

main.cpp|1|int test()|
main.cpp|3|// test|
main.cpp|4|printf("test");|

[Jenna]

I did a quick test with trunk (modified my IncrementalSearch-plugin to spit out the style for every found string, and I get:

Code Select Expand

pos: 4; style: 11
pos: 20; style: 2
pos: 37; style: 6
three different styles.

[MortenMacFly]

Code Select Expand

pos: 4; style: 11
pos: 20; style: 2
pos: 37; style: 6
three different styles.

Same here with a dummy project (plugin), however, debugging into the code Loaden mentioned I also get zero always. So the root must be somewhere else, not in scintilla.

[ollydbg]

debugging into the code Loaden mentioned I also get zero always.

Confirmed. strange...

[Jenna]

debugging into the code Loaden mentioned I also get zero always.

Confirmed. strange...

No, not strange.
The code do not use the cbEditor, but just the cbStyledTextctrl, but without a lexer and without highlighting any code, so GetStyleAt can not return anything but the default value, because the text is not styled.

The following patch should do the trick:

Code Select Expand

Index: coderefactoring.cpp
===================================================================
--- coderefactoring.cpp (Revision 6654)
+++ coderefactoring.cpp (Arbeitskopie)
@@ -156,6 +156,9 @@
                 continue; // failed
             control->SetText(detector.GetWxStr());
         }
+        cbEditor::ApplyStyles(control);
+        EditorColourSet EdColSet;
+        EdColSet.Apply(EdColSet.GetLanguageForFilename(files[i]), control);

         Find(control, files[i], targetText);
     }


Shall I commit it ?

[ollydbg]

thanks jen for the explanation.
Yes, I found that in coderefactoring.cpp line 131.
There are code snippet

Code Select Expand

    // now that list is filled, we'll search
    cbStyledTextCtrl* control = new cbStyledTextCtrl(editor->GetParent(), wxID_ANY, wxDefaultPosition, wxSize(0, 0));


Shall I commit it ?

I full agree.

[Loaden]

debugging into the code Loaden mentioned I also get zero always.

Confirmed. strange...

No, not strange.
The code do not use the cbEditor, but just the cbStyledTextctrl, but without a lexer and without highlighting any code, so GetStyleAt can not return anything but the default value, because the text is not styled.

The following patch should do the trick:

Code Select Expand

Index: coderefactoring.cpp
===================================================================
--- coderefactoring.cpp (Revision 6654)
+++ coderefactoring.cpp (Arbeitskopie)
@@ -156,6 +156,9 @@
                 continue; // failed
             control->SetText(detector.GetWxStr());
         }
+        cbEditor::ApplyStyles(control);
+        EditorColourSet EdColSet;
+        EdColSet.Apply(EdColSet.GetLanguageForFilename(files[i]), control);

         Find(control, files[i], targetText);
     }


Shall I commit it ?

Thank Jens!
I can't solved this issue so long time.
I am very happy to see what is the reason.
Thanks a lot!

But, I am worry about the search performance?
So, I want look into it.

[Jenna]

After a quick test with ThreadSearch-plugin and search for wxString, searching is about 25 times slower: ~400ms against ~9800ms .

I just used wxStropWatch and measured the for-loop in SearchInFiles about line 142.

[ollydbg]

I think:
the lexer/parser in scintilla is not as fast as CC's parser. So, I'd rather add a function in Parserthread class to do the search. Currently all the comments were skipped in CC's Tokenizer.

[MortenMacFly]

After a quick test with ThreadSearch-plugin and search for wxString, searching is about 25 times slower: ~400ms against ~9800ms .

Currently all the comments were skipped in CC's Tokenizer.

Which makes me wonder: Wat's the actual purpose of using the styles in scintilla to quey for comments? I mean: This can only be less performant...?!

[ollydbg]

Which makes me wonder: Wat's the actual purpose of using the styles in scintilla to quey for comments? I mean: This can only be less performant...?!

The re-factoring model was designed by Loaden, so I think he can some idea. See:

Code Select Expand

int test()
{
    // test
    printf("test");
}

A simple plain text search will give 3 matches.
But in-fact, the one we interested was only the function name. So, both matched text result in comments and c-strings should be removed. Sometimes, the matched text in comments should also be involved.
Currently the only way is do a query to scintilla.

I'm thinking a better way by CC.

[Loaden]

Which makes me wonder: Wat's the actual purpose of using the styles in scintilla to quey for comments? I mean: This can only be less performant...?!

The re-factoring model was designed by Loaden, so I think he can some idea. See:

Code Select Expand

int test()
{
   // test
   printf("test");
}

A simple plain text search will give 3 matches.
But in-fact, the one we interested was only the function name. So, both matched text result in comments and c-strings should be removed. Sometimes, the matched text in comments should also be involved.
Currently the only way is do a query to scintilla.

I'm thinking a better way by CC.

Thank ollydbg explained.
I found a way to solved, will be less performant littler.

We can't solved this issue by CC.
It's only scintilla related.

EDIT:
Sorry! Found a bug, fixed now.

[Loaden]

Sorry! Found a bug, fixed now.

[Loaden]

More improved.
This is the final version.