dccsillag / magma-nvim

Interact with Jupyter from NeoVim.
GNU General Public License v3.0
977 stars 49 forks source link

Add option to share Magma between buffers #106

Open phcerdan opened 11 months ago

phcerdan commented 11 months ago

Initialize magma as usual: :MagmaInit or :MagmaInit kernel_name Change buffer in neovim and use shared option:

:MagmaInit kernel_name shared

TODO: Improve argument parsing for :MagmaInit

Now, the same kernel is shared between two buffers

Fix #105

phcerdan commented 11 months ago

This is POC for #105 It works, but requires: TODO:

benlubas commented 10 months ago

Improve argument parsing for MagmaInit

@phcerdan What do you mean by this? I'd love to have this feature as well, so I'm happy to put some work in to get it over the finish line. Although it will probably end up just living in my fork instead of getting into this upstream at the moment

phcerdan commented 10 months ago

Probably your fork will become the main, @WhiteBlackGoose doesn't seem to have to the time?

This PR is working, but I requires some love on how to pass the shared option to the :MagmaInit command.

MagmaInit right now allows 0 or 1 arguments. This PR introduces that if there are at least 2 arguments, :MagmaInit workenv foo boo this_is_shared the existing kernel (workenv) is shared in the current buffer.

    @pynvim.command("MagmaInit", nargs="*", sync=True, complete='file')  # type: ignore
    @nvimui  # type: ignore
    def command_init(self, args: List[str]) -> None:
        self._initialize_if_necessary()

        if args:
            args = args[0].split(" ")
            kernel_name = args[0]
            shared = False
            if len(args) > 1:
                shared = True
            print(args)

That argument logic has to be improved. That is the only missing part from WIP to ready IMO.

benlubas commented 10 months ago

Unfortunately this PR doesn't handle io.py,

It's like a 4 line change if we don't expand the functionality to allow loading into the same kernel. I haven't spent much time looking into loading into a shared kernel. I consider that a nice to have, but something that's not necessary.

:eyes: it looks like it might be really easy to do

I regret saying that

benlubas commented 10 months ago

Preliminary PR is up: https://github.com/benlubas/molten-nvim/pull/4

I got save and load to a point where it seems like they would work, but I must have broken save/load at some point along the way b/c it just doesn't work in my version of the plugin at all (i have saving to a file, and reading from a file working without error, but trying to open a cell after it's been loaded produces output that says it's loading, and it just stays like that), and just confirmed that's how magma behaves as well

None-the-less, @phcerdan, if you're able to test that branch that'd be awesome. Don't feel obligated, I'm going to be using it as well, so hopefully it can be merged.

Update, it's merged, and Molten is now stable