Bug when using the 'Free-Spacing' regex mode within the 'Find in Files' dialog !

guy038 commented 1 year ago

Hi, Dear Developers,

Not a critical bug, but ...

Open a new tab and type in the following text :

  aaa
  abc
  cde
  def
  zzz

Save the modifications using the name Free_Spacing_Bug.txt
Open the Find in Files dialog ( Ctrl + Shift + F )
Type in (?x-i) ^ \x20 \x20 abc | ^ \x20 \x20 def in the Find what : zone
Type in $0 in the Replace with : zone ( NOT important )
Type in Free_Spacing_Bug.txt in the Filters : zone
Type in the name of the folder, containing the Free_Spacing_Bug.txt file, in the Directory : zone
Tick the Wrap around option
Select the Regular expression search mode

Expected behaviour

As soon as you click on the Find All button, you get, as expected, the Search results windows saying :

Search "(?x-i) ^ \x20 \x20 abc | ^ \x20 \x20 def" (2 hits in 1 file of 1 searched), which are  '  abc' and  '  def'

Now, repeat the same operations but select the four-lines text, below, as the input of the Find what: zone :

(?x-i)
^ \x20 \x20 abc
|
^ \x20 \x20 def

Note that this four-lines regex is strictly equivalent to the single line regex (?x-i) ^ \x20 \x20 abc | ^ \x20 \x20 def

Present behaviour

This time, after a click on the Find in Files button, we get :

Search "(?x-i)\r\n^ \x20 \x20 abc\r\n|\r\n^ \x20 \x20 def" (0 hits in 0 files of 1 searched)

NO hits occurred. In addition, the search mode have wrongly moved to the Extended (\n, \r, \t, \0, \x...) mode

Expected behaviour :

We should get the same result as above

Important :

This wrong behaviour also occurs if you select the Find in Projects dialog
Luckily, I verified that the multi-lines regex, in free-spacing mode, works correctly in the Find, Replace and Mark dialogs !
This wrong behaviour, when using the Find in Files dialog, exists from a very long time ( at least since the 6.8.1 version and probably since the adoption of the new PRCE regex engine, in Notepad++ v6.0, which allows the free-spacing regex mode )
If you use a one-line regex, in free-spacing mode , everything is OK in ALL cases

Best Regards,

guy038

mpheath commented 1 year ago

Luckily, I verified that the multi-lines regex, in free-spacing mode, works correctly in the Find, Replace and Mark dialogs !

https://github.com/notepad-plus-plus/notepad-plus-plus/blob/11f7ba2e0a80dae890c5162a92635d6c6151af64/PowerEditor/src/ScintillaComponent/FindReplaceDlg.cpp#L4165

sets extended mode and affects

Find:

case IDC_FINDALL_OPENEDFILES Button: "Find All in All Opened Documents"
case IDC_FINDALL_CURRENTFILE Button: "Find All in Current Document"

Find In Files & (as confirmed by guy038) Find In Projects:

case IDD_FINDINFILES_FIND_BUTTON Button: "Find All"

So it is at least four buttons that updates the combo for the search which can transform to literal escapes \r and \n and switches to extended mode.

Related: https://github.com/notepad-plus-plus/notepad-plus-plus/commit/e94b452d2aabcecd6ec0d119b08a27cbbb854b82 committed Jun 10, 2009 which apparently fixes a crash by adding the function named FindReplaceDlg::combo2ExtendedMode.

alankilborn commented 1 year ago

the search mode have wrongly moved to the Extended (\n, \r, \t, \0, \x...) mode

I complained about this one time and was told that it was expected behavior and by-design. See the rejected #11847. Rereading that now, I see that it was deemed too risky to change for too little gain.

alankilborn commented 1 year ago

Here's a good one (bug(s)):

Put Guy's text from above into a tab and select it all:

Press Ctrl+f (so that selected text get auto-populated into Find what when Find window appears):

Press Find All in Current Document; see:

There certainly should be 1 hit! Well but maybe not, because the Search results window shows what really was searched for.

Now look at the Find window; it has changed (to match Search results window):

As a user, why do I want it to work this way??

alankilborn commented 1 year ago

IMO, this "jump to extended mode" thing should probably be dealt with BEFORE anyone looks at Guy's initial complaint in this issue.

guy038 commented 1 year ago

Hello, @mpheath,

I re-read your answer carefully and, indeed, you're perfectly right about this bug.

I didn't notice, when posting this issue, that this wrong behaviour also affects two buttons of the Find dialog !

So, my intial post was not complete :-((

Would it be easy enough to fix this bug and allow the multi-lines free spacing regex mode for all the search dialogs ?

Cheers,

guy038

mpheath commented 1 year ago

@guy038

I re-read your answer carefully and, indeed, you're perfectly right about this bug.

I know about the symptoms of the fix. The actual bug that caused the fix to be committed in 2009 not so much that I could find.

I searched SourceForge patches and discussions to find an issue with details of the find in files crash. Closest was notepad++ crashing on find in files though no information to reproduce. Or this NP++ Crashes when doing a "find in files" with no information to reproduce.

Quote from ticket:

i've been using it for a while with no problem. however, starting a few days ago, it crashes every time i try to use this feature.

Seems the bug did not exist before the time of the ticket.

I didn't notice, when posting this issue, that this wrong behaviour also affects two buttons of the Find dialog !

So, my intial post was not complete :-((

The source code shows what the fix affects. If you cannot read the source code and understand it then I guess you tried your best to investigate. I went a little further by viewing the blame part of the code which led me to the 2009 fix.

Would it be easy enough to fix this bug and allow the multi-lines free spacing regex mode for all the search dialogs ?

Revert the fix done in 2009 for the Find Replace Dialog.

I reverted the fix locally by commenting the code and compiled with GCC on 2022-12-25.

No crashing while testing on Windows 7. If actual carriage return or line feed characters cause a crash, I do not see it happening.

Much has changed since June of 2009. The Boost library may have been the cause and many fixes have been done with Boost since 2009. Trying to find an issue report at the Boost tracker that fixes this is like trying to find a needle in a haystack. Searching for crash or newline finds many tickets.

The fix may have served a purpose in the time period of 2009 to contain the crash, though it may not be needed anymore.

Perhaps the maintainer could do a temporary revert of the fix which could be trialed for a period of time and reapply the fix if needed.

Compiled Notepad++ from source with the fix reverted. All tests are multiple line selection which are automatically added to the combo.

Test Results

#### RegexPattern ``` (?x-i) ^ \x20 \x20 abc | ^ \x20 \x20 def ``` #### Free_Spacing_Bug.txt ``` aaa abc cde def zzz ``` ## Normal Using Free_Spacing_Bug.txt text #### Find > Find All in Current Document ``` Search " aaa abc cde def zzz" (1 hit in 1 file of 1 searched) D:\Reports\Notepad++\notepad-plus-plus\PowerEditor\gcc\bin.x86_64\test\Free_Spacing_Bug.txt (1 hit) Line 1: aaa ``` #### Find > Find All in All Opened Documents ``` Search " aaa abc cde def zzz" (1 hit in 1 file of 2 searched) D:\Reports\Notepad++\notepad-plus-plus\PowerEditor\gcc\bin.x86_64\test\Free_Spacing_Bug.txt (1 hit) Line 1: aaa ``` #### Find in Files > Find All ``` Search " aaa abc cde def zzz" (1 hit in 1 file of 1 searched) D:\Reports\Notepad++\notepad-plus-plus\PowerEditor\gcc\bin.x86_64\test\Free_Spacing_Bug.txt (1 hit) Line 1: aaa ``` ## Extended Using Free_Spacing_Bug.txt text #### Find > Find All in Current Document ``` Search " aaa abc cde def zzz" (1 hit in 1 file of 1 searched) D:\Reports\Notepad++\notepad-plus-plus\PowerEditor\gcc\bin.x86_64\test\Free_Spacing_Bug.txt (1 hit) Line 1: aaa ``` #### Find > Find All in All Opened Documents ``` Search " aaa abc cde def zzz" (1 hit in 1 file of 2 searched) D:\Reports\Notepad++\notepad-plus-plus\PowerEditor\gcc\bin.x86_64\test\Free_Spacing_Bug.txt (1 hit) Line 1: aaa ``` #### Find in Files > Find All ``` Search " aaa abc cde def zzz" (1 hit in 1 file of 1 searched) D:\Reports\Notepad++\notepad-plus-plus\PowerEditor\gcc\bin.x86_64\test\Free_Spacing_Bug.txt (1 hit) Line 1: aaa ``` ## Regular Expression Using RegexPattern text #### Find > Find All in Current Document ``` Search "(?x-i) ^ \x20 \x20 abc | ^ \x20 \x20 def" (2 hits in 1 file of 1 searched) D:\Reports\Notepad++\notepad-plus-plus\PowerEditor\gcc\bin.x86_64\test\Free_Spacing_Bug.txt (2 hits) Line 2: abc Line 4: def ``` #### Find > Find All in All Opened Documents ``` Search "(?x-i) ^ \x20 \x20 abc | ^ \x20 \x20 def" (2 hits in 1 file of 2 searched) D:\Reports\Notepad++\notepad-plus-plus\PowerEditor\gcc\bin.x86_64\test\Free_Spacing_Bug.txt (2 hits) Line 2: abc Line 4: def ``` #### Find in Files > Find All ``` Search "(?x-i) ^ \x20 \x20 abc | ^ \x20 \x20 def" (2 hits in 1 file of 1 searched) D:\Reports\Notepad++\notepad-plus-plus\PowerEditor\gcc\bin.x86_64\test\Free_Spacing_Bug.txt (2 hits) Line 2: abc Line 4: def ```

Debug Info...

``` Notepad++ v8.4.8 (64-bit) Build time : Dec 25 2022 - 15:35:20 Built with : GCC 11.2.0 Path : D:\Reports\Notepad++\notepad-plus-plus\PowerEditor\gcc\bin.x86_64\notepad++.exe Command Line : Admin mode : OFF Local Conf mode : ON Cloud Config : OFF OS Name : Windows 7 Professional (64-bit) OS Build : 7601.0 Current ANSI codepage : 1252 Plugins : none ```

alankilborn commented 1 year ago

@mpheath Would your proposed reversion of the "fix" include preventing the automatic switch to Extended Search mode? I think so, but thought I'd ask.

mpheath commented 1 year ago

@alankilborn

This code excerpt does the automatic switch to Extended search mode

https://github.com/notepad-plus-plus/notepad-plus-plus/blob/13dfaa96a8b224b000d470184ecae7a05057be45/PowerEditor/src/ScintillaComponent/FindReplaceDlg.cpp#L4205-L4208

which is within the combo2ExtendedMode function of the fix. Revert the fix will remove the transformation of actual carriage return to \r, actual line feed to \n and remove the automatic switch to Extended search mode.

notepad-plus-plus / notepad-plus-plus