[Highlight_Occurrences] Improve `find_visible_occurrences` accuracy

[pintassilgo]

I like the Highlight_Occurrences' option visible_fallback a lot, so much that I still believe it should be implemented in Cuda as I suggested in #5406. Currently, Cuda HiAll is limited by find_hi_max_line_len and find_hi_max_lines. I think this function should work in Cuda the same way it works in Highlight_Occurrences, as a fallback for the user to still be able to have HiAll when document is off the limits.

But as the title states, the main thing I want to suggest here is to improve its accuracy at the edges. If I place the caret on a word, all the visible instances are highlighted (I use word-wrap):

But when I scroll down a bit, matches from last visible line are not highlighted. Sometimes, even the 4th last visible line has non-highlighted matches:

Since highlighting only for visible portion is cheap, the function should expand the range a little bit to avoid such issue. This also happens with word-wrap disabled:

Even without using word-wrap, you can see from the arrows in the image below that there are some N letters at the edge of many lines that should be highlighted (because they are part of other NULL words), but are not:

[Alexey-T]

Since highlighting only for visible portion is cheap, the function should expand the range a little bit to avoid such issue.

how much to expand the range? 3 lines? 30? 50? make an option?

[Alexey-T]

better to add the option 'how many lines to highlight out of the visible area'. can you make the patch?

https://github.com/CudaText-addons/cuda_hilite_occurrences

[pintassilgo]

how much to expand the range? 3 lines? 30? 50? make an option?

At least 5 chars to the sides and 5 lines up and down. I suggest multiplying all the visible area by two at every side so that everything is already highlighted when user press PageUp/PageDown or click at the scrollbar (when scrollbar_click_jump is false, which is the default).

I'll see if it's easy for me to make the change, because Python is not a language I'm used to code.

[pintassilgo]

Line 347 of __init__.ph:

offset_lines = [(0, get_line(ed_self, i))  for i in range(_line_top, _line_bottom + 1)]

Just replacing 1 by 2 seems to fix the highlight at the bottom when I scroll down.

offset_lines = [(0, get_line(ed_self, i))  for i in range(_line_top, _line_bottom + 2)]

But I'm struggling on how to do the same to fix the highlight at the top when I scroll up.

It's also needed to extend the ranges to the sides to fix the last image from my initial comment.

[pintassilgo]

I did a few more tests and the change from my previous value should use value 15 instead of 2, even 10 is still subject to miss some matches, but with 15 it's fine.

And I was able to fix the matches at the top too. Line 338:

_line_top    = ed_self.get_prop(app.PROP_LINE_TOP)

Replace it by

_line_top    = ed_self.get_prop(app.PROP_LINE_TOP) - 15

The same thing, I tested some values and even 10 was also subject to miss some matches, but 15 is fine.

There are still two things to fix:

1. The last image from my first comment (expand the width range to match at the right edge of the window).
2. Cuda needs to update matches when window resizes. I just tested maximizing Cuda window and there were many missing highlights at the bottom.

[pintassilgo]

Line 333:

w += text_len -1

Replace it by

w += text_len +1

That fixes the missing highlight at the right edge (last image of my initial comment).

[Alexey-T]

reading PROP_LINE_TOP / PROP_LINE_BOTTOM must ensure that all visible lines are handled. plugin updates this line range (TOP / BOTTOM) on scrolling if "visible_fallback" plugin option is on.

• do you have "visible_fallback" on?
• if yes, can you add to def find_visible_occurrences printing of 2 vars: _line_top and _line_bottom? and see them on scrolling and on maximize/restore. are they ok?

i don't get idea why adding 15 is needed.

[Alexey-T]

Line 333:

w += text_len -1

Replace it by

w += text_len +1

That fixes the missing highlight at the right edge (last image of my initial comment).

fix applied, ok.

[pintassilgo]

do you have "visible_fallback":true?

Yes, of course, it's required for all we are discussing here.

if yes, can you add to def find_visible_occurrences printing of 2 vars: _line_top and _line_bottom? and see them on scrolling and on maximize/restore. are they ok?

For this test, I removed the two 15 changes I made. Lines print(_line_top) and print(_line_bottom) were added after line 339.

When first visible line is 300 (from the start) and last visible is the first portion of line 307 (it's a long wrapped line), console shows 299 and 306.

After I scroll a bit untill I can see the issue, console shows 300 and 307, while first line at the top is the rest of wrapped line 301 and the last visible line is the rest of wrapped line 308. This way, the last few matches aren't highlighted:

Now I maximized the window, it was still printed 300 and 307, but now first line is 301 and last line is 319. All the matches starting from the last ones of line 308 aren't highlighted, the highlights are still the same as when window was small:

As I said, the two 15 values are needed to fix the scroll issue, but even then there's still a need to fix the resize issue.

[Alexey-T]

1.

This way, the last few matches aren't highlighted:

need to add print debug to plugin to find what is wrong. maybe smth like w variable is small? or limit to line_len exists?

2. i reproduce problem with window maximize-restore. gtk2: print is showing old values. qt5: print don't occur

repro plugin for on_scroll: cuda_tst_rsz.zip

[pintassilgo]

Sorry for the dumb question, but how to install plugin from zip?

[Alexey-T]

Sorry for the dumb question, but how to install plugin from zip?

open .zip in Cud, it will suggest to install it.

fixed beta with ok on_scroll firing on wnd maximize/restore. http://uvviewsoft.com/c/

[pintassilgo]

With cudatext-qt5-8 it looks highlights are updating correctly when I resize and there's no missing matches at the edges. Is that expected, did you fix this?

What I'm noticing now is Cuda is failing to clear status message. When there are highlights, the message displays index of the highlight under the caret and the total of matches highlighted. But if I click over a blank space, of if open a new empty document, I expect this message to be cleared because there are no longer highlights, but the message is still there. This should be fixed.

[pintassilgo]

And it would be good to, instead of displaying time time spent to process the highlights, the message warns when the highlight count doesn't cover the entire document when it's off the limits.

Now:

Matches highlighted: 202/369 (16ms)

Suggested:

Matches highlighted: 202/369 (partial)

The (partial) string would only be displayed when visible_fallback is true and current document is off the limits, so user shouldn't assume the count is for the entire document.

Btw, I think visible_fallback should be true by default, but I don't really care about the default value.

[pintassilgo]

What I'm noticing now is Cuda is failing to clear status message.

Can be fixed by appending app.msg_status('') at line 172, inside def work, next to if not res:.

[pintassilgo]

And it would be good to, instead of displaying time time spent to process the highlights, the message warns when the highlight count doesn't cover the entire document when it's off the limits.

I implemented that by:

1. Adding hili_full_doc = True at the top (I did it at line 31).
2. Adding global hili_full_doc at the beginning of def _get_occurrences (I did just above global occurrences).
3. Replaced app.msg_status(_('Matches highlighted: {}/{} ({}ms)').format(idx, ncount, tick)) by:

       if hili_full_doc:
           app.msg_status(_('Matches highlighted: {}/{} ({}ms)').format(idx, ncount, tick))
       else:
           app.msg_status(_('Matches highlighted: {}/{} (partial) ({}ms)').format(idx, ncount, tick))

But I believe displaying the time spent in status is not expected or useful for the user, it's only for debug, so it should be removed by default.

[pintassilgo]

If you approve these changes, now I'm satisfied with the results, pending just the suggestion to remove the tick in status (maybe leaving it under some debug flag/option). I also keep the suggestion to turn visible_fallback on by default, but this decision doesn't affect me.

This plugin uses on_scroll, so you say it degrades performance, but I don't notice any issues here, for me it's fine.

[Alexey-T]

Can be fixed by appending app.msg_status('') at line 172, inside def work, next to if not res:.

applied to git, okay.

[Alexey-T]

And it would be good to, instead of displaying time time spent to process the highlights, the message warns when the highlight count doesn't cover the entire document when it's off the limits.

I implemented that by:

applied to git, okay. with little modification. removed 'ms' time display, ok.

[Alexey-T]

just the suggestion to remove the tick in status (maybe leaving it under some debug flag/option). I also keep the suggestion to turn visible_fallback on by default

done. done. in git.

[pintassilgo]

Thanks. Since you remove the tick, there are some unused code left, like

tick = round((time.time() - time_start) * 1000)

Also references to time_start, maybe even the import time. If you plan to revisit this matter later, at least comment out those lines.

[Alexey-T]

agree, done in git.

[pintassilgo]

Thanks. I think we're done here. To finish this whole highlight/find thing, imo the only fix missing is #5424, particularly #5424#issuecomment-1996713250