#1762 [SciTEGTK.cxx] Fix bug #1254 find.close.on.find=0 doesn't work on Gtk

[jacky yang]

Fix bug #1254 find.close.on.find=0 doesn't work on Gtk

You could see it.
bug #1254 https://sourceforge.net/p/scintilla/bugs/1254/

When I run SciTE 3.6.0 on Fedora, that happens again.

The code in scite/src/SciTEBase.cxx :

int SciTEBase::FindNext(bool reverseDirection, bool showWarnings, bool allowRegExp) {*

...

        if (!replacing && closeFind) {
            DestroyFindReplace();
        }
    }
    return posFind;
}

seems that there is no problem.

But You could find that in scite/gtk/SciTEGTK.cxx :

void SciTEGTK::DestroyFindReplace() {
    dlgFindReplace.Destroy();
}

void SciTEGTK::FRFindCmd() {
FindReplaceGrabFields();
bool isFindDialog = !dlgFindReplace.wComboReplace;
if (isFindDialog)
dlgFindReplace.Destroy();
if (findWhat[0]) {
FindNext(isFindDialog && reverseFind);
}
}

So You can repalce it with :

void SciTEGTK::FRFindCmd() {
FindReplaceGrabFields();
bool isFindDialog = !dlgFindReplace.wComboReplace;
if (isFindDialog && closeFind)
dlgFindReplace.Destroy();
if (findWhat[0]) {
FindNext(isFindDialog && reverseFind);
}
}

Now it works well.

[jacky yang]

When You set find.use.strip=1 or use default option, You must add more code in scite/gtk/SciTEGTK.cxx :

Add a member variable and function in class FindStrip.

class FindStrip : public FindReplaceStrip {
public:
WStatic wStaticFind;
WButton wButton;
WButton wButtonMarkAll;
enum { checks = 6 };
WCheckDraw wCheck[checks];
bool * m_pCloseFind;

FindStrip() : m_pCloseFind(NULL) {
}
void SetCloseFind( bool * pCloseFind ) {
m_pCloseFind = pCloseFind;
}
virtual void Creation(GtkWidget *boxMain);

...
}

Modify FindStrip::FindNextCmd() and FindStrip::MarkAllCmd() :

void FindStrip::FindNextCmd() {
GrabFields();
if (pSearcher->FindHasText()) {
pSearcher->FindNext(pSearcher->reverseFind);
}
if( (NULL != m_pCloseFind) && (false == (*m_pCloseFind)) ) {
return;
}
Close();
}

void FindStrip::MarkAllCmd() {
GrabFields();
pSearcher->MarkAll();
pSearcher->FindNext(pSearcher->reverseFind);
if( (NULL != m_pCloseFind) && (false == (*m_pCloseFind)) ) {
return;
}
Close();
}

Initialize FindStrip::m_pCloseFind in class SciTEGTK :

SciTEGTK::SciTEGTK(Extension *ext) : SciTEBase(ext) {

...

// Fullscreen handling
fullScreen = false;

findStrip.SetCloseFind( &closeFind );

instance = this;
}

Now it works really well.

[jacky yang]

Using "SciTEGTK::replacing" may be better than "bool isFindDialog = !dlgFindReplace.wComboReplace;" .

You could replace

void SciTEGTK::FRFindCmd() {
FindReplaceGrabFields();
bool isFindDialog = !dlgFindReplace.wComboReplace;
if (isFindDialog && closeFind)
dlgFindReplace.Destroy();
if (findWhat[0]) {
FindNext(isFindDialog && reverseFind);
}
}

with

void SciTEGTK::FRFindCmd() {
FindReplaceGrabFields();
bool isFindDialog = !replacing;
if (isFindDialog && closeFind)
dlgFindReplace.Destroy();
if (findWhat[0]) {
FindNext(isFindDialog && reverseFind);
}
}

[Neil Hodgson]

closeFind is available from Searcher which can be accessed through pSearcher. See the Windows implementation of the find strip in win32/Strips.cxx.

[jacky yang]

(SciTEGTK.findStrip.closeFind != SciTEGTK.closeFind) == true

[Neil Hodgson]

I don't understand what you are saying here. There is no FindStrip::closeFind field. FindStrip may access the closeFind field on SciTEGTK through pSearcher which points back to the SciTEGTK instance. That is,
SciTEGTK::findStrip.pSearcher->closeFind is SciTEGTK::closeFind

