From 4003d6d923e9136ff726f35c954c68b8c18bf681 Mon Sep 17 00:00:00 2001 From: Scott Sumner <30118311+sasumner@users.noreply.github.com> Date: Thu, 14 May 2020 20:12:34 -0400 Subject: [PATCH] Disallow backward regex searches due to sometimes surprising results The full story, debated before, is that regular expression searching in a backward direction from the caret position causes matches that the user does not expect. The best thing that was decided to do (group decision) is to fully disable upward searching. Fix #3640, close #8269 --- PowerEditor/src/NppCommands.cpp | 40 ++++--- .../src/ScitillaComponent/FindReplaceDlg.cpp | 101 ++++++++++++------ 2 files changed, 94 insertions(+), 47 deletions(-) diff --git a/PowerEditor/src/NppCommands.cpp b/PowerEditor/src/NppCommands.cpp index 7bdc98a23..edfa41fd1 100644 --- a/PowerEditor/src/NppCommands.cpp +++ b/PowerEditor/src/NppCommands.cpp @@ -1022,23 +1022,31 @@ void Notepad_plus::command(int id) case IDM_SEARCH_FINDNEXT : case IDM_SEARCH_FINDPREV : { - if (!_findReplaceDlg.isCreated()) - return; - - FindOption op = _findReplaceDlg.getCurrentOptions(); - op._whichDirection = (id == IDM_SEARCH_FINDNEXT?DIR_DOWN:DIR_UP); - generic_string s = _findReplaceDlg.getText2search(); - FindStatus status = FSNoMessage; - _findReplaceDlg.processFindNext(s.c_str(), &op, &status); - if (status == FSEndReached) + if (_findReplaceDlg.isCreated()) { - generic_string msg = _nativeLangSpeaker.getLocalizedStrFromID("find-status-end-reached", TEXT("Find: Found the 1st occurrence from the top. The end of the document has been reached.")); - _findReplaceDlg.setStatusbarMessage(msg, FSEndReached); - } - else if (status == FSTopReached) - { - generic_string msg = _nativeLangSpeaker.getLocalizedStrFromID("find-status-top-reached", TEXT("Find: Found the 1st occurrence from the bottom. The beginning of the document has been reached.")); - _findReplaceDlg.setStatusbarMessage(msg, FSTopReached); + FindOption op = _findReplaceDlg.getCurrentOptions(); + if ((id == IDM_SEARCH_FINDPREV) && (op._searchType == FindRegex)) + { + // regex upward search is disabled + // make this a no-action command + } + else + { + op._whichDirection = (id == IDM_SEARCH_FINDNEXT ? DIR_DOWN : DIR_UP); + generic_string s = _findReplaceDlg.getText2search(); + FindStatus status = FSNoMessage; + _findReplaceDlg.processFindNext(s.c_str(), &op, &status); + if (status == FSEndReached) + { + generic_string msg = _nativeLangSpeaker.getLocalizedStrFromID("find-status-end-reached", TEXT("Find: Found the 1st occurrence from the top. The end of the document has been reached.")); + _findReplaceDlg.setStatusbarMessage(msg, FSEndReached); + } + else if (status == FSTopReached) + { + generic_string msg = _nativeLangSpeaker.getLocalizedStrFromID("find-status-top-reached", TEXT("Find: Found the 1st occurrence from the bottom. The beginning of the document has been reached.")); + _findReplaceDlg.setStatusbarMessage(msg, FSTopReached); + } + } } } break; diff --git a/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp b/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp index 96b3d1867..7f1dbd84a 100644 --- a/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp +++ b/PowerEditor/src/ScitillaComponent/FindReplaceDlg.cpp @@ -342,9 +342,10 @@ void FindReplaceDlg::fillFindHistory() ::SendDlgItemMessage(_hSelf, IDWHOLEWORD, BM_SETCHECK, BST_UNCHECKED, 0); ::EnableWindow(::GetDlgItem(_hSelf, IDWHOLEWORD), (BOOL)false); - //regex upward search is disable in v6.3 due to a regression - ::EnableWindow(::GetDlgItem(_hSelf, IDC_FINDPREV), (BOOL)false); + // regex upward search is disabled + ::SendDlgItemMessage(_hSelf, IDC_BACKWARDDIRECTION, BM_SETCHECK, BST_UNCHECKED, 0); ::EnableWindow(::GetDlgItem(_hSelf, IDC_BACKWARDDIRECTION), (BOOL)false); + ::EnableWindow(::GetDlgItem(_hSelf, IDC_FINDPREV), (BOOL)false); // If the search mode from history is regExp then enable the checkbox (. matches newline) ::EnableWindow(GetDlgItem(_hSelf, IDREDOTMATCHNL), true); @@ -1062,26 +1063,36 @@ INT_PTR CALLBACK FindReplaceDlg::run_dlgProc(UINT message, WPARAM wParam, LPARAM } } - if (isMacroRecording) - saveInMacro(IDOK, FR_OP_FIND); + if ((_options._whichDirection == DIR_UP) && (_options._searchType == FindRegex)) + { + // this can only happen when shift-key was pressed + // regex upward search is disabled + // turn user action into a no-action step + } + else + { + if (isMacroRecording) + saveInMacro(IDOK, FR_OP_FIND); + + FindStatus findStatus = FSFound; + processFindNext(_options._str2Search.c_str(), _env, &findStatus); + + NativeLangSpeaker *pNativeSpeaker = (NppParameters::getInstance()).getNativeLangSpeaker(); + if (findStatus == FSEndReached) + { + generic_string msg = pNativeSpeaker->getLocalizedStrFromID("find-status-end-reached", TEXT("Find: Found the 1st occurrence from the top. The end of the document has been reached.")); + setStatusbarMessage(msg, FSEndReached); + } + else if (findStatus == FSTopReached) + { + generic_string msg = pNativeSpeaker->getLocalizedStrFromID("find-status-top-reached", TEXT("Find: Found the 1st occurrence from the bottom. The beginning of the document has been reached.")); + setStatusbarMessage(msg, FSTopReached); + } + } - FindStatus findStatus = FSFound; - processFindNext(_options._str2Search.c_str(), _env, &findStatus); // restore search direction which may have been overwritten because shift-key was pressed _options._whichDirection = direction_bak; - NativeLangSpeaker *pNativeSpeaker = (NppParameters::getInstance()).getNativeLangSpeaker(); - if (findStatus == FSEndReached) - { - generic_string msg = pNativeSpeaker->getLocalizedStrFromID("find-status-end-reached", TEXT("Find: Found the 1st occurrence from the top. The end of the document has been reached.")); - setStatusbarMessage(msg, FSEndReached); - } - else if (findStatus == FSTopReached) - { - generic_string msg = pNativeSpeaker->getLocalizedStrFromID("find-status-top-reached", TEXT("Find: Found the 1st occurrence from the bottom. The beginning of the document has been reached.")); - setStatusbarMessage(msg, FSTopReached); - } - nppParamInst._isFindReplacing = false; } return TRUE; @@ -1409,16 +1420,17 @@ INT_PTR CALLBACK FindReplaceDlg::run_dlgProc(UINT message, WPARAM wParam, LPARAM _options._isWholeWord = false; ::SendDlgItemMessage(_hSelf, IDWHOLEWORD, BM_SETCHECK, _options._isWholeWord?BST_CHECKED:BST_UNCHECKED, 0); - //regex upward search is disable in v6.3 due to a regression + //regex upward search is disabled ::SendDlgItemMessage(_hSelf, IDC_BACKWARDDIRECTION, BM_SETCHECK, BST_UNCHECKED, 0); _options._whichDirection = DIR_DOWN; } ::EnableWindow(::GetDlgItem(_hSelf, IDWHOLEWORD), (BOOL)!isRegex); - //regex upward search is disable in v6.3 due to a regression - ::EnableWindow(::GetDlgItem(_hSelf, IDC_FINDPREV), (BOOL)!isRegex); + // regex upward search is disabled ::EnableWindow(::GetDlgItem(_hSelf, IDC_BACKWARDDIRECTION), (BOOL)!isRegex); + ::EnableWindow(::GetDlgItem(_hSelf, IDC_FINDPREV), (BOOL)!isRegex); + return TRUE; } case IDWRAP : @@ -2675,9 +2687,18 @@ void FindReplaceDlg::execSavedCommand(int cmd, uptr_t intValue, const generic_st switch (intValue) { case IDOK: - nppParamInst._isFindReplacing = true; - processFindNext(_env->_str2Search.c_str()); - nppParamInst._isFindReplacing = false; + if ((_env->_whichDirection == DIR_UP) && (_env->_searchType == FindRegex)) + { + // regex upward search is disabled + // this macro step could have been recorded in an earlier version before it was not allowed, or hand-edited + // make this a no-action macro step + } + else + { + nppParamInst._isFindReplacing = true; + processFindNext(_env->_str2Search.c_str()); + nppParamInst._isFindReplacing = false; + } break; case IDC_FINDNEXT: @@ -2692,16 +2713,34 @@ void FindReplaceDlg::execSavedCommand(int cmd, uptr_t intValue, const generic_st case IDC_FINDPREV: // IDC_FINDPREV will not be recorded into new macros recorded with 7.8.5 and later // stay playback compatible with 7.5.5 - 7.8.4 where IDC_FINDPREV was allowed but unneeded/undocumented - nppParamInst._isFindReplacing = true; - _env->_whichDirection = DIR_UP; - processFindNext(_env->_str2Search.c_str()); - nppParamInst._isFindReplacing = false; + if (_env->_searchType == FindRegex) + { + // regex upward search is disabled + // this macro step could have been recorded in an earlier version before it was not allowed, or hand-edited + // make this a no-action macro step + } + else + { + _env->_whichDirection = DIR_UP; + nppParamInst._isFindReplacing = true; + processFindNext(_env->_str2Search.c_str()); + nppParamInst._isFindReplacing = false; + } break; case IDREPLACE: - nppParamInst._isFindReplacing = true; - processReplace(_env->_str2Search.c_str(), _env->_str4Replace.c_str(), _env); - nppParamInst._isFindReplacing = false; + if ((_env->_whichDirection == DIR_UP) && (_env->_searchType == FindRegex)) + { + // regex upward search is disabled + // this macro step could have been recorded in an earlier version before it was disabled, or hand-edited + // make this a no-action macro step + } + else + { + nppParamInst._isFindReplacing = true; + processReplace(_env->_str2Search.c_str(), _env->_str4Replace.c_str(), _env); + nppParamInst._isFindReplacing = false; + } break; case IDC_FINDALL_OPENEDFILES: nppParamInst._isFindReplacing = true;