equalsraf / neovim-qt

Neovim client library and GUI, in Qt5.
https://github.com/equalsraf/neovim-qt/wiki
ISC License
1.83k stars 171 forks source link

Adding support for multiple windows in the same process on macOS #850

Closed Furkanzmc closed 3 years ago

Furkanzmc commented 3 years ago

I'm openning this to continue the conversation from #848 so that we can keep the conversation separate.


On macOS, it's more common for windows to belong to the same process than the other way around. VimR works the same way, and all the common macOS apps. The only notable example that doesn't do that is Alacrity.

Having the windows belong to the same window makes window navigation much simpler with app exposè or "Cmd+`". This behavior would only be specific to macOS because having multiple windows on the other platforms provides no such benefits (I'm not 100% sure about this for Linux though. It's been a while since I was there.).

Here's how I think this would work.

I can think of a few cases when you want multiple nvim instances and or multiple top level windows, but I'm not sure if these match your motivation:

  1. run multiple nvim instances, one in each container/machine/etc
  2. neovim isolation is not perfect - e.g. some features are not 100% independent per tab , or i dont want to list all buffers just the buffers i am working on right now - this is probably addressable in neovim, however as a consequence people run multiple neovim instances anyway.
  3. multiple screens - however in this case what people usually wan is one neovim instance for multiple top level windows

Similar motivations. I use different environments/variables to manage my sessions and having them all in one nvim session is not feasiable for me.

Also, ideally the new instance in a window would:

1- Inherit the environment if it's launched in the terminal. This would mean that I can launch different Neovim instances in the same process with different Python virtual environments or whatever else environment people use. 2- Inherit the environment of the previously active window if it's launched from an existing window with a command.

jgehrig commented 3 years ago

At a high-level, this all sounds good to me.

Keep in mind our design will be impacted by what is possible/practical within the Qt Framework.

How will we handle the joining of QProcesses or spawning of a second QMainWindow? Does Qt provide an API or guidance on how to handle this for MacOS?

Generally, the pattern I've seen used here:

  1. Spawn process nvim-qt (first instance)
  2. Check if another process is already running. None found -> Initialize as normal
  3. Spawn process nvim-qt (second instance)
  4. Check if another process is already running. Found -> Begin hand-off to already running process a. Open IPC Socket to existing process b. Send startup info to existing process (SIGNAL?) c. Exit
  5. Original process receives startup info (SLOT?), spawn new QMainWindow.

Maybe it makes sense to write some quick proof-of-concept code, to evaluate various hand-off mechanisms...

A few comments:

1- Inherit the environment if it's launched in the terminal.

I agree this behavior would be nice. I think it will also be quite tricky...

We may be at the mercy of however Qt handles environment variables. I think they are coupled to QProcess, and however it was spawned. We may be forced to inherit from the first launched QProcess instance?

From a running nvim-qt instance, a new window can be launched with a command. And this command would only be available for macOS, or it would exist but be no-op on other platforms. I think it not existing is better.

Are you suggesting a new command line argument?

I too would like to avoid a command-line argument to join/spawn a new window. In my opinion, nvim-qt filename.txt should always spawn a new QMainWindow. Adding a buffer to an existing window can be accomplished using existing tools like nvr or from within nvim.

If absolutely necessary, we can add platform-specific arguments and behavior differences. See arguments.h or input.h as a design pattern example.

Like you said, I don't see a way to do this without breaking :cq.

I think we can keep :cq support to some degree.

When one process owns multiple windows, we can simply return the last known m_exitStatus when the process exits. Each time a window closes, it can update the value of a static m_exitStatus. Once the final window closes, we exit with code m_exitStatus. We may need to refactor some of the code, so m_exitStatus is thread-safe and static.

This approach should be possible, right?

equalsraf commented 3 years ago

I'm not so sure about how to handle open -a nvim-qt. My gut feeling is that it would open the file either in the first window created, or in the one that was last activated. I don't have a technical understanding of how it works yet.

The passing of the even in handled my Mac OS X - on the Qt side there is an event that hands you the file path

https://github.com/equalsraf/neovim-qt/blob/94e4d1f4cfd3f14fda3b3ddc84b5f677bd3c1c1b/src/gui/app.cpp#L86

currently this is then handled in the shell (simulated as a file drop handling - nvim handles this as :drop)

https://github.com/equalsraf/neovim-qt/blob/94e4d1f4cfd3f14fda3b3ddc84b5f677bd3c1c1b/src/gui/shell.cpp#L1680

My main question here would be - if you have multiple windows, which one will open the file?

1- Inherit the environment if it's launched in the terminal. This would mean that I can launch different Neovim instances in the same process with different Python virtual environments or whatever else environment people use.

If I am reading this correctly, you want the process to go like this

  1. you launch the first instance from your terminal (with PWD=/home/username), the first instance starts and inside neovim :echo $PWD prints /home/username
  2. now from a second terminal (with PWD=/home/username/project) you start a new neovim-qt window. A new nvim process is started, a new window for neovim-qt appears - and in this instance/window :echo $PWD prints /home/username/project - likewise every other command in the new window operates in a completely separate instance of neovim

This passing of the environment would not be possible without additional work - essentially a frontend to reach out to neovim-qt.

2- Inherit the environment of the previously active window if it's launched from an existing window with a command.

This sounds ok.


More generally we are starting to mix two different things here:

  1. the ability to launch multiple windows (i.e. nvim instances) from the same neovim-qt process
  2. mechanisms to find running instances and IPC to existing instances to do other things

These can be treated separately - it is probably best to focus on 1. for now.

Furkanzmc commented 3 years ago

When I mention environment, I mean the environment variables.

Here's how I see it working.

As @equalsraf suggested, I'll keep this issue only to the feature where we can create a new window with a command. I won't mess with an environment manipulation with each process.


My main question here would be - if you have multiple windows, which one will open the file?

I think it makes sense that the last window to have the focus would open the file with open -a.

When one process owns multiple windows, we can simply return the last known m_exitStatus when the process exits. Each time a window closes, it can update the value of a static m_exitStatus. Once the final window closes, we exit with code m_exitStatus. We may need to refactor some of the code, so m_exitStatus is thread-safe and static.

This makes sense. I'll do that.


@jgehrig, does this sound good to you? I'd like to keep the current PR and make these changes on top of that.

jgehrig commented 3 years ago

@Furkanzmc

All sounds great. I think building upon the existing PR is a good approach.

Thanks! I look forward to the feature :+1:

Furkanzmc commented 3 years ago

@jgehrig ad65c4166f3dbc3103a0a262ea3f78396ce8de87 seems to have broken the feature. The following doesn't work any more.

call GuiNewWindow({"server": expand("~/path/to/nvim.sock")})

The change to GuiNewWindow is the problem.

jgehrig commented 3 years ago

@Furkanzmc

Sorry for the break. Did you identify the root cause?

Do you think this change is the issue?

function! GuiNewWindow(...) abort
    let server = get(a:, 0, 0)
    let nvim = get(a:, 1, 0)

    call rpcnotify(0, 'Gui', 'NewWindow', server, nvim)
endfunction

Merge got delayed by #881. We're back online now. Once the issue above is resolved, I'll hit merge.

Furkanzmc commented 3 years ago

Yes. If you want the Vim function like that, you'll need to change the handling of that message and the documentation for the function as well. @jgehrig

jgehrig commented 3 years ago

This documentation?

                            *GuiNewWindow(opts)*
|GuiNewWindow()| creates a new window with the given map of arguments.
Supported arguments are:

The following parameters desceibe the fields in {opts}.

Parameters: ~
    {server} (optional, string): Server to connect to for the new window.
    {nvim} (optional, string): Path to nvim instance.

It is hard to follow what the command should be from this...

You described two optional arguments server and nvim. The VimScript above matches what you described in the documentation.

Your command above has 1 argument: a dictionary with optional keys "server" and "nvim".

The command can be structured however you like, but we should support :call GuiNewWindow() with no arguments.