Observe mousemoveevent option #2111
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…events so that we can unify all of our option observation under one method
Kethku
force-pushed
the
observe-mousemoveevent-option
branch
from
2877a44
to
8a4f44f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tracks the mousemoveevent option so that we dont send mouse move events when we shouldn't.
To do this I updated our settings infrastructure to allow option settings. This is done by adding an
optionattribute to the field that should be backed by an option instead of a global variable. For now all options require a name as they are normally named by neovim and have strange conventions.While I was in these systems I also added a Changed event which is raised whenever a field on a setting object is updated. This allowed me to remove our current option observation code in favor of tracking updated events.
@fredizzimo to do this I recombined the keyboard settings and window settings structs so that they could both be handled in one location. I believe you introduced that change (maybe) so lmk if you think that will cause problems. Another possible solution for this might be to allow our event system to have multiple subscribers. I didn't go this route because thats a larger change that I didn't want to sneak in here. But I could be convinced if somebody feels strongly.
Fixes #1838
What kind of change does this PR introduce?
Did this PR introduce a breaking change?
Global variable Neovide settings are no longer set in nvim on startup.
This feature was a bit odd from the start as I don't really see the use case beyond maybe giving auto completes for variables on startup? I removed it because option values are best implemented via an Option type. Since we don't own the Option or Value types, I couldn't implement From for Option which I needed for lines and columns. I could see a usecase for bidirectional settings in the future, but this initial setting seems of limited value. However on the off chance somebody depends on it, I'm capturing the change here.