Patch #978 2006-04-01 13:16
dermeister
Make Search->Find and Search->Find in files more consistent- Download
- 978-Make_Search_Fin.patch (3.1 KB)
Index: src/sdk/finddlg.cpp
===================================================================
--- src/sdk/finddlg.cpp (revision 2366)
+++ src/sdk/finddlg.cpp (working copy)
@@ -57,7 +57,7 @@
EVT_ACTIVATE( FindDlg::OnActivate)
END_EVENT_TABLE()
-FindDlg::FindDlg(wxWindow* parent, const wxString& initial, bool hasSelection, bool findInFilesOnly)
+FindDlg::FindDlg(wxWindow* parent, const wxString& initial, bool hasSelection, bool findInFilesOnly, bool findInFilesActive)
: FindReplaceBase(parent, initial, hasSelection),
m_Complete(!findInFilesOnly)
{
@@ -113,6 +113,11 @@
if (!m_Complete)
XRCCTRL(*this, "nbFind", wxNotebook)->DeletePage(0); // no active editor, so only find-in-files
+ else if (findInFilesActive)
+ {
+ XRCCTRL(*this, "nbFind", wxNotebook)->SetSelection(1); // Search->Find in Files was selected
+ XRCCTRL(*this, "cmbFind2", wxComboBox)->SetFocus();
+ }
}
FindDlg::~FindDlg()
@@ -125,9 +130,11 @@
cfg->Write(CONF_GROUP _T("/search_hidden"), XRCCTRL(*this, "chkSearchHidden", wxCheckBox)->GetValue());
// save last searches (up to 10)
- wxComboBox* combo = XRCCTRL(*this, "cmbFind1", wxComboBox);
- if (!m_Complete)
+ wxComboBox* combo;
+ if (IsFindInFiles())
combo = XRCCTRL(*this, "cmbFind2", wxComboBox);
+ else
+ combo = XRCCTRL(*this, "cmbFind1", wxComboBox);
wxArrayString previous;
for (int i = 0; (i < combo->GetCount()) && (i < 10); ++i)
{
@@ -311,7 +318,8 @@
void FindDlg::OnActivate(wxActivateEvent& event)
{
- if (!m_Complete && XRCCTRL(*this, "cmbFind2", wxComboBox))
+ // Why is there the second part in the expression? Is this really necessary?
+ if (IsFindInFiles() && XRCCTRL(*this, "cmbFind2", wxComboBox))
XRCCTRL(*this, "cmbFind2", wxComboBox)->SetFocus();
else
XRCCTRL(*this, "cmbFind1", wxComboBox)->SetFocus();
Index: src/sdk/finddlg.h
===================================================================
--- src/sdk/finddlg.h (revision 2366)
+++ src/sdk/finddlg.h (working copy)
@@ -10,7 +10,7 @@
class FindDlg : public FindReplaceBase
{
public:
- FindDlg(wxWindow* parent, const wxString& initial = wxEmptyString, bool hasSelection = false, bool findInFilesOnly = false);
+ FindDlg(wxWindow* parent, const wxString& initial = wxEmptyString, bool hasSelection = false, bool findInFilesOnly = false, bool findInFilesActive = false);
~FindDlg();
wxString GetFindString() const;
wxString GetReplaceString() const{ return wxEmptyString; }
Index: src/sdk/editormanager.cpp
===================================================================
--- src/sdk/editormanager.cpp (revision 2366)
+++ src/sdk/editormanager.cpp (working copy)
@@ -1109,7 +1109,7 @@
FindReplaceBase* dlg;
if (!replace)
- dlg = new FindDlg(Manager::Get()->GetAppWindow(), phraseAtCursor, hasSelection, explicitly_find_in_files || !ed);
+ dlg = new FindDlg(Manager::Get()->GetAppWindow(), phraseAtCursor, hasSelection, !ed, explicitly_find_in_files);
else
dlg = new ReplaceDlg(Manager::Get()->GetAppWindow(), phraseAtCursor, hasSelection);
History
When you open the find-dialog with "Search->Find" you see both tabs - one for "Find" and one for "Find in files" - in the dialog and the tab for "Find" is selected.
But if you open the same dialog with "Search->Find in files" there is only the tab for "Find in files" even if there is an open editor. It would be better if both tabs are there (as long as there is an open editor) and the tab for "Find in files" should be selected.
This patch implements this behaviour.
Hmm, patch doesn't apply (failed hunks)...
I just updated the patch. It sould now apply to revision 2321.
reverted from svn for now, because the provided patch introduces a bug, which prevents the Search-pattern edit control getting the focus. This is only one (visible) bug!
Other side-effects exist too because the FindDlg::m_Complete flag is used in all event handlers to decide if we are in "Find" or in "Find in Files" mode.
Will look at this and possibly correct it later ...
As far as I can see only the destructor uses FindDlg::m_Complete to check if Find in Files was selected (should be around line 130 in finddlg.cpp). In my opinion this is *not* correct. The function FindDlg::IsFindInFiles should be used instead because this function does the correct check.
The current way would even fail if you open the FindDlg with "Search->Find" and then select the FindInFiles-page.
The same issue appears in FindDlg::OnActivate. This function sets the focus to the wrong edit control if you do the same thing as described above. Using FindDlg::IsFindInFiles instead of FindDlg::m_Complete should fix this issue.
One word to the focus bug: It looks like the edit control never gets the focus if you switch between the pages. And that is what the constructor does if my original patch is applied and "Search->FindInFiles" is selected. Maybe we have to set the focus after the "...->SetSelection(1);"-line from my patch? Or perhaps set it always to the edit control after someone selects another page?
I'll take another look at this problem this evening and probably provide another (hopefully better) patch...
OK, looks as if I was right. I made a few changes according to my last comment and it seems to work. I could not reproduce any of the described issues. The patch here is updated and should apply to revision 2366.
Just an additional note: I'm not sure about the expression in the if-statement in FindDlg::OnActivate. The second part (XCONTROL(... ) looks useless to me - or did I miss something? Maybe some possible race-conditions during construction? But if that's the reason for this check then it should also be checked in the else-branch for the same reason...
ad "I'm not sure about the expression in the if-statement in FindDlg::OnActivate. The second part (XCONTROL(... ) looks useless to me..." :
this is used to prevent a crash when the dialog wasn't created, this happens when no project or no editor of a project is opened. i recoded this part and added a check for null pointers.
general:
there are still a few bugs left,
* switching between tabs (find) && (find in files) - the editcontrol doesn't get the focus
* pressing Ctrl-F or Ctrl-Shift-F without active project - the dialog appears (should not) - but it doesn't crash C::B ;-)
* some occurances of m_Complete are left
from my point of view the Handling with IsFindInFiles() and Complete is still buggy - we would need both flags as private flags for example (bool m_findInFilesOnly, bool m_findInFilesActive) in order to clean up all possibilities, but i don't care at the moment.
@dermeister: if you have time please clean it up
your patch is commited to svn rev 2373
thanks for your patch
"* switching between tabs (find) && (find in files) - the editcontrol doesn't get the focus"
Yes, but I think this was the case before, too. I didn't see any code that handles the switching between the tabs to set the focus to the edit control. Maybe we should add this?
"* pressing Ctrl-F or Ctrl-Shift-F without active project - the dialog appears (should not) - but it doesn't crash C::B ;-)"
Well - I did not test this. Does this also happen without this patch? And which dialog should not appear? The find-dialog? The find-in-files-dialog? Or Both? Personally I would say that "find in files" should even work without any project and "find" should work only with open editors - no matter if there is an open project.
"* some occurances of m_Complete are left"
Yes, but they are there to see if the "find" tab is there or not - they are not used to check which tab is active! Thus, these occurances *must* stay...
"from my point of view the Handling with IsFindInFiles() and Complete is still buggy - we would need both flags as private flags for example (bool m_findInFilesOnly, bool m_findInFilesActive) in order to clean up all possibilities, but i don't care at the moment.
@dermeister: if you have time please clean it up"
Well, I will have a look at this but I fear I don't have the time in the next two weeks... sorry for that...