[jacky yang]

It is crazy to remove DestroyFindReplace() from class SciTEBase,SciTEGTK and SciTEWin.

Modify these files :

scite/src/SciTEBase.h
scite/src/SciTEBase.cxx
scite/gtk/SciTEGTK.cxx
scite/win32/SciTEWin.h
scite/win32/SciTEWinDlg.cxx

scite/src/SciTEBase.h

class SciTEBase : public ExtensionAPI, public Searcher, public WorkerListener {
...
void ReplaceOnce(bool showWarnings=true);
int DoReplaceAll(bool inSelection); // returns number of replacements or negative value if error
int ReplaceAll(bool inSelection);
int ReplaceInBuffers();
virtual void UIClosed();
virtual void UIHasFocus();
virtual void DestroyFindReplace() = 0;
virtual void GoLineDialog() = 0;
virtual bool AbbrevDialog() = 0;
virtual void TabSizeDialog() = 0;
...
};

scite/src/SciTEBase.cxx

int SciTEBase::FindNext(bool reverseDirection, bool showWarnings, bool allowRegExp) {
...
wEditor.Call(SCI_SCROLLRANGE, start, end);
wEditor.Call(SCI_SETTARGETRANGE, start, end);
SetSelection(start, end);
if (!replacing && closeFind) {
DestroyFindReplace();
}
}
return posFind;
}

scite/gtk/SciTEGTK.cxx

class SciTEGTK : public SciTEBase {
...
virtual void FindIncrement();
void FindInFilesResponse(int responseID);
virtual void FindInFiles();
virtual void Replace();
void FindReplaceResponse(int responseID);
virtual void FindReplace(bool replace);
virtual void DestroyFindReplace();
virtual void GoLineDialog();
virtual bool AbbrevDialog();
virtual void TabSizeDialog();
virtual bool ParametersDialog(bool modal);
...
};

scite/gtk/SciTEGTK.cxx

void SciTEGTK::DestroyFindReplace() {
dlgFindReplace.Destroy();
}

scite/win32/SciTEWin.h

class SciTEWin : public SciTEBase {
...
virtual void FindIncrement();
virtual void Find();
virtual void FindInFiles();
virtual void Replace();
virtual void FindReplace(bool replace);
virtual void DestroyFindReplace();

BOOL GoLineMessage(HWND hDlg, UINT message, WPARAM wParam);
static BOOL CALLBACK GoLineDlg(HWND hDlg, UINT message, WPARAM wParam, LPARAM lParam);
virtual void GoLineDialog();
...
};

and

scite/win32/SciTEWinDlg.cxx

void SciTEWin::DestroyFindReplace() {
if (wFindReplace.Created()) {
::EndDialog(reinterpret_cast<HWND>(wFindReplace.GetID()), IDCANCEL);
wFindReplace.Destroy();
}
}

Everything is OK.

[Neil Hodgson]

I do not think everything is OK as that does not appear to close the dialog or strip in all the cases where it currently does and should continue to do so. A patch targetted at just the minimum set of points and just for GTK+ is better than modifying code that is shared between platforms.

[Neil Hodgson]

The minimum patch appears to be

diff -r e5de17970142 gtk/SciTEGTK.cxx
--- a/gtk/SciTEGTK.cxx  Tue Sep 15 08:55:28 2015 +1000
+++ b/gtk/SciTEGTK.cxx  Wed Sep 16 09:31:18 2015 +1000
@@ -2029,7 +2029,7 @@
 void SciTEGTK::FRFindCmd() {
    FindReplaceGrabFields();
    bool isFindDialog = !dlgFindReplace.wComboReplace;
-   if (isFindDialog)
+   if (isFindDialog && closeFind)
        dlgFindReplace.Destroy();
    if (findWhat[0]) {
        FindNext(isFindDialog && reverseFind);
@@ -4210,7 +4210,8 @@
    if (pSearcher->FindHasText()) {
        pSearcher->FindNext(pSearcher->reverseFind);
    }
-   Close();
+   if (pSearcher->closeFind)
+       Close();
 }

 void FindStrip::MarkAllCmd() {

[jacky yang]

You set find.use.strip=1 or replace.use.strip=1. What will happen when you click menu Find or Replace after you have already open FindIncrementPanel?
The findStrip or replaceStrip and FindIncrementPanel will display at the bottom of window at the same time.

You can change it. Make it mutually exclusive between in FindIncrementPanel, findStrip, replaceStrip. Make it mutually exclusive between in dlgFindReplace, findStrip, replaceStrip. You can switch between dlgFindReplace, findStrip, replaceStrip and FindIncrementPanel whenever you want it.

scite/gtk/SciTEGTK.cxx

Replace the old SciTEGTK::Find():

void SciTEGTK::Find() {
    if (dlgFindReplace.Created()) {
        dlgFindReplace.Present();
        return;
    }
    SelectionIntoFind();
    if (props.GetInt("find.use.strip")) {
        replaceStrip.Close();
        findStrip.SetIncrementalBehaviour(props.GetInt("find.strip.incremental"));
        findStrip.Show(props.GetInt("strip.button.height", -1));
    } else {
        if (findStrip.visible || replaceStrip.visible)
            return;
        FindReplace(false);
    }
}

With the new one:

void SciTEGTK::Find() {
    if (dlgFindReplace.Created()) {
        if( !replacing ) {
            dlgFindReplace.Present();
            return;
        }
        dlgFindReplace.Destroy();
    }
    gtk_widget_hide(wIncrementPanel);
    if (replaceStrip.visible)
        replaceStrip.Close();
    SelectionIntoFind();
    if (props.GetInt("find.use.strip")) {
        findStrip.SetIncrementalBehaviour(props.GetInt("find.strip.incremental"));
        findStrip.Show(props.GetInt("strip.button.height", -1));
    } else {
        if (findStrip.visible)
            findStrip.Close();
        FindReplace(false);
    }
}

Replace the old SciTEGTK::Replace():

void SciTEGTK::Replace() {
    if (dlgFindReplace.Created()) {
        dlgFindReplace.Present();
        return;
    }
    SelectionIntoFind();
    if (props.GetInt("replace.use.strip")) {
        findStrip.Close();
        replaceStrip.SetIncrementalBehaviour(props.GetInt("replace.strip.incremental"));
        replaceStrip.Show(props.GetInt("strip.button.height", -1));
    } else {
        if (findStrip.visible || replaceStrip.visible)
            return;
        FindReplace(true);
    }
}

With the new one:

void SciTEGTK::Replace() {
    if (dlgFindReplace.Created()) {
        if( replacing ) {
            dlgFindReplace.Present();
            return;
        }
        dlgFindReplace.Destroy();
    }
    gtk_widget_hide(wIncrementPanel);
    if (findStrip.visible)
        findStrip.Close();
    SelectionIntoFind();
    if (props.GetInt("replace.use.strip")) {
        replaceStrip.SetIncrementalBehaviour(props.GetInt("replace.strip.incremental"));
        replaceStrip.Show(props.GetInt("strip.button.height", -1));
    } else {
        if (replaceStrip.visible)
            replaceStrip.Close();
        FindReplace(true);
    }
}

Replace the old SciTEGTK::FindIncrement():

void SciTEGTK::FindIncrement() {
    findStrip.Close();
    replaceStrip.Close();
    FindIncrementSetColour(true);
    failedfind = false;
    gtk_widget_show(wIncrementPanel);
    gtk_widget_grab_focus(GTK_WIDGET(IncSearchEntry));
    gtk_entry_set_text(GTK_ENTRY(IncSearchEntry), "");
}

With the new one:

void SciTEGTK::FindIncrement() {
    if (findStrip.visible)
        findStrip.Close();
    if (replaceStrip.visible)
        replaceStrip.Close();
    gboolean isIncrementPanelVisible = gtk_widget_get_visible(wIncrementPanel);
    if ( FALSE ==  isIncrementPanelVisible) {
        FindIncrementSetColour(true);
        failedfind = false;
        gtk_widget_show(wIncrementPanel);
        gtk_widget_grab_focus(GTK_WIDGET(IncSearchEntry));
        gtk_entry_set_text(GTK_ENTRY(IncSearchEntry), "");
    }
}

You can press Esc to close dlgFindReplace, findStrip, replaceStrip or FindIncrementPanel whenever you do it.

All the code is tested to run on fedora.