Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve highlighting feedback #4

Closed

Conversation

PaulPorfiroff
Copy link

Tried to improve a bit highlight-on-fly feature:

  • Omit highlighting selection if there are no other occurences of it in text buffer. Useful when buffer spans multiple pages. Made this available as highlightSelectionExcludeUnique config option.
  • Constant delay of 100ms before highlighting after changing selection perceived really slow, as compared to default selection marker. I've just refactored that delay into highlightSelectionThrottle setting to allow modifying it with ease.

@t9md
Copy link
Owner

t9md commented Nov 28, 2015

Tried to improve a bit highlight-on-fly feature:

  • Omit highlighting selection if there are no other occurences of it in text buffer. Useful when buffer spans multiple pages. Made this available as highlightSelectionExcludeUnique config option.

I don't need this feature, what the purpose of this?
This is the feature misleading user to think highlight not working?
I don't want introduce complexity to codebase for this.

  • Constant delay of 100ms before highlighting after changing selection perceived really slow, as compared to default selection marker. I've just refactored that delay into highlightSelectionThrottle setting to allow modifying it with ease.

So you want make delay user-configurable? I don't thinks 100ms is not problematically slow for most of user. But I can add this config params, Is this really need?

@t9md
Copy link
Owner

t9md commented Nov 28, 2015

For about your 1st suggestion, your concern is performance overhead for highlight, to avoid scan whole buffer, you have following configuration parameter to control this

  • displayCountOnStatusBar: (currently need restart to change effective, I can change this later)
    if disabled, don't scan whole buffer to show count on statusbar.
  • highlightSelectionExcludeScopes:
    You can disable for specific filetype by specifying scope.
  • highlightSelectionMinimumLength

And highlight itself won't scan whole buffer, only scan visible buffer ranges, so theoretically its performance impact is same regardless of number of line of buffer.

t9md added a commit that referenced this pull request Nov 28, 2015
@t9md
Copy link
Owner

t9md commented Nov 28, 2015

Sorry I wanted my commit to bind to issue #5.

@t9md
Copy link
Owner

t9md commented Nov 28, 2015

Released v0.3.9.
By the way your original PR use getCountForKeyword to check its occurrence, if you want mitigate performance impact, avoiding to call getCountForKeyword is important since its scan whole buffer.
So for this purpose, use showCountOnStatusbar config param.

@t9md t9md closed this Nov 28, 2015
@PaulPorfiroff
Copy link
Author

Sorry for way too late response.

For about your 1st suggestion ...
I don't need this feature, what the purpose of this?

Just to clear things up, I was talking specifically about feature from #1 purposal, not about fixed markers.

This suggestion allows quickly reason about unused or not declared variables/functions/types in multipage buffers.

E.g. given this part of buffer visible, you can't tell whether forgottenInTheAnnalsOfHistory is some global variable or just an unused clutter by looking at first screen, while the second one clearly shows there are no other occurences found, so may remove this guy.

quick-highlight-pr4-1
quick-highlight-pr4-2

for this purpose, use showCountOnStatusbar config param

Using displayCountOnStatusBar config could be one option. But status bar count doesn't show up without dispatching quick-highlight:toggle command simply by modifying selection.

So I opted for another option: just to omit highlighting, as e.g. KDE Kate highlight selection extension does.

Perhaps you're right, that's a bit out of concept of status bar icon, but I liked it this way, so shared.
Maybe, showing status bar icon always and falling back to this behaviour when icon is turned off in options, would be the way to go? Overwhelming, though.

Your concern is performance overhead for highlight ...

Highlight itself won't scan whole buffer, only scan visible buffer ranges

Yeah, that's why I wanted an explicit option turned off by default for new behaviour.

@t9md
Copy link
Owner

t9md commented Nov 28, 2015

So you want to distinguish one occurrence or may occurrence of keyword for hightlightSelection.
Understood.

Currently you can know number of occurrence by manually toggle and check statusbar icon as you described.

After I understand background of this PR but I still think this feature is misleading and dont want to add even if KDE support this.

I thinks updating occurrence count icon on highlightSelection solve your requirement.
Im' also thinking about new feature to show count on hover indicator like I did in other my package.

Anyway I need some time.

e.g. incremental search on my vim-mode-plus package

gif

@PaulPorfiroff
Copy link
Author

So I opted for another option: just to omit highlighting, as e.g. KDE Kate highlight selection extension does.

Please, excuse my stupid mistake and just ignore as e.g. KDE Kate highlight selection extension does. part. That's incorrect example.

Im' also thinking about new feature to show count on hover indicator like I did in other my package.

That feels too find-and-replaceish and heavy to me in current case (meaning on-the-fly selections).

Nevertheless, your package fits best at the moment, thanks.

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

Successfully merging this pull request may close these issues.

None yet

2 participants