[CLOSED] Find in Files Improvements (Part 3: Auto-update) #4370
Comments
|
One question I have is whether we can/should assume that the current document is the only one that could get modified. (Due to inline editors that's definitely not true, but we could fix that by watching the active editor's document, and then my question above still applies). Certainly an extension could modify any document at any time... We could avoid that risk by adding a generic pub-sub-style message that dispatches on all edits to any document. Another question is about performance: this adds code that runs on every keystroke all the time. Would it be safer to execute async with throttling/debouncing? |
|
I wanted to search for something that had a lot of results but scattered across only a few files. So I right clicked on the "test/spec" folder and searched for "runs" (1014 matches). I went to the first occurrence (test/spec/LowLevelFileIO-test.js:92) and pressed enter at the end of line 91 which moves the result down to line 93. The find result changed to line 97. If I deleted the blank line by selecting it and pressing delete, the find result changes back to line 92. If I do it again but undo the insertion by pressing Ctrl+Z, the find result changes to line 96. I tried it again and couldn't repro. Then I hit enter a second time to insert 2 blank lines and the find result changed to line 98. It seems like every time I attempt a change I get a different result. I haven't tried Mac yet. Maybe you weren't seeing this behavior on the Mac? |
|
That event could work, maybe it could be triggered from Document.js as an export event, to keep both in the same file. If this works I'll try to implement it it. The change events that CodeMirror sends are not actually sent on every keystroke. CodeMirror already bundles really close events. I think it sends every history change event. So if several keystrokes are close enough that it can be undone with a single action, then all those changes are sent as one. We are already using this change events all over the code, including for Inline Editors and it doesn't seem to be too slow. But if it feels slow we could update the view every a few seconds (or once per screen refresh using requestAnimation), and not on every change. I think that the view update might be the part that costs the most, unless you end up updating a single line file.
|
|
It would be good to get an understanding of the impact of this change. The JavaScript itself probably isn't too bad, but it looks like the results panel is rebuilt on every change event, if there are search results in the current document. Big DOM changes like this can really affect performance. We should run with the Chrome profiler to get an accurate measurement of the impact. Here are a couple possible ways to mitigate the impact:
|
|
Ok. The content of the panel is rebuilt after something changed. It should be easy to delay this with a timer. The function could still update the results object, but just redraw the panel on idle time. I will update the code and see how that works. |
|
I updated the code to use a new |
|
Nominating for Sprint 31 |
|
We talked about this a bit more in yesterday's architecture meeting. Two notes:
|
|
|
|
|
|
Any thing else that I should do before we can merge this? I think I solved all the concerns/issues already. |
|
|
|
No problem. Just checking on this :) |
|
Seems like it works pretty well. I tried it on brackets source and it was very responsive to changes and pagination was OK. |
|
Awesome. I used several times and it works great, specially when re-factoring the code. It makes it so much easier. |
Friday Aug 09, 2013 at 22:13 GMT
Originally opened as adobe/brackets#4729
Here comes the part 3 of the Find in Files improvements PR #3151.
This PR updates the find in files results on documents changes. It listens the change event on the current document and searches over the modified lines, adding the new results or deleting the old ones. It also updates the line numbers of the rest of the results, it can remove a complete file from the results when there aren't any anymore or add a new one.
TomMalbran included the following code: https://github.com/adobe/brackets/pull/4729/commits
The text was updated successfully, but these errors were encountered: