Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quick find functionality #2383

Conversation

MojaveWastelander
Copy link

@MojaveWastelander MojaveWastelander commented Oct 3, 2016

Added 'quick find' functionality to search dialog.
Enabled by default, not saved in settings.
When search dialog is opened writing something in search box will search and mark all matches in current document.
Toggling any control on search tab will reset the search, as well as changing search combo box selection.
When search dialog is closed all marks are removed.

@cmeriaux
Copy link
Contributor

cmeriaux commented Oct 3, 2016

Hello, I tested with the nightlibuid
1553095 is OK
966ef61 is KO. Bookmark are always affected.
Like I said before, you should also add "quick search" in the replace panel.

@MojaveWastelander
Copy link
Author

MojaveWastelander commented Oct 3, 2016

Hello, what do you mean by "bookmarks are always affected"? I used a test file and set some bookmarks, then I made a few searches and closed the window, all bookmarks remained unchanged and marks were removed. Regarding replace panel is need to remove the check in code. There is also a bug when a search is done in one document and then switching to another one does not clear marks in previous document.

@cmeriaux
Copy link
Contributor

cmeriaux commented Oct 4, 2016

Hello,
I did more tests to confirm the problem :
If "Mark/Bookmark line" is enable in the Mark panel, the Quick find function in "Find" panel always affect the existing bookmark.
If "Mark/Bookmark line" is disable in the Mark panel, bookmark are not affected however, bookmarked words always loose their mark color. For me It's a problem. I think that "quick find" should use another color than bookmark.
As a good example, "Incremental Search" (available in "search" menu) do something similar than "quick find" but use a different Scintilla Color. So it doesn't perturbate "Mark" and "bookmark" function

Scintilla allows you to define multiple style
Here is style used by Npp:

#define SCE_UNIVERSAL_FOUND_STYLE 31
#define SCE_UNIVERSAL_FOUND_STYLE_SMART 29
#define SCE_UNIVERSAL_FOUND_STYLE_INC 28
#define SCE_UNIVERSAL_TAGMATCH 27
#define SCE_UNIVERSAL_TAGATTR 26
#define SCE_UNIVERSAL_FOUND_STYLE_EXT1 25
#define SCE_UNIVERSAL_FOUND_STYLE_EXT2 24
#define SCE_UNIVERSAL_FOUND_STYLE_EXT3 23
#define SCE_UNIVERSAL_FOUND_STYLE_EXT4 22
#define SCE_UNIVERSAL_FOUND_STYLE_EXT5 21

You may create a new one, or use the icnremental style (SCE_UNIVERSAL_FOUND_STYLE_INC)

http://www.scintilla.org/ScintillaDoc.html#SCI_INDICSETSTYLE

Another problem, "Quick find" follows the direction configured. I think it should process the complete document.

Added member function that clears marks by style.
Clear marks when switching between search tabs.
@MojaveWastelander
Copy link
Author

MojaveWastelander commented Oct 4, 2016

The problem was with the ProcessOperation I was using, ProcessMarkAll was using _doMarkLine setting, I switched to ProcessMarkAll_IncSearch which just marks specific ranges and uses SCE_UNIVERSAL_FOUND_STYLE_INC. Regarding direction it actually is searching whole document by setting SCI_SETCURRENTPOS to 0.

@@ -2278,6 +2278,7 @@ void FindReplaceDlg::enableReplaceFunc(bool isEnable)
::ShowWindow(::GetDlgItem(_hSelf, IDC_FINDALL_OPENEDFILES), !hideOrShow);
::ShowWindow(::GetDlgItem(_hSelf, IDCCOUNTALL),!hideOrShow);
::ShowWindow(::GetDlgItem(_hSelf, IDC_FINDALL_CURRENTFILE),!hideOrShow);
::ShowWindow(::GetDlgItem(_hSelf, ID_QUICK_FIND), SW_SHOW);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the SW_SHOW deliberate or should it be !hideOrShow as is in the three lines above it?
When deliberate add a explanatory comment to prevent anyone to interpret as a copy/paste error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it should not depend on isEnable argument as it should be visible on search and replace tabs, I'll add a comment.

@cmeriaux
Copy link
Contributor

cmeriaux commented Oct 5, 2016

@MojaveWastelander What you've done is very nice. Thanks a lot.
What about adding in the mark panel the similar function "quick mark" ? Is it possible ?
Once again, thanks.

@cmeriaux
Copy link
Contributor

cmeriaux commented Oct 5, 2016

@MojaveWastelander here is another problem.
When "quick find" is enable, the auto-fill of search field with the selected word is broken. Do you confirm ?

@cmeriaux
Copy link
Contributor

cmeriaux commented Oct 5, 2016

@MojaveWastelander here is another problem concerning Smart Highlighting.
Start from a fresh Notepad++
Select a word.
Check that SmartHighlighting works : it highlights all similar words
Open find panel
Do a quick find
close find panel
Try again the Smart Highlighting. It should'nt works anymore until Npp restart.

Do you confirm ?

@MojaveWastelander
Copy link
Author

Yes and yes, I'll look into it. Thanks for feedback.

Fix: words under caret not taken as search values when search window is opened.
@MojaveWastelander
Copy link
Author

Seems to be fixed now, took longer to find what was causing the second issue than to fix it. The question now is when to save or to save in general quick search queries? Right now when quick search is enabled nothing that is written in the search box is saved.

@dail8859
Copy link
Contributor

dail8859 commented Oct 6, 2016

Still has a problem with regular expressions locking it up.

Also using escaped newline characters with regular expressions is broken, even with "Quick find" turned off:

  1. Open new file
  2. Press enter 3 times
  3. Open find dialog
  4. Set mode to Regular expression
  5. "Quick find" doesn't matter if it is checked or not
  6. Search for \r\n (assuming windows line endings)
  7. No matches

Doing the above steps with the v7 release counts 3 matches.

Also, even on what I would consider "small" files (~500kB) having lots of matches can cause N++ to have noticeable lag. If possible I'd suggest once the first match is found don't search any further than you have to (e.g. only the text visible on the screen). However, this would probably also mean getting the Scintilla notifications if the view is scrolled to research the text on the screen.

Enabled by default, not saved in settings.

Forcing someone to use this every time they open N++ is not a good idea. Personally I open large files quite often and search through them. I wouldn't want to to have to uncheck this every time.

@MojaveWastelander
Copy link
Author

Regarding regex search I'm using search methods provided by n++, so "quick find" search is the same as default search. On version that I develop on there doesn't seem to be issues with regex search, I guess it was some issue in previous version.
From what I seen marking only matches on the current view is possible.
"enabled by default" is more of a testing state, when feature will be more polished it will be disabled by default.

…ines)

Fix: When switching documents marks set by quick find in current document are removed
Update: quick find check box disabled by default
…l try to find a match in whole document (only if search is requested from the dialog)
@cmeriaux
Copy link
Contributor

cmeriaux commented Oct 7, 2016

hello @MojaveWastelander
Another issue with the last version

When quick search is selected but panel is closed, the function Find volatile next is broken. "Find volatile next" finds the first occurence of the selected word.
Same issue with find volatile previous, find next and find previous

Do you confirm the issue ?

Thanks

@MojaveWastelander
Copy link
Author

Hello, I disabled the option when search dialog is "closed", so all external search methods should not be affected by it.

@cmeriaux
Copy link
Contributor

cmeriaux commented Oct 7, 2016

It's validated here. Thanks a lot
I wish you a good week-end

@cmeriaux
Copy link
Contributor

cmeriaux commented Nov 3, 2016

Hello,
what is your last commit (merge branch ....)
You should remove it with 'git rebase -i' command

@@ -37,6 +37,7 @@ EXSTYLE WS_EX_TOOLWINDOW
CAPTION "Replace"
FONT 8, "MS Shell Dlg", 0, 0, 0x0
BEGIN
AUTOCHECKBOX "&Quick find", ID_QUICK_FIND, 129, 115, 125, 8, 0, WS_EX_LEFT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

elsewhere they used other syntax

CONTROL "&Quick find",ID_QUICK_FIND,"Button",BS_AUTOCHECKBOX | WS_TABSTOP,

What about using it ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line was generated by ResEdit and I didn't looked what is the difference between using syntax with CONTROL, I guess I should use it as well for consistency.

@MojaveWastelander
Copy link
Author

MojaveWastelander commented Nov 3, 2016

Hello, I made those fixes at work computer, but I when I tried to commit them there was one commit on github that I didn't pulled before making those fixes, so I merged those commits.

…rent view.

Fix: Quick find does not wrap search when no matches found.
@donho
Copy link
Member

donho commented Nov 13, 2016

There's already a feature named incremental search which does the same thing (CTRL+ALT+I).
I prefer to not mix the both so user can choose the different GUI, and of course, no regression.

@donho donho closed this Nov 13, 2016
@cmeriaux
Copy link
Contributor

@donho the feature incremental search that already exist is useless. It's far beyond the classical search search due the missing options (search mode option, whole word only option ...)
It should me decommissioned and replaced by the one developped by @MojaveWastelander or something available in the find and replace GUI.

Moreover, an incremental mode is very useful in Regular Expression mode. It allows us to validate the regular expression interactively before doing a replace.

@MojaveWastelander
Copy link
Author

MojaveWastelander commented Nov 23, 2016

Initially the idea was to add this functionality to incremental search (it makes more sense for it to be there), but I left it on find for testing purposes (as it had all checkboxes and features available and easily accessible). One of main differences from incremental search is that search is done in current view, thus typing search query is fast no matter how big file is (it was one of issues mentioned before). Main idea is that incremental search is incomplete feature-wise and it could use some ideas that I implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants