-
Notifications
You must be signed in to change notification settings - Fork 761
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
Add search widget #1054
base: master
Are you sure you want to change the base?
Add search widget #1054
Conversation
Related ticket: https://youtrack.jetbrains.com/issue/VIM-2544 |
Hi, thank you for your interest, this is a nice feature. I've added a related ticket.
|
} | ||
|
||
class VimSearchWidget : StatusBarWidget, VimStatusBarWidget { | ||
var content: String = "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Directly accessing the content
here leaks incapsulation. Let's have special functions like setSearch(current, total)
and clearSearch()
.
(function names are subject to change).
@@ -1340,13 +1340,15 @@ abstract class VimSearchGroupBase : VimSearchGroup { | |||
} else if (lastPatternTrailing!![ppos + 1] == '?') { | |||
Direction.BACKWARDS | |||
} else { | |||
injector.listenersNotifier.notifySearchUpdated(editor, res) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a good place for running listeners. The finItOffset
should be a clean function that returns the result of the search. If it's not clean, let's not make it worse at least.
This widget is activated while doing a search (forward/backward, word search, etc.). It shows the current occurrence that you're on, as well as the total number of occurrences for the search terms. Works similarly to IntelliJ's built-in search tool.
This relies on search highlights being shown. But perhaps this is acceptable since the users who would care about this widget would also likely have highlights turned on?
If tests are expected for this, could you point me to some similar examples? I couldn't find tests, for example, for the macro widget.