do-me / SemanticFinder

SemanticFinder - frontend-only live semantic search with transformers.js
https://do-me.github.io/SemanticFinder/
MIT License
226 stars 16 forks source link

CodeMirror marker handling #1

Closed kungfooman closed 11 months ago

kungfooman commented 1 year ago

Right now there is code like:

https://github.com/do-me/SemanticFinder/blob/36f69d8204b777a83a7f699538037be17cd8d40d/index.html#L177-L182

Which can be simplified to:

function removeHighlights() {
    editor.getAllMarks().forEach(_ => _.clear());
}

While also removing the manual marker tracking array + marker pushing code.

Just pointing it out, but maybe you plan something else with the markers array?

do-me commented 1 year ago

Good point, this function (as many others) should definitely be simplified.

Ragrding your question

maybe you plan something else with the markers array?

Yes, I actually just did! Have a look at the version I updated yesterday and click on "scroll results". It toggles between all markers and scrolls there, so the array comes in handy there, maybe also for some kind of potential future export or similar.

However, on a second though you're absolutely right, as the markers array is almost the same as calling editor.getAllMarks().

The only advantage I see at the moment, that the markers array is already sorted and editor.getAllMarks() would require a sort by id first:

image

Feel free to open a PR if you'd like to implement/change this! For now, I rather want to focus on more features. One update coming later this day!

kungfooman commented 1 year ago

Oh, very nice, I am happy having the presorted markers array!

, maybe also for some kind of potential future export or similar.

I suggest to write code like this then:

markers.forEach(_ => _.clear());
markers.length = 0;

1) Shorter 2) Keeps the potentially exported reference 3) Not creating GC (compared to = []);

do-me commented 11 months ago

Finally taken care of. But there are many other lines to optimize... Always happy for feedback!