gcv / julia-snail

An Emacs development environment for Julia
GNU General Public License v3.0
231 stars 21 forks source link

Get completion working in org babel src blocks #87

Closed MasonProtter closed 2 years ago

MasonProtter commented 2 years ago

This implements the idea I suggested in https://github.com/gcv/julia-snail/pull/86#issuecomment-1100917903, it turned out to be pretty easy.

One problem remains though, and after bashing my head against the wall, I still can't figure out the problem so I was wondering if you have any suggestions: julia-snail-org-interaction-mode is not automatically turning on inside org-mode buffers like it should. Rather, one still needs to explicitly do M-x julia-snail-org-interaction-mode but after that it appears to work great.

MasonProtter commented 2 years ago

One problem remains though, and after bashing my head against the wall, I still can't figure out the problem so I was wondering if you have any suggestions: julia-snail-org-interaction-mode is not automatically turning on inside org-mode buffers like it should. Rather, one still needs to explicitly do M-x julia-snail-org-interaction-mode but after that it appears to work great.

Okay, I figured it out. The problem was that the extensions only get loaded once you actually do M-x julia-snail, so my minor mode was only working if I activated julia-snail before visiting a .org file. Tricky tricky.

MasonProtter commented 2 years ago

Here's a demo of it in action https://user-images.githubusercontent.com/29157027/163737867-113a899a-b083-485f-a288-93d341dc7b32.mp4

MasonProtter commented 2 years ago

Thanks for taking the time to review all this.

More seriously, I'm guessing julia-snail--module-at-point will break in an org file if the src block has something like this:

#+begin_src julia :module Mod1
module Mod2
# point set here when (julia-snail--module-at-point) is called
end
#+end_src

The module should be Mod1.Mod2.

To fix this problem, julia-snail--module-at-point needs to call julia-snail--cst-module-at, but the buffer in the invocation will have to be a temporary one containing the src block code. Then you combine the results with the module variable from the src block.

Okay, I think I have addressed this with my latest revision, though I will note that I'm not sure this can ever matter, since the repl completion won't be able to complete any names within Mod2 anyways until after that src block is actually run. It's the same reason that completion doesn't know about local variables in examples like

let some_name = 1
    some_<TAB>
end

Nonetheless, it's good to do this right in case we ever do get smarter completion.


To restrict the org-mode specific implmentation of julia-snail--module-at-point to ob-julia.el... Well, there is unfortunately no particularly good way to do it. Try making :around advice for julia-snail--module-at-point, and activate that advice the first time the ob-julia extension is turned on. The advice code would be in ob-julia.el, and it would in effect do the major mode dispatch you're thinking of and provide the org-mode implementation, but keep it out of julia-snail.el. It would never be deactivated. It's (a little?) ugly, but should work.

I instead made julia-snail-repl-completion-at-point take an optional argument module-finder which is meant to be a function for finding the current module. The default is the julia-snail--module-at-point function.


julia-snail-org-interaction-mode can be moved into ob-julia.el (and renamed to follow the namespace convention). There's already an example of a minor mode being provided in an extension, see repl-history.el.

I have not yet done this because I don't know how to make my minor mode get set up at require time rather than once the user actaully runs M-x julia-snail. So far as I can tell, this is inherent to the way you've set up extensions (but I'm no expert so maybe I'm missing something obvious). So I guess I could alternatively make a separate package which depends on julia-snail and does it's own custom stuff at require time and then hooks into the extension interface.

gcv commented 2 years ago

Those changes look really good. You mentioned this problem where the minor mode doesn't get initialized early enough before, and I don't quite understand it. What does the minor mode need to do before the user runs M-x julia-snail?

MasonProtter commented 2 years ago

It needs to be initialized so that the hooks to org mode are set up and run.

gcv commented 2 years ago

Ah, I think I see. Bit of a chicken-and-egg problem. Let me think about it.

gcv commented 2 years ago

I got it. Use the julia-snail/ob-julia-init function to register the necessary org-mode-hook. Then iterate over all open org-mode buffers and force the addition of the completion-at-point function in each one. It's probably a good idea to only do this initialization once (put a sentinel defvar in ob-julia.el and setf it inside the init function).

Does that make sense and seem like it'll work?

Also, let's see what happens if we get rid of the after-revert-hook.

MasonProtter commented 2 years ago

I got it. Use the julia-snail/ob-julia-init function to register the necessary org-mode-hook. Then iterate over all open org-mode buffers and force the addition of the completion-at-point function in each one. It's probably a good idea to only do this initialization once (put a sentinel defvar in ob-julia.el and setf it inside the init function).

Yeah I guess that's possible, but it feels kinda wrong in the sense that it makes extensions feel very second class that their elisp code doesn't get loaded until very late, after all the usual setup stuff has already completed. I'll give your suggestion a try later today and see how it goes.

gcv commented 2 years ago

True. I intended for it to be possible for extensions to be individually enabled on a per-project basis. It seems that I went too far, and that it should be possible to make loading them nicer.

MasonProtter commented 2 years ago

Maybe we could have two classes of extension files -- ones that get loaded during initialization, and ones that get loaded when you start up julia-snail? That way I can put things like relevant minor mode hooks in the initialization files, and then everything else into the other file?

Maybe that's a needless over-complication though

gcv commented 2 years ago

I also wanted a way to avoid wrangling the load-path (or making the user do it) and preserve lazy-loading. But there's probably a better way to do it.

gcv commented 2 years ago

We could also make ob-julia its own top level thing, with autoloads and other Emacs packaging niceties, rather than an extension.

MasonProtter commented 2 years ago

Yep, that's certainly possible. For now, lets see if we can keep it as an extension, but as it develops it might start to make more sense to split it out into it's own package that simply depends on julia-snail.

MasonProtter commented 2 years ago

Okay @gcv, I've moved the minor mode into the extensions folder as you suggested and added a thing to map over all the current buffers and activate the interaction mode in any org mode buffers. I've also removed that revert-hook stuff.

gcv commented 2 years ago

I just tried it and love it. Thank you!