[RFC] main: make --embed wait for first request so embedding UI can display startup messages #8754

bfredl · 2018-07-17T13:36:52Z

This allows to properly handle first buffer swap message, as well as
errors in startup files and --cmd commands in an external UI starting nvim as a child, just by using --embed-ui option instead of --embed

Note: this does nothing that a rpcrequest() in a --cmd and a corresponding handler in the UI client cannot do. But I think default matters, making a UI that has non-racy startup should be simple out of the box, rather than the UI needing to know details of the nvim startup process and event handling model even if it only ever wants to use nvim_ui_attach and nvim_input.

bfredl · 2018-07-17T13:43:19Z

Also, later on nvim_ui_attach should be made non-deferred, but it might require some refactors, for instance in a wait-return state it cannot redraw the screen, it should rather just retransmit the screen (or even just fake the message if the new client wants to be narrower than the current message displayed before the wait-return prompt). That will now however only affect connecting to running instance, this PR handles the start-up case.

jamessan

LGTM other than the lint warning

oni-link · 2018-07-17T15:48:24Z

src/nvim/main.c

@@ -819,6 +829,14 @@ static void command_line_scan(mparm_T *parmp)
            mch_exit(0);
          } else if (STRICMP(argv[0] + argv_idx, "headless") == 0) {
            headless_mode = true;
+          } else if (STRICMP(argv[0] + argv_idx, "embed-ui") == 0) {


Will this option (later) be added to nvim --help and the internal help?

It will be added now.

bfredl · 2018-07-17T15:50:20Z

Yes, I will add some docs and test also :)

bfredl · 2018-07-17T16:37:54Z

Messages sometimes get truncated, so this needs some more work.

bfredl · 2018-07-17T18:51:28Z

After some discussion with @justinmk I think we could use the following rule to avoid adding more an option:

with --embed nvim always waits for any request on channel 1 before continuing with startup, this will break some clients but not all non-ui --embed-ers as using --embed-ui as --embed would. The old behavior can still be recovered with nvim --headless --cmd "call stdioopen({'rpc': v:true})"

bfredl · 2018-09-04T12:39:51Z

Update: now handles messages more consistently with TUI (never show more than one "press RETURN" prompt, unless the screen is really full or something). Note: unlike early TUI early messages can here be affected by colors loaded by vimrc. Which means if one message was displayed before :color and one after, the first will appear with builtin colors and the second with user colors. I think this is better than (1) wiping away the earliest message before it can be seen or (2) showing two "press RETURN" prompts just to not mix messages with different color schemes. Anyway this situation should be pretty rare. (swap messages, which should be the most common, will be shown with the user colors, just like TUI).

justinmk · 2018-09-13T10:45:25Z

test/functional/helpers.lua

@@ -362,6 +362,7 @@ local function clear(...)
    table.insert(args, arg)
  end
  set_session(spawn(args, nil, env))
+  session:request('nvim_eval', "0")


How about a comment:

-- Dummy request so that --embed starts the UI.

justinmk · 2018-09-13T12:47:31Z

src/nvim/generators/gen_api_dispatch.lua

@@ -128,6 +128,7 @@ for i,f in ipairs(shallowcopy(functions)) do
    end
    newf.impl_name = f.name
    newf.remote_only = true
+    newf.async = f.async


Does this affect behavior somewhere or only api_info metadata ?

I must have forgotten to remove this line, shallowcopy(f) already handles this...

bfredl · 2018-09-18T10:48:27Z

Updated with a note in starting.txt, I will add a test or two also.

bfredl · 2018-09-18T12:43:13Z

Added tests.

justinmk · 2018-09-18T12:47:50Z

src/nvim/msgpack_rpc/channel.c

@@ -40,6 +40,8 @@
 static PMap(cstr_t) *event_strings = NULL;
 static msgpack_sbuffer out_buffer;

+static bool seen_stdio_request = false;


got_stdio_request is more common convention.

Give embeders a chance to set up nvim, by processing a request before startup. This allows an external UI to show messages and prompts from --cmd and buffer loading (e.g. swap files)

marvim added the RFC label Jul 17, 2018

jamessan approved these changes Jul 17, 2018

View reviewed changes

oni-link reviewed Jul 17, 2018

View reviewed changes

bfredl force-pushed the embed_ui branch 3 times, most recently from 8e1a1ef to a4653be Compare July 24, 2018 15:08

bfredl mentioned this pull request Aug 1, 2018

Lines and columns initialized too late? #2403

Closed

