WIP: Trying to fix #5176 #5272
WIP: Trying to fix #5176 #5272
Conversation
|
Adding responses to #5176 (comment) here.
This is to aid in incremental searches
Something we want to keep is the highlighting of results in the "overview ruler":
Limiting these results too heavily will hurt the UX by not showing additional results. Make sure you add the overview ruler in the demo when working on this:
If I'm understanding you correctly, searching for
This is by design, incremental search means we can avoid doing much of the work, provided regex is not enabled which I think disables incremental completely.
The overview ruler is again an important piece here, see this result which is after running
There are 19 results, and every one of these is represented in the overview ruler, allowing the user to know immediately how far back the term(s) are. |
| /** | ||
| * Number of matches in each chunk | ||
| */ | ||
| private _chunkSize: number = 200; | ||
| /** | ||
| * Time in ms | ||
| * 1 ms seems to work fine as we just need to let other parts of the code to take over | ||
| * and return here when their work is done | ||
| */ | ||
| private _timeBetweenChunkOperations = 1; | ||
|
|
||
| /** | ||
| * This should be high enough so not to trigger a lot of searches | ||
| * and subsequently a lot of canceled searches which clean up their own | ||
| * decorations and cause flickers | ||
| */ | ||
| private _debounceTimeWindow = 300; | ||
| /** | ||
| * Using this mainly for resizing event | ||
| */ | ||
| private _longerDebounceTimeWindow = 1000; |
There was a problem hiding this comment.
There's actually a perfect helper for just this scenario in the main code:
This will let you continue to search until the deadline is reached, essentially scaling the solution to high end CPUs that can handle searching a lot more immediately instead of the arbitrary 200 line chunk limit. This is how we draw ascii glyphs to "warm up" the texture atlas without doing any of this optional work in a blocking manner.
For your scenario it's important work that the user is waiting on, not optional, so PriorityTaskQueue is more appropriate. It works much the same, just it's backed by setTimeout, not requestIdleCallback.
There was a problem hiding this comment.
Here is what I understood and correct me if I am wrong.
I will be en-queuing call back functions (Tasks) that process X number of lines. The task queue will run as many tasks as possible until we exceed the time limit at which point it yields, then do a new setTimeout to start processing of next batch of Tasks.
About the race condition comment. I think it arises from line 71 this._idleCallback = undefined; which in my opinion should be removed and inserted above line 95 this.start() that way new enqueue calls wont trigger new setTimeouts.
I am currently highlighting all of the matches so this will not be a problem. |
No, this is what the incremental flag is doing
Notice that the second B is selected. That's all it does. It does not help performance. I removed this behavior. |
Broke regex search
Moved constants to an Enum
AnouarTouati
force-pushed
the
search-start-at-middle-of-view
branch
from
c71ef1b to
6ecf521
Compare
I'd have to play around with it, we just want to make sure it's consistent |
and fixed multipe highlighting of regex matches
…eal with returning -1 on max highlight reached or about firing without decorations option set.
This draft is WIP attempt to fix #5176 @jerch
This is obviously a breaking change.
I tried to go with the conservative approach to keep the code changes to a minimum. But, I had to re-write most of the code.
If this is the direction you want the code to go in. I can keep working on this and maintaining the search addon from now on.
Also ignore the commits I was experimenting with stuff. Will squash them later.
2025-01-06.02-40-30.mp4