Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

[PM] Lots of Find in Files improvements #3151

Closed
wants to merge 1 commit into from
Closed

[PM] Lots of Find in Files improvements #3151

wants to merge 1 commit into from

Conversation

TomMalbran
Copy link
Contributor

So here I made the next improvements to find in files covering most of the Trello Card and some extras:

  • Both the results and the dialog now use a proper template. The dialog template was used just for the content and only in Find in Files, but it could be used for all the find templates with few modifications. (Implemented in [OPEN] Find in Files Improvements (Part 1: Templates) #3324)
  • Clicking on a table row, now selects it, making it easier to go through all the results. The only problem is that the highlight color and the selected color are the same, so if needed one would need to be changed. (Implemented in [OPEN] Find in Files Improvements (Part 1: Templates) #3324)
  • The results list now allows paging with just 2 links. I left the 100 results per page and increased to 300 the results per file, since we can now see all of them. It could still be increased, but haven't checked what would be right max. (Implemented in Find in Files Improvements (Part 2: Pagination) #4303)
  • Double clicking on a result now adds it to the working set. (Implemented in Find in Files Improvements (Part 2: Pagination) #4303)
  • The file results are now sorted having the current document always first to make it possible to select the first result in the current document, if there is one. (Implemented in Find in Files Improvements (Part 4: Sorting) #6585)
  • Searching through the files now uses DocumentManager for files in the working set, since only those could have unsaved changes and FileUtils for the rest. These should avoid creating a Document for every file creating lots of console errors as mentioned in Find in Files causes console to get filled up with messages from LanguageManager #3146. (Not required anymore)
  • All the find methods can now search through documents outside of the project in the working set.
  • Every change done to the current document modifies the results: add or delete results and change the line numbers when needed. (Implemented in Find in Files Improvements (Part 3: Auto-update) #4729)
  • Added a new command to search only the files in the working set, and changed the scope functions to work with multiple files, so if we add a multiple file selection, a new search method could be easily added.
  • Added 2 new commands to select the previous/next row in the search result table. Wasn't sure of the right command for Mac, so I made one up following the key order used in the find/replace commands.
  • Moved all the search menu items with the 3 new commands and the Find in File/Subtree command to a new Search menu. This reduced the size of the Edit menu and allowed me to add new menu items.

I also experimented on moving the search to a Web Worker, since Brackets stalls for a long time when searching on big projects, like Brackets, but it couldn't access brackets.fs probably for having the same restrictions as working in a browser (could this be changed in the CEF configuration?). I tested a bit just moving the search, but wasn't able to do it yet using Transferable Objects (without those, brackets crashes and wouldn't make a good optimization). Anyway moving just the search would still stall the window during the file reading which is already a long time. A filtering system would reduce this a lot in big project without needing to use Web Workers.

@TomMalbran
Copy link
Contributor Author

Its been a while with no comments here, but I know that this is a big request. Should I just start splitting this requests into smaller parts so it can easily be reviewed and merged. I could go step by step, make a small request, wait until is merged, start with another part until everything is covered? Might make it a lot easier since most of the changes aren't that big anyway, but look big all together.

@njx
Copy link
Contributor

njx commented Apr 16, 2013

Hey @TomMalbran -- sorry (again) for letting this languish so long. Tagging [PM]--to @adrocknaphobia to take a look at this and evaluate what's currently in the pull request.

@TomMalbran
Copy link
Contributor Author

@njx That is ok, I know is a big requests with several things so it would take its time to review it. The bullet list at the start should have all the new features. I started splitting up this request, to make it easier to review and merge, is that ok too?

@njx
Copy link
Contributor

njx commented Apr 16, 2013

Let's wait until Adam's had a chance to look at it before doing more work to split it up--he might have opinions about which pieces he does/doesn't want right now. Thanks!

@njx
Copy link
Contributor

njx commented May 10, 2013

Hmmm, somehow the [PM] tag got lost. To @adrocknaphobia again.

@redmunds
Copy link
Contributor

Assigning [PM] issue to @jadbox

@TomMalbran
Copy link
Contributor Author

@redmunds I will just close this. Most of it was done in separate PRs, and the rest will be done in the same way.

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

Successfully merging this pull request may close these issues.

5 participants