onivim / oni

Oni: Modern Modal Editing - powered by Neovim
https://www.onivim.io
MIT License
11.35k stars 299 forks source link

Opening Multiple Files #503

Closed CrossR closed 7 years ago

CrossR commented 7 years ago

Hi,

Love the project! I've been messing and using this in my day to day, and its made my Windows Dev experience much better, and very close to how I edit in Linux with pure neovim.

One thing I wonder is how to open multiple files? I expected that I would be able to use the tab or split open from the menu and select multiple files to have them all open at once, but that doesn't seem to be the case.

Normally I'd use some form of fuzzy file opener to just open up more tabs/splits, but I'm also having an issue where that isn't opening the current directory...I can open a separate issue for that though.

This occurs with 0.25 and 0.26rc in Windows 10.

I've identified the relevant bit in Menu.js that deals with this, so I guess I could submit a PR?

It looks like the dialog.showOpenDialog just needs the multiSelections property added, and a bit of logic to deal with how the first file is dealt with. If there is no file open should a new tab be made anyway and leave an empty tab, or should we open the first file in the first tab, assuming no changes have been made in that tab?

Wasn't sure if I was missing something that would let me already do this, or there was a reason it didn't work as (I) expected.

Cheers!

keforbes commented 7 years ago

There isn't really a good way to open multiple files yet, although I did recently add drag-n-drop support for multiple files. So you can give that a try in the short term.

On Wed, Jun 28, 2017, 1:25 PM Ryan C notifications@github.com wrote:

Hi,

Love the project! I've been messing and using this in my day to day, and its made my Windows Dev experience much better, and very close to how I edit in Linux with pure neovim.

One thing I wonder is how to open multiple files? I expected that I would be able to use the tab or split open from the menu and select multiple files to have them all open at once, but that doesn't seem to be the case.

Normally I'd use some form of fuzzy file opener to just open up more tabs/splits, but I'm also having an issue where that isn't opening the current directory...I can open a separate issue for that though.

This occurs with 0.25 and 0.26rc in Windows 10.

I've identified the relevant bit in Menu.js that deals with this, so I guess I could submit a PR?

It looks like the dialog.showOpenDialog just needs the multiSelections property added, and a bit of logic to deal with how the first file is dealt with. If there is no file open should a new tab be made anyway and leave an empty tab, or should we open the first file in the first tab, assuming no changes have been made in that tab?

Wasn't sure if I was missing something that would let me already do this, or there was a reason it didn't work as (I) expected.

Cheers!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/extr0py/oni/issues/503, or mute the thread https://github.com/notifications/unsubscribe-auth/AAx5ZW3krizpCJWA2qKcppmkQP2M-kLVks5sIqizgaJpZM4OIaix .

Bretley commented 7 years ago

The main issue with opening multiple files is behavior. We can't know for sure how you'd like to open your files, and even if we can we don't know that others will.

As for your fuzzy finder issue, is it loading forever or is it just empty?

CrossR commented 7 years ago

Yeah that's true, after I posted the comment I tried to work out what is the most sensible behavior, but there is arguments for a bunch of different ways.

It's not empty, it's just regardless of where my file is open, it only shows files relative to oni in some fashion, I think the electron folder. If I swap the search command to the suggested windows one in Config.ts, It swaps to the program files folder that oni is installed in.

Just looks like on my machines it's unable to open the current folder, so always opens the oni relative ones.

bryphe commented 7 years ago

@CrossR , Glad to hear Oni is making your Windows Neovim experience better, that's great 👍 👍 Thanks for trying it out and for the feedback! Really appreciate you opening up the issue.

It sounds like you dug in and have a PR for opening multiple files. It's definitely a scenario we'd like to support and is really common in workflows. I don't think there are any technical blockers, since as you mentioned there is the multiSelections property available on the electron dialogs.

If there is no file open should a new tab be made anyway and leave an empty tab, or should we open the first file in the first tab, assuming no changes have been made in that tab?

Good question. As you mentioned you could make arguments for either behavior. As an aside, I'm working on adding a visible tab list / buffer list in Oni: image

I think that the first behavior - opening file in the first tab - is more natural in that context (otherwise you get an empty tab).

Wasn't sure if I was missing something that would let me already do this, or there was a reason it didn't work as (I) expected.

Thanks for checking - in general, we've been very incremental about adding features, and need help filling them out completely - so if something isn't working the way you expect, it's probably Oni missing something and not you 😄 . Glad you logged an issue - probably other people had the same question!

And if you have a PR, that's even better - I'm very open to accepting PRs and happy to help you get it in.

Oh and I opened #506 to track the discussion around the Fuzzy Finder / QuickOpen - I figured we can keep this issue for a fix for the multi-selection, and then #506 to track the fuzzy finder problems. Likely they will be different code changes anyway.

Thanks again for trying Oni & logging the issue!

CrossR commented 7 years ago

All sounds good to me!

I've add the 2 line change to make Split and Tab open multiple files, I just need to swap it now so it will check if the current tab is empty and has no edits in. Any suggestions on how to do that?

I did get it working to test how it felt by comparing against the Oni title, ie by checking the current Title wasn't "[No Name] - ONI" but thats hardly a good solution since the moment the title bar changes it will fail. I figure I'll either want to try and hook into and get the state of the current window/neovim instance, or possibly just call a vim command and get a response from that if possible.

bryphe commented 7 years ago

I've add the 2 line change to make Split and Tab open multiple files, I just need to swap it now so it will check if the current tab is empty and has no edits in. Any suggestions on how to do that?

Thanks for looking at this change! I'm assuming you're modifying Menu.js to handle multiple files coming back, specifically the callbacks from dialog.showOpenDialog:

                click: (item, focusedWindow) => {
                    dialog.showOpenDialog(mainWindow, ['openFile'], (files) => {
                        executeVimCommandForFiles(":e", files)
                    })
                }

The Menu.js file is in the main process, which doesn't know about Neovim - all of that action happens in the renderer process. So unfortunately we can't make that more robust check to see if the current tab is empty / has no edits - but if you look at those definitions, like executeVimCommand, we actually pass a message to the renderer process like:

    const executeVimCommand = (command) => mainWindow.webContents.send("menu-item-click", command)

This message - menu-item-click gets sent to the renderer process, over in NeovimEditor.tsx, which can talk directly to Neovim to get this more robust check:

        ipcRenderer.on("menu-item-click", (_evt: any, message: string) => {
            if (message.startsWith(":")) {
                this._neovimInstance.command("exec \"" + message + "\"")
            } else {
                this._neovimInstance.command("exec \":normal! " + message + "\"")
            }
        })

The _neovimInstance over here lets us do a bunch of things - we can ask about the current window, you can evaluate a VimL block, etc. Check out NeovimInstance.ts (specifically, the INeovimInstance interface to see the capabilities / examples).

So in this case, what we might want to do, is create a new message type - like open-files that passes an array from Menu.js to NeovimInstance.tsx (main process -> renderer process). We could hook it up much like the menu-item-click event. Then we could use the _neovimInstance in the handler for our new open-files event to execute our more robust check to see whether we have a buffer open, and branch based on that.

Hope that helps! 😄 It's a lot of moving parts, so please let me know if you need any more details.

CrossR commented 7 years ago

I've almost got this working now.

It works for all cases except opening multiple splits whilst having no file open, and no modifications made.

I've got some VimL that I'm evaluating that says "If the file is modified, or the file is not called [No Name], then the first file in a new split/tab. If the file has no modifications and is called [No Name] then we can instead open the first file in the current window. The rest are then just iterated over in a loop and we call tab or split on them.

That works fine for tabs. For splits, I think the changing of the buffer or the async nature of using Promises with _neovimInstance.command( ) causes splits to fail after the first file.

I've done some poking around, but was wondering if anyone had any ideas?

bryphe commented 7 years ago

It works for all cases except opening multiple splits whilst having no file open, and no modifications made.

Awesome, sounds like great progress!

For splits, I think the changing of the buffer or the async nature of using Promises with _neovimInstance.command( ) causes splits to fail after the first file.

What's the VimL you are using here? If you need to do major chaining / branching, you can potentially create a VimL function that we call directly from the UI layer to do the work. There's a few examples in oni/vim/core/oni-core-interop/plugin/init.vim. The OniApiInfo and OniUpdateWindowDisplayMap are good examples of ones that have sort of extended logic that were easier to implement directly in VimL. I hope to keep this layer relatively thin, though.

Hope that helps. Feel free to open up a PR, I'm happy to help take a look at it further if that still didn't solve the issue.

keforbes commented 7 years ago

I believe this issue was resolved with #526 so I'll go ahead and close it.