Update for Nvim 0.3.2

[CrossR]

This change moves the call to _checkAndFixIfBlocked for the latest Nvim version.

This is because the API has changed, such that it does not work any more. (Buffers are not setup until nvim_ui_attach is called, which broke the way it worked, and caused us to hang).

Currently, the call is just moved to after the editor is setup. However, in reality we want to remove the hacky code entirely, since it isn't the best way of dealing with it now nvim has given more support. In the current code state, if the code is just straight removed, the messages do not show at all, and the user just has to hit enter blindly, which is exactly what _checkAndFixIfBlocked was added to stop.

If we can show the errors, that would be better. If we can show them in some Oni UI, that would be nicer again. I think that may be possible once we externalise the messages? Not 100% on that.

Do you have any opinion on this @bryphe ? I'm not fully sure why the errors aren't displayed, since https://github.com/neovim/neovim/wiki/Following-HEAD#20180922 seems to imply we should be getting that info now, so its likely that Oni just isn't able to show it yet due to how we init the editors.

[codecov[bot]]

Codecov Report

Merging #2613 into master will decrease coverage by 0.04%. The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2613      +/-   ##
==========================================
- Coverage   45.77%   45.73%   -0.05%     
==========================================
  Files         361      361              
  Lines       14637    14650      +13     
  Branches     1927     1931       +4     
==========================================
  Hits         6700     6700              
- Misses       7706     7719      +13     
  Partials      231      231

Impacted Files	Coverage Δ
...ng/Tutorial/Tutorials/TargetsVimPluginTutorial.tsx	92.3% <ø> (ø)	:arrow_up:
browser/src/neovim/NeovimProcessSpawner.ts	21.27% <ø> (ø)	:arrow_up:
browser/src/Editor/NeovimEditor/NeovimEditor.tsx	8.52% <0%> (-0.11%)	:arrow_down:
browser/src/neovim/NeovimInstance.ts	5.37% <0%> (-0.11%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update eefa283...1a2c7a4. Read the comment docs.

[akinsho]

@CrossR great you picked up on the change 👍 re. needing to still use check if blocked, from the link you posted it doesn't seem like anything was added to help with the actually handling of init.vim errors, seems like we'd still face the same situation since theres no api for those sorts of errors (i.e. an event to inform us of them and an api to use to clear the blockage), although I wonder if the section

For an UI that wants to do additional initialization after init.vim the following pattern can be used: Before nvim_ui_attach send a singlenvim_command request with the command "autocmd VimEnter * call rpcrequest(1, 'vimenter')". In the vimenter method handler the UI can then safely execute any requests, and nvim will only enter normal mode when this handler returns.

Might be of some use although I'm not sure how it differs from just calling check if blocked (other than it would occur before going into normal mode)

[justinmk]

it doesn't seem like anything was added to help with the actually handling of init.vim errors, seems like we'd still face the same situation since theres no api for those sorts of errors (i.e. an event to inform us of them and an api to use to clear the blockage)

After the UI is attached, the old situation is no longer possible: init.vim errors will display like any other errors (from plugins or any other normal runtime error). That's the purpose of https://github.com/neovim/neovim/wiki/Following-HEAD#20180922 : to allow a UI to attach. Previously nvim would block waiting for user-input before the UI could attach. So there's no longer a need for special handling of startup, beyond what is normally done during runtime.

[akinsho]

@justinmk thanks for the explanation 👍, that makes sense

[bryphe]

@CrossR - the changes look good to me! Thanks for taking on this work 👍

I'm glad to see that hacky code start to go away...

I'm not fully sure why the errors aren't displayed, since https://github.com/neovim/neovim/wiki/Following-HEAD#20180922 seems to imply we should be getting that info now, so its likely that Oni just isn't able to show it yet due to how we init the editors.

With the latest changes - do we get init.vim errors shown correctly? Or are there still issues?

[justinmk]

With the latest changes - do we get init.vim errors shown correctly

The current PR still calls _checkAndFixIfBlocked() with Nvim 0.3.2, which defeats the purpose of the aforementioned new --embed behavior. IIUC _checkAndFixIfBlocked tries to cancel out of a "Press Enter" prompt, but that's no longer necessary, and furthermore it will prevent errors from displaying by the normal mechanisms.

Simply remove the call to _checkAndFixIfBlocked entirely for Nvim 0.3.2.

[CrossR]

With the latest changes - do we get init.vim errors shown correctly? Or are there still issues?

With the latest update to nvim (0.3.2) and _checkAndFixIfBlocked removed entirely for 0.3.2, the result (for cases of init.vim issues) is as follows:

Which is why I had left the call in so far. I assume what is happening is that Oni hasn't initialised the editor in some fashion, or the errors are not being captured correctly on our end etc. Once I hit enter, the editor loads as expected. With the call left in, the behaviour is the same as the latest release and nvim 0.3.1.

I would prefer to remove the call entirely, so do you have any idea what would be waiting/causing the errors to not being shown?

[justinmk]

I assume what is happening is that Oni hasn't initialised the editor in some fashion, or the errors are not being captured correctly on our end etc

It must be the former, because, unless Oni usually "captures" errors during normal operation, there should be no need to capture anything on startup.

@CrossR in your screenshot I don't see anything that looks like "Oni UI started". Oni should draw the UI before doing anything else.

Once I hit enter, the editor loads as expected.

That means that a prompt is waiting but wasn't displayed. Did Oni receive a redraw UI event with the screen contents? If so, it should draw the screen.