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

rplugin: conditionally enable nested notifications #262

Merged
merged 1 commit into from Nov 8, 2017

Conversation

bfredl
Copy link
Member

@bfredl bfredl commented Jun 17, 2017

@justinmk
Copy link
Member

Instead of introducing "urgent" as a jargon, can we re-use the "deferred" vs "non-deferred" concept?

@bfredl
Copy link
Member Author

bfredl commented Jun 24, 2017

@justinmk The only issue is that it makes deferred=True the default. In general bool options are False as default and True for the special case.

@justinmk
Copy link
Member

In general bool options are False as default and True for the special case.

This rule really gets in the way. It is IMO much lower-priority than good names, consistency, and pretty much everything else.

@bfredl
Copy link
Member Author

bfredl commented Oct 29, 2017

consistency, and pretty much everything else.

In this case consistency/everything else is that rplugin infrastructure (with @tarruda:s original restriction, or the neovim/neovim#6544 reimplementation) do implement an modified actor model where each rplugin only will handle one top-level request or notification at a time. (the modification is that nested requests are ok, but they can only be invoked because the rplugin itself did something that requires the request to be handled, to avoid dead-lock). On the other hand nvim is best described as a state machine of user input, and "deferred" /"undeferred" ("async" should never have been used for this) refers to what input states this event can be processed in, not whether the event is "nested" or not. It is therefore a different distinction and I see no point to use the same terminology.

"urgent" might not be optimal, but preferably it should be a word that positively describes how it deviates from the default (actor-like) model. Perhaps allow_nested, which indicates the relevant consequence (that it might be called nestedly inside a request handler)

@bfredl bfredl force-pushed the nodelay branch 2 times, most recently from 0e71c04 to 80c0145 Compare October 29, 2017 08:47
@bfredl bfredl changed the title WIP: make ready for nodelay rplugin: conditionally enable nested notifications Oct 29, 2017
@bfredl bfredl force-pushed the nodelay branch 3 times, most recently from 786c581 to 9752bbc Compare November 8, 2017 18:07
@bfredl bfredl merged commit 6a7139b into neovim:master Nov 8, 2017
bfredl added a commit to bfredl/python-client that referenced this pull request Nov 8, 2017
Brings the client up-to-date with Nvim 0.2.1

Changes since 0.1.13:
* a2e1169 Fix tests on windows (neovim#201)
* 9a0e729 fix an indexing bug when setting lines in a Range object (neovim#270)
* 4abd5d0 Documentation update (neovim#272)
* a703b47 make sure logging always uses UTF-8 regardless of locale (neovim#276)
* 68aa352 argument to allow nested notification handlers (neovim#262)
bfredl added a commit to bfredl/python-client that referenced this pull request Nov 8, 2017
Brings the client up-to-date with Nvim 0.2.1

Changes since 0.1.13:
* a2e1169 Fix tests on windows (neovim#201)
* 9a0e729 Fix an indexing bug when setting lines in a Range object (neovim#270)
* 4abd5d0 Documentation update (neovim#272)
* a703b47 Make sure logging always uses UTF-8 regardless of locale (neovim#276)
* 68aa352 Argument to allow nested notification handlers (neovim#262)
bfredl added a commit to bfredl/python-client that referenced this pull request Nov 8, 2017
Brings the client up-to-date with Nvim 0.2.1

Changes since 0.1.13:
* a2e1169 Fix tests on windows (neovim#201)
* 9a0e729 Fix an indexing bug when setting lines in a Range object (neovim#270)
* 4abd5d0 Documentation update (neovim#272)
* a703b47 Make sure logging always uses UTF-8 regardless of locale (neovim#276)
* 68aa352 Add argument to allow nested notification handlers (neovim#262)
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