Developer forums (C::B DEVELOPMENT STRICTLY!) > Plugins development
ThreadSearch plugin is born !
jamieo:
Nice plugin! The preview idea is very cool (minimal tab clutter), thanks for sharing. :)
I have some suggestions I hope you'll find useful, although it's possible they are already on your todo list...
Background colour
The hard coded XP background colour looks a bit out of place on Vista - no doubt the same is true on other platforms and themes. This is an easy fix for win32 using 'GetSysColor' but I'm unsure if there is an equivalent function for other platforms (if one is even needed)... I found the following wxWidgets implementation, if colours are normally correct on other platforms then perhaps it's good enough?
--- Code: ---ThreadSearchView.cpp
..
..
void ThreadSearchView::set_properties()
{
// Set correct background colour on win32 systems
SetBackgroundColour(wxSystemSettings::GetColour(wxSYS_COLOUR_BTNFACE));
..
--- End code ---
Search Results
The filename is not visible in the thread search results listview as the preceding path takes up what little space there is, an option to show only the filename along with saving column widths would be a huge improvement.
The preview normally shows the highlighted line at the bottom (depending on what was previously shown) - would it be possible for the line to be initially set in the middle each time? ie, if 5 lines are visible, the 3rd line is the one highlighted with the search term. A program I use called Search & Replace does this and it's good for quickly getting the context of the result. It might be tricky knowing how many lines are visible but perhaps you could have an option which sets the amount of lines above the highlight - in this case you would set it to 2.
An improvement of lower priority would be to maximise the vertical space usable for search results as it is at such a premium in the messages window. A good idea for this would be to (optionally?) relocate the controls to an advanced search dialogue and instead provide a search toolbar similar to visual studio etc. The toolbar could be as simple as a combobox (pressing enter performs search with existing options) and a button which opens the new dialogue.
Bug report
Keyboard shortcuts (Ctrl+V/X/C/Z etc.) are sent to the active editor when caret is in the thread search combobox. This is unexpected and awkward for pasting search terms etc.
Not really a bug but on my system a search which produces a lot of hits (eg. 'if') occupies the CPU such that codeblocks and (more annoyingly) the cancel search button is unresponsive until the search is complete. I think it should be possible to correct this without incurring a significant performance hit.
Thanks again for sharing your work, I hope you appreciate my suggestions and I look forward to your views as well as any future releases.
Jamie
dje:
Hi all !
Sorry... I didn't know I was a bug :) !
I'll replace all the Jérôme by Jerome !
Pecan, could you explain what this correction change ?
--- Quote ---Even though the cbSTC is hidden, aren't GUI calls being made from this background thread?
--- End quote ---
Well, I do not have the answer to this interesting question.
As I use the view as parent during hidden cbSTC creation whereas it is not useful, I'll replace it by NULL to limit gui interactions:
--- Code: ---cbStyledTextCtrl* control = new cbStyledTextCtrl(m_pThreadSearchView, -1, wxDefaultPosition, wxSize(0, 0));
--- End code ---
=>
--- Code: ---cbStyledTextCtrl* control = new cbStyledTextCtrl(NULL, -1, wxDefaultPosition, wxSize(0, 0));
--- End code ---
--- Quote ---Keyboard shortcuts (Ctrl+V/X/C/Z etc.) are sent to the active editor when caret is in the thread search combobox. This is unexpected and awkward for pasting search terms etc.
--- End quote ---
This has been corrected with patch 1704 integration.
I think your nightly may be too old.
--- Quote ---Not really a bug but on my system a search which produces a lot of hits (eg. 'if') occupies the CPU such that codeblocks and (more annoyingly) the cancel search button is unresponsive until the search is complete. I think it should be possible to correct this without incurring a significant performance hit.
--- End quote ---
Yes, that's a limitation.
The worker thread sends events to the view to add found lines.
One event per file is sent.
As click or other events are enqueued after worker thread events, the application hangs if you search after an expression present in lots of files.
I would like to be able to specify a lower priority to the thread events but I think events handlers do not manage events priority.
--- Quote ---wxSystemSettings::GetColour(wxSYS_COLOUR_BTNFACE)
--- End quote ---
This works with
--- Code: ---wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOW)
--- End code ---
on my XP environment.
I'll test it (not today) on my Linux environment.
--- Quote ---The filename is not visible in the thread search results listview as the preceding path takes up what little space there is, an option to show only the filename along with saving column widths would be a huge improvement.
--- End quote ---
Yes, I'll think about it. At first glance, it may be configurable so that user can choose between the two solutions (I think directory is often useful).
On XP, I just put the cursor on the file path so that the tool tip gives me the full path.
--- Quote ---The preview normally shows the highlighted line at the bottom
--- End quote ---
I already saw that point. I'll try to improve it but I think that it's lower priority than many other points.
I really like the toolbar idea but my first priority is to make this plugin run on "other than Windows" platforms.
Finally, thanks for your positive feedback and your constructive remarks/propositions.
Dje
Pecan:
--- Quote from: Pecan on March 17, 2007, 06:20:10 pm ---I have a fundamental, maybe stupid, question.
ThreadSearch does the searches using a hidden cbStyledTextCtrl.
Threads are not allowed to do GUI calls without carefull critical sections.
Even though the cbSTC is hidden, aren't GUI calls being made from this background thread?
--- End quote ---
The reason I ask the question is this from the wxWidgets manual:
--- Quote ---::wxMutexGuiEnter
void wxMutexGuiEnter()
This function must be called when any thread other than the main GUI thread wants to get access to the GUI library. This function will block the execution of the calling thread until the main thread (or any other thread holding the main GUI lock) leaves the GUI library and no other thread will enter the GUI library until the calling thread calls ::wxMutexGuiLeave().
Typically, these functions are used like this:
void MyThread::Foo(void)
{
// before doing any GUI calls we must ensure that this thread is the only
// one doing it!
wxMutexGuiEnter();
// Call GUI here:
my_window->DrawSomething();
wxMutexGuiLeave();
}
Note that under GTK, no creation of top-level windows is allowed in any thread but the main one.
This function is only defined on platforms which support preemptive threads
--- End quote ---
When I attempt to run ThreadSearch under Linux, I get all sorts of freezes and segment violations as it tries to update/paint the cbStyledTextCtrl.
My thought is: that you cannot allocate a GUI control in a secondary thread. Nor can you cause it to update itself or paint. That's a gui call.
IE, you should not use a GUI editor in the secondary thread.
That may be why it crashes/hangs Linux CB.
I had this problem trying to write a wxTextCtrl log in KeyMacs. I had to completely remove the writes because they caused GUI calls to the wxTextCtrl.
I finally solved the problem with wxMutexGuiEnter/Leave, but I had to take the allocation of the text control out of the secondary thread.
jamieo:
--- Quote from: dje on March 18, 2007, 10:57:07 pm ---This has been corrected with patch 1704 integration.
I think your nightly may be too old.
--- End quote ---
Weird, my nightly (svn 3720) is from yesterday! :shock: I will download today's and try a clean install...
Update: Downloaded CB_20070318_rev3721_win32, extracted it to it's own folder and added mingwm10.dll, wxmsw26u_gcc_cb.dll and your unmodified plugin. Moved my profile folder elsewhere, started codeblocks, loaded a project, copied some text from the editor, clicked inside the threadsearch combobox, hit ctrl+v, text is pasted to my last position in the editor! I'm on Vista so could somebody else please try ctrl+v in the combobox?
PS. I had a quick look at patch 1704 and it's for 'OnContextMenu' while my problem involves the keyboard shortcuts...
--- Quote ---Yes, that's a limitation.
The worker thread sends events to the view to add found lines.
One event per file is sent.<snip>
--- End quote ---
I guessed that was the case, on win32 I have used 'WaitForSingleObject' with 1ms time-out to work around this. The PSDK recommends 'WaitForMultipleObjects' if the wait is for any length of time but it is unnecessary when used with a short time-out. In very fast loops I have set this up to execute on every nth iteration. With a little tuning it should give the gui enough cycles to run smoothly with little impact on performance if any (although I have no numbers to prove it!).
--- Quote ---wxSystemSettings::GetColour(wxSYS_COLOUR_WINDOW)
--- End quote ---
Hmm.. are you saying COLOUR_BTNFACE is incorrect on your system for the beige xp coloured background as featured in your plugin and the screenshots previously in this thread? I think COLOUR_WINDOW is an older pre-win95 constant - eg. the background of 'Program Manager' in Windows 3.11?! On my system at least it results in a pure white background that looks a bit ugly and washed out (see attached png). I don't have XP installed to check but looking at some software I've written in the past I've always used 'COLOR_BTNFACE' or it's alias 'COLOR_3DFACE' to match the same themed background you had hard coded. Also, a drawback of using a colour that doesn't match the themed background is that themed buttons will have a 'halo' of colour around the button that won't match (please see magnified examples in the png).
Here's what the PSDK says about these constants.
--- Code: ---COLOR_3DFACE = 15
Face color for three-dimensional display elements and for dialog box backgrounds.
COLOR_BTNFACE = 15
Face color for three-dimensional display elements and for dialog box backgrounds.
COLOR_WINDOW = 5
Window background.
--- End code ---
--- Quote ---it may be configurable so that user can choose between the two solutions (I think directory is often useful).
--- End quote ---
Yes, an option for either would be great.
--- Quote ---
--- Quote ---The preview normally shows the highlighted line at the bottom
--- End quote ---
I already saw that point. I'll try to improve it but I think that it's lower priority than many other points.
I really like the toolbar idea but my first priority is to make this plugin run on "other than Windows" platforms.
Finally, thanks for your positive feedback and your constructive remarks/propositions.
--- End quote ---
As I said, the toolbar idea would be a low priority suggestion as it is purely a gui thing and would be more than just a small mod. I'm glad you appreciate my feedback, perhaps I'll look into some of the low priority ideas (eg. highlighted line) to see if I can contribute a patch or two, if you agree. Although, I have not done any wxWidgets so don't count on anything great, anytime soon! :lol:
Jamie
[attachment deleted by admin]
jamieo:
Here's a suggestion for achieving a consistent line position in the search preview and having it offset an optional number of lines.
You do have to have another 'GotoLine' but as I believe they both occur before a repaint there should be no impact on update speed. Certainly on my machine (1.7Ghz Centrino Laptop) it was just as quick as before - which is instant! Optimally, you would use an 'if' condition to check the initial 'GotoLine' as it's only required if you previously viewed the same file from a position later in the file - but as there was no noticeable performance hit I kept is simple for the sake of clarity.
The attached png shows the result of previewing a line. So far while using the up/down arrows to traverse (quickly) up and down the results the line is always bang in the middle. :)
--- Code: ---ThreadSearchView.cpp
In ThreadSearchView::UpdatePreview replace:
// Display the selected line
m_pSearchPreview->GotoLine(line);
with:
// Display the selected line // Preview sets position inconsistently depending on the previous
m_pSearchPreview->GotoLine(0); // position so first, set line to a position before the destination,
m_pSearchPreview->GotoLine(line+2); // then set it to the destination postion + any desired offset.
--- End code ---
Jamie
[attachment deleted by admin]
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version