mattwparas / steel

An embedded scheme interpreter in Rust
Apache License 2.0
980 stars 46 forks source link

Integration with Helix #225

Closed Mawfyy closed 2 weeks ago

Mawfyy commented 3 weeks ago

Hello, I appreciate your work with Steel and as a Plugin system!

Testing the plugin system (great work btw) I founded a misunderstanding about how to integrate this with Helix.

In STEEL.md (https://github.com/mattwparas/helix) you mentioned using the master branch on steel, but in the master branch it has a conflict with steel-core dependency

Helix fork: image

Steel: image

mattwparas commented 3 weeks ago

Thanks for opening this, I'll take a look and update things accordingly

mattwparas commented 3 weeks ago

Looks like there is an issue with the master branch of steel at the moment that is causing some things to not work correctly, once I can figure it out I'll make sure that the instructions are lined up with the latest work

Mawfyy commented 3 weeks ago

Thanks, I appreciate your time and effort!

mattwparas commented 3 weeks ago

Looks like #217 introduced a regression, I'm investigating

@jrvidal FYI - something strange where we're losing things that are multi arity across module boundaries.

jrvidal commented 3 weeks ago

I might have time to take a look into it today. Any other details about it?

mattwparas commented 3 weeks ago

Trying to reproduce as minimally as possible, within helix it is much harder to debug. I'll update here once I get somewhere

mattwparas commented 3 weeks ago

For the sake of clarity, if you have a working build of steel + helix on the latest master, trying to use any of the functions from helix/commands.scm seems to fail. All of them are generated to be multi arity, like so:

(provide theme)

;;@doc
;;Change the editor theme (show current theme if no name specified).
(define (theme . args)
  (helix.theme *helix.cx* args))

But attempting to use those by importing like so:

(require (prefix-in hx. "helix/commands.scm"))

(hx.theme "boo_berry")

raises an error due to the arguments not getting wrapped in a list. (multi-arity hx.theme) returns #f, so it seems somewhere along the line, we're losing the fact that it should accept multiple arguments.

I'm still trying to reproduce outside of helix

jrvidal commented 3 weeks ago

I think I found it, it's something in the parsing stage.

jrvidal commented 3 weeks ago

OK, no, I was wrong. It's something about macro expansion. The improper: true field is lost after this call. So it seems macro expansion matters more that I initially thought :disappointed:

FWIW I think reverting is totally fine until we're more confident that the change is sound.

jrvidal commented 3 weeks ago

More concretely, the problem is with the @doc macro :facepalm:

EDIT: here's a branch that reverts improper list support in case we want to unblock helix integration.

mattwparas commented 3 weeks ago

Ah good catch! Looks like I need some more tests around that.

I'll see if I can plumb the improper field through, or quickly add support before reverting

mattwparas commented 3 weeks ago

Also - thanks for investigating! Getting it down to the macro expansion line is super helpful, I'm looking over the code to see how hard it would be to just add support for improper lists there now. Realistically I don't think its too hard, but famous last words of course :smile:

mattwparas commented 3 weeks ago

Everything should be fixed now as per #226 - I've also updated the instructions for helix. Check the branch steel-event-system on my fork of helix, the toml has been updated

Mawfyy commented 2 weeks ago

Thanks for your time and effort!

I get other bug related with keybindings module...

image

mattwparas commented 2 weeks ago

Sheesh, sorry about that, I forgot to update those as well.

In the mean time while I update those, this is going to be the most concrete source of truth: https://github.com/mattwparas/helix-config/blob/master/helix.scm

That repo has my entire helix config, which is pretty much guaranteed to work since I daily drive off of it

Mawfyy commented 2 weeks ago

Okay, i gonna check it, thanks!

mattwparas commented 2 weeks ago

Just updated the instructions, although I haven't tested them extensively, I just more or less copied over some minimal examples from my helix config directly

Mawfyy commented 2 weeks ago

I tried copy your configuration to .config/helix and i did execute cargo xtask code-gen and i get this image

mattwparas commented 2 weeks ago

So in helix.scm just comment out these lines:

(require "term.scm")

(provide open-term
         new-term
         kill-active-terminal
         switch-term
         term-resize
         xplr
         open-debug-window
         close-debug-window)
Mawfyy commented 2 weeks ago

It works, thank you so much!

Also, i would like to update the documentation ^^