Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

Integrate Yode into Oni #894

Open
hoschi opened this issue Nov 8, 2017 · 9 comments
Open

Integrate Yode into Oni #894

hoschi opened this issue Nov 8, 2017 · 9 comments

Comments

@hoschi
Copy link

hoschi commented Nov 8, 2017

Yode allows you to edit parts of a JS file, at the moment functions. It allows the user to stay focused on the important parts of a task. Past discussion started here

There is a guide to integrate Yode in an editor with a reference implementation: https://github.com/hoschi/yode/blob/master/docs/integration.md

Current tasks on Yode side: https://github.com/hoschi/yode/projects/2

Last comment is here

@hoschi
Copy link
Author

hoschi commented Nov 8, 2017

It's actually great timing, as I'm going to start working a bit on some new IEditor implementations - a file explorer window, and a markdown previewer. So I think those will be good test cases - and maybe we can implement Yode in parallel as an IEditor?

I looked into the Code I found but there wasn't much. As I understand it an IEditor renders its contents on its own and doesn't use NeoVim? @bryphe

I think, in Oni terms, Yode would use a normal NeovimEditor enhanced with stuff a plugin API would give a Oni-Yode-Plugin, so I can use NeoVim editing capabilities.
Yode integrates with "text changed" hooks at the moment. I need a way to get notified about text changes of the user to refresh Yodes state.
Also it needs to set text to e.g. change the file when the user changed a function in its own function buffer.
So this can be done with hooks/callbacks in JS which translates to native NeoVim events/autocomands? I'm not sure if this is the job of an IEditor because NeovimEditor implements already that interface.
This was just a short version or big picture, the integration docs have more events listed/linked to code.

Additional things the referencen implementation misses and would be needed in Oni:

  • a save hook for function buffers, because they can't be written as they are not a file, so the file should be written instead.
  • buffer deletion is implemented in core, but not in the demo UI ... this "should" work

@badosu
Copy link
Collaborator

badosu commented Nov 9, 2017

Will this be provided as an optional plugin for Oni?

I am not most people would like this be enabled by default.

@TalAmuyal
Copy link
Collaborator

I took a look on Yode's into video and got really excited - It looks AWESOME!
It's at the top of my Oni wish list!