This was referenced Aug 27, 2018

diff mode does not seem to work equalsraf/neovim-qt#450

Closed

When neovim gets stuck while initializing neovim-qt does not respond equalsraf/neovim-qt#78

Closed

bfredl force-pushed the embed_ui branch 2 times, most recently from 4a75398 to 89fb426 Compare September 4, 2018 12:19

bfredl changed the title ~~[RFC] main: add --embed-ui allow to wait for embedding UI to connect~~ [RFC] main: make --embed wait for first request so embedding UI can display startup messages Sep 4, 2018

justinmk reviewed Sep 13, 2018

View reviewed changes

justinmk mentioned this pull request Sep 17, 2018

[RFC] Input Ctrl-C if Neovim is blocked waiting for input equalsraf/neovim-qt#462

Merged

bfredl force-pushed the embed_ui branch from 89fb426 to ee80a13 Compare September 18, 2018 10:47

bfredl force-pushed the embed_ui branch from ee80a13 to 997a8a5 Compare September 18, 2018 12:42

justinmk reviewed Sep 18, 2018

View reviewed changes

bfredl force-pushed the embed_ui branch from 997a8a5 to 0b349d4 Compare September 18, 2018 12:50

startup: wait for embedder before executing startup commands and files

bd8d43c

Give embeders a chance to set up nvim, by processing a request before startup. This allows an external UI to show messages and prompts from --cmd and buffer loading (e.g. swap files)

bfredl force-pushed the embed_ui branch from 0b349d4 to bd8d43c Compare September 18, 2018 17:23

bfredl merged commit 809fff9 into neovim:master Sep 18, 2018

justinmk removed the RFC label Sep 18, 2018

jamessan mentioned this pull request Sep 20, 2018

Handle blocked input equalsraf/neovim-qt#463

Closed

This was referenced Sep 20, 2018

[RFC] startup: don't erase screen on :hi Normal during startup (for non-newgrid UIs) #9021

Merged

[RFC] always wait for UI with --embed, unless --headless is supplied #9024

Merged

justinmk mentioned this pull request Apr 26, 2019

Multiple :echom commands in init.vim results in white screen equalsraf/neovim-qt#538

Closed

justinmk mentioned this pull request Sep 11, 2019

--embed: write errors to stderr #8541

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[RFC] main: make --embed wait for first request so embedding UI can display startup messages #8754

[RFC] main: make --embed wait for first request so embedding UI can display startup messages #8754

Uh oh!

bfredl commented Jul 17, 2018 •

edited

Loading

Uh oh!

bfredl commented Jul 17, 2018

Uh oh!

jamessan left a comment

Uh oh!

oni-link Jul 17, 2018

Uh oh!

bfredl Jul 17, 2018

Uh oh!

bfredl commented Jul 17, 2018

Uh oh!

bfredl commented Jul 17, 2018

Uh oh!

bfredl commented Jul 17, 2018

Uh oh!

bfredl commented Sep 4, 2018

Uh oh!

justinmk Sep 13, 2018

Uh oh!

justinmk Sep 13, 2018

Uh oh!

bfredl Sep 18, 2018

Uh oh!

bfredl commented Sep 18, 2018

Uh oh!

bfredl commented Sep 18, 2018

Uh oh!

justinmk Sep 18, 2018

Uh oh!

Uh oh!

Uh oh!

[RFC] main: make --embed wait for first request so embedding UI can display startup messages #8754

[RFC] main: make --embed wait for first request so embedding UI can display startup messages #8754

Uh oh!

Conversation

bfredl commented Jul 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bfredl commented Jul 17, 2018

Uh oh!

jamessan left a comment

Choose a reason for hiding this comment

Uh oh!

oni-link Jul 17, 2018

Choose a reason for hiding this comment

Uh oh!

bfredl Jul 17, 2018

Choose a reason for hiding this comment

Uh oh!

bfredl commented Jul 17, 2018

Uh oh!

bfredl commented Jul 17, 2018

Uh oh!

bfredl commented Jul 17, 2018

Uh oh!

bfredl commented Sep 4, 2018

Uh oh!

justinmk Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

justinmk Sep 13, 2018

Choose a reason for hiding this comment

Uh oh!

bfredl Sep 18, 2018

Choose a reason for hiding this comment

Uh oh!

bfredl commented Sep 18, 2018

Uh oh!

bfredl commented Sep 18, 2018

Uh oh!

justinmk Sep 18, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bfredl commented Jul 17, 2018 •

edited

Loading