I hope to have a chance helping with it (right after #49).

@bryphe
Copy link
Member

bryphe commented Nov 9, 2017

Thanks for moving this to another issue, @hoschi !

Yes, Yode looks really cool! The intro video was very well done - I need to do something like that for Oni 😄

I looked into the Code I found but there wasn't much. As I understand it an IEditor renders its contents on its own and doesn't use NeoVim? @bryphe

I realized this is something that has changed a bit so I wanted to give some ideas here. There's really two levels - the concept of an IEditor and an IWindowSplit.

The IWindowSplit is super simple - it's just an element that takes up space in the window layout. Right now, it only has a render method that returns a React component. Later, we'll need to add some sort of focus or enter/leave events, but other stuff like rendering and input handling is up to them at the moment.

The idea is you can create a split very easily with the Oni API, like:

class MyWindowSplit {
       public render(): JSX.Element {
            return <div>Hello World</div>
       }
}

const myWindowSplit = new MyWindowSplit()

Oni.windows.split(SplitDirection.Right, myWindowSplit)

That kind of split would be super boring though and not too useful.

We can implement a few things with the split though:

  • A split for rendering Markdown based on the active buffer
  • A split for a file explorer
  • A split for a browser window
    etc

Even the status bar could be refactored to a fixed height 'split' at the bottom.

These ones wouldn't necessarily need to even be integrated with Neovim, although for stuff like the file explorer it would make sense to do this, because we want to have a consistent navigation and input model.

And beyond that, the core of Oni is the IEditor, which also has a render method, but handles a bunch of stuff - input, notifying when buffers have changed, notifying when the mode has changed, integration with language servers, etc. NeovimEditor is the main one that we have today and has an associated NeovimInstance.

The model we have at the moment is somewhat busted in that NeovimEditor is essentially an IWindowSplit with a bunch of extra things. The main difference between it and a pane is that it is registered with Oni.editors and so plugins can interact with it that way. An example of where this abstraction is somewhat broken is that the INeovimEditor is responsible for rendering the tab bar.

What I'd like to do is basically have some special splits for working with editors - an TabbedEditorCollectionSplit that manages a collection of IEditors. You could imagine that the TabbedEditorCollectionSplit could have a bunch of IEditors, and it could take the responsibility of rendering the tab bar, as well as rendering the active editor for the selected the tab.

The nice thing about this abstraction is that the yode model fits in pretty easily - it would still manage a collection of IEditors, but instead of having a model of tab + only one editor shown, it could show them in whatever layout made sense.

There's still some work left to get there:

  • There's a bunch of assumptions in the code right now that there is only ever one NeovimEditor (and one NeovimInstance), we'll have to break those assumptions.
  • Need to factor out the tab bar functionality from NeovimEditor to something more general like the TabbedEditorCollectionSplit
  • The window split logic still has a bunch of //TODOs (in WindowManager.ts 😄 )

Here's some ideas for getting started prototyping:

  • We could start by creating a YodeWindowSplit that implements a render - just a hello world render to make sure it's working OK
  • Add a single NeovimEditor to that split with a fixed height / width, and see if it works
  • Bring in the Yode API and hook it up to the positioning logic for that component

Will this be provided as an optional plugin for Oni?

It might be jarring when people come into Oni having an expectation of a Neovim or VSCode experience. Having it is a plugin seems reasonable - I can see some really cool cases for bringing it in by default for a few cases, like for Find all References, or Go to Definition, or Go to test case, etc. So it might not be the default strategy when you open Oni, but perhaps we surface it in some of those places?

Sorry for the wall of text, I hope that made sense! On my end, I'm starting to look at the file explorer + integrated browser window, so I hope as I drill into those it might help clarify some of the unknowns (and have more examples in place).

@hoschi
Copy link
Author

hoschi commented Nov 10, 2017

So it might not be the default strategy when you open Oni, but perhaps we surface it in some of those places?

I agree in general. I also really think we need to use this in day to day editing in a "beta phase" and see how things work, because I think it feel really different in some places. One example I already thought of is editing functional React components where I think want to focus on the render function instantly. Not

  • open that file
  • move to render function body with vim movement
  • hit keybinding to "open function under cursor" in a new split
  • close the file

Which no one does then, because the outcome for the three additional steps are not worth in every situation. Instead an action for "open render function of that component" make 100% more sense here. There are problems like "how to generalize this logic and not focus on React", but I just wanted to give an example of that new use cases arise when it is there. On the other side "Go to definition" is a "no brainer" for me, opens a function buffer here saves you a good amount of space. If we don't want this as default, there can still be two binding for the "open file and jump to position" and the "open function which contains this definition, or if this thing is a function just open it" without much additional logic I think. With the command palette it is now even easier to recognize these things for new users as both commands show when you type "Go to def" 😁

I repeat some things in my words to make sure I got it right, tell me if I don't @bryphe 😉

There's a bunch of assumptions in the code right now that there is only ever one NeovimEditor (and one NeovimInstance), we'll have to break those assumptions.

At the moment there is one IEditor on the screen, NeovimEditor, which renders the "text editing" part like buffers and splits. Splits of the editing space are rendered by NeoVim at the moment, not by Oni.

An example of where this abstraction is somewhat broken is that the INeovimEditor is responsible for rendering the tab bar.

Tabs are rendered by Oni, but the state is still in that one NeoVim instance, here

What I'd like to do is basically have some special splits for working with editors - an TabbedEditorCollectionSplit that manages a collection of IEditors. You could imagine that the TabbedEditorCollectionSplit could have a bunch of IEditors, and it could take the responsibility of rendering the tab bar, as well as rendering the active editor for the selected the tab.

Here we have more IEditors, one for each tab, and only render the IEditor which is responsible for that tab content. Tab state is now not only rendered by Oni, state is also managed by Oni and not by NeoVim anymore.

The nice thing about this abstraction is that the yode model fits in pretty easily - it would still manage a collection of IEditors, but instead of having a model of tab + only one editor shown, it could show them in whatever layout made sense.

At this point even the splits are managed by Oni, not by NeoVim anymore?
So the implementation of the IEditor interface which enables the user to edit text, what is now NeovimEditor, manages only one "vim window" instead of "tabs + splits + windows (aka splits)" by that time?

For me IWindowSplit is confusing ... does this stand for Oni splits like tab bar, status line, main content area or does it stand for splits which are managed by the NeoVim instance at the moment? Also in combination with WindowManager which manages at the moment one split, the NeovimEditor instance.

Sorry for the wall of text, I hope that made sense! On my end, I'm starting to look at the file explorer + integrated browser window, so I hope as I drill into those it might help clarify some of the unknowns (and have more examples in place).

That would be great, just wanted to ask some big questions to get a picture of that future in my head 😉
For what I can tell by now, the plan for prototyping sounds good to me. Hook me up when we can work on this 😀

@hoschi
Copy link
Author

hoschi commented Nov 11, 2017

(FYI I started to put the stuff from my notes to issues, here is the project which tracks the tasks I work on next https://github.com/hoschi/yode/projects/2)

@hoschi
Copy link
Author

hoschi commented Dec 19, 2017

@bryphe Can you have a look at my questions above?

I hope I can finish the last tasks in the GH project linked above and/or start on the Oni integration in my upcoming holidays 😁

@bryphe
Copy link
Member

bryphe commented Dec 20, 2017

Hi @hoschi !

Sorry I missed your questions, taking a look now.

I hope I can finish the last tasks in the GH project linked above and/or start on the Oni integration in my upcoming holidays 😁

Sounds awesome! 😄

I also really think we need to use this in day to day editing in a "beta phase" and see how things work, because I think it feel really different in some places.

Definitely. We've been pushing on getting features in early, backed by an experimental feature flag, so that we can test them out, as well as ensure they stay up-to-date with other changes on master. We could hook up something like an experimental.yode.enabled that would let us add this as the default for go-to definition, or for adding other commands, like opening up the react component as you mentioned.

With the command palette it is now even easier to recognize these things for new users as both commands show when you type "Go to def" 😁

For sure. It's easy to have multiple commands exposed in the command palette. Ideally, we'd have better discoverability here too by showing when a command is bound to a key.

At the moment there is one IEditor on the screen, NeovimEditor, which renders the "text editing" part like buffers and splits. Splits of the editing space are rendered by NeoVim at the moment, not by Oni.

This is exactly right! Although we're making progress on decoupling this... If you enable the sidebar via experimental.sidebar.enabled, you'll see the sidebar + file explorer:

There is still only one IEditor - the NeovimEditor highlighted in green here.... but the sidebar and file explorer actually have their own Neovim instance - SharedNeovimInstance that they use. So it's a step towards the decoupling.

At this point even the splits are managed by Oni, not by NeoVim anymore?
So the implementation of the IEditor interface which enables the user to edit text, what is now NeovimEditor, manages only one "vim window" instead of "tabs + splits + windows (aka splits)" by that time?

I would actually like to do more split management in Oni, and have the splits fully managed by Oni. Right now, in the file explorer picture, Oni manages the splits between the sidebar/file explorer/NeovimEditor, but inside the neovim editor, vim manages those splits. I would like to split that up and have the ability to have side-by-side or vertically-split NeovimEditor, so that we could have separate tab lines.

For me IWindowSplit is confusing ... does this stand for Oni splits like tab bar, status line, main content area or does it stand for splits which are managed by the NeoVim instance at the moment? Also in combination with WindowManager which manages at the moment one split, the NeovimEditor instance.

The WindowManager and IWindowSplit are managing Oni splits. The status bar is actually just rendered separately, outside of Window Manager. The tab bar is actually rendered as part of the NeovimEditor today, so that isn't a split either.

The IWindowSplit implementations we have today are:

The Neovim splits are managed internally by the NeovimEditor class - it has an instance of NeovimWindowManager which it uses.

The IWindowSplit is basically just a surface that is available to render React components, so there is a lot of flexibility there. We could probably render Yode directly there without too much drama - we'd just have to think about how we'd handle the Neovim piece of the integration.

Some ideas:

  • We could create a bunch of NeovimEditors for each of the yode 'windows' in the split - the downside is each of those spin up a new Neovim process. We would get the language integration and things like that for free, basically.
  • We could use the SharedNeovimInstance, like how the file explorer binds to it to create an instance. However, we don't have language services hooked up there yet.

Just some thoughts off the top of my head... Let me know if you have any other questions 😄

@hoschi
Copy link
Author

hoschi commented Dec 26, 2017

There is still only one IEditor - the NeovimEditor highlighted in green here

@bryphe "here"? Probably forget to upload a screenshot?

I would like to split that up and have the ability to have side-by-side or vertically-split NeovimEditor, so that we could have separate tab lines.

Like Atoms tab-split logic? Never understood what the benefit is compared to vims tab-split logic where tabs act as container and splits used for window management.

we'd just have to think about how we'd handle the Neovim piece of the integration

For the integration prototype I would go with the first one and create a bunch of NeovimEditors, because I find it really unusable without language integration.

The overall picture is clearer now, thanks for the explanations. I think I have enough to start it. I finish the tests for Yode and then tackle the integration. If I get stuck I can work on some Yode tasks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants