-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Add Steel as an optional plugin system #8675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
I've been using this Steel + Helix integration for a bit. My config is here: https://github.com/zetashift/helix-config The main downside here imho is that Helix isn't as great for Lisps(or Scheme in this case?) as Emacs for example. But besides that this worked nicely. I have no real complaints and I actually did not need to script so many things as with my VSCode or NeoVim configs because a lot just works ™️ . |
I think we can definitely improve, this is just the first prototype after all. You can't do everything at once. For example, I want to fully revamp the config system in the future (and remove the toml based config) so that is works even better with scheme |
Oh! I'm very sorry if I came over as demanding. I meant more like, in my experience this was what I stumbled upon, and it's not even a permanent thing, because a LSP for Steel might be in the works as well. I completely agree that you can't do everything at once, nor should somebody be obliged to do it. And maybe some lower hanging fruit can be picked up by people like me. For example if the tree-sitter grammar for scheme can improve things for Steel than that's another avenue. |
No worries I didn't read it as negative. I ws more trying to highlight that this is just an initial prototype that can & will Improve (although Mathew already did a great job!) |
This is something that people with more familiarity with tree sitter and indents could be able to answer - are indent queries capable of matching a proper lisp indent mode? I did spend some time trying to mimic it but without much success |
rust-toolchain.toml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Github says the ending newline is missing in this file.
I don't know Scheme / Lisp but it looks like the indentation should not be too difficult to model with tree-sitter indent queries (just using (list) @indent
; Align lists to the second element (except if the list starts with `define`)
(list . (symbol) @first . (_) @anchor
(#not-eq? @first "define")
(#set! "scope" "tail")) @align In order to improve this, I'd need to understand in which cases lists are aligned to the first, in which to the second element and when they are not aligned at all. Is this documented somewhere? If I find the time, I'll also try to understand the scheme indent implementation you wrote. Now that the plugin system is making progress, it might finally be time for me to learn the language 😄 |
This is great, I really need to learn more about tree sitter idents! I used this as a reference to get started: https://github.com/ds26gte/scmindent#how-subforms-are-indented Note, my implementation is not complete and was mostly an exercise in 1. seeing if it could be done and 2. getting something far enough to make editing scheme code more pleasant, I would not verbatim use it as a canonical reference. |
Thanks a lot for the reference @mattwparas. I created a PR (#8720) with indent queries that should cover everything from it except for some simplifications regarding keywords. Feel free to try it out and report any issues it has (you can just copy the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
already left some notes but haven't got trough everything.
I think we should be mindful of not exposing too many implementation details and not tying the API too closely to how helix is currently implemented (as that would make evolution harder and may feel clunky).
In the longterm I would like a lot more focus on configuration but to get a good steel API for that we probably need to revamp our config system internally first so that its not tied to closely to toml/serde (And that there is a separate config of a mutable buffer config) so it may be better to do that in future PRs.
Ideally I would like something like the following to work eventually:
(config
(theme "everforest")
(soft-wrap
enable #true
wrap-at-text-width #true)
(text-width 85)
(keymap "normal"
("=" write)
("j" my_custom_function)))
helix-term/src/commands/engine.rs
Outdated
@@ -0,0 +1,265 @@ | |||
use helix_core::syntax::Configuration; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think the name engine is a bit ambiguous. Could we call it something like script_engine
to make it clearer what the file actually does
} | ||
|
||
// Attempt to fetch the keymap for the extension | ||
fn get_keymap_for_extension<'a>(cx: &'a mut Context) -> Option<SteelVal> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is too specical cased IMO.
While nice for a proof of concept the editor should have a generic concept of per buffer keymaps and then plugins should just interface with that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would agree, it requires a little hacking from the steel side to make it work as you can tell. Do you think it is worth investing in that machinery to expose that from the editor side now?
} | ||
} | ||
|
||
thread_local! { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how suitable threadlocals really are.
I think tokio can and does move our main loop to different threads (and I also don't want to constrain interaction with plugins to a single thread).
In the worst case these could be globals but for almost all of these it feels like its really adding more extensability to the editor here instead of in the editor itself.
I think we should instead take the approach of making the core editor more extensible and then exposing those APIs to plugins.
These APIs would ofcourse require access to somekind of context. This would be similar to the context problem you mentinoed in your description. I think the best way to handle that one would be if we stored the context as a global inside the engine (using run_with_reference).
In any callbacks that want access to the context we check for that global (even user code could do it themselves right?) and throw an error if a function that needs a context is called without context access.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm currently refactoring the implementation to instead just from the steel side of things, have one variable that refers to the context. Whenever we call steel code, we'll do like you said - just refresh the context variable with the current context for the duration of the steel code scope, otherwise it'll be some poisoned value that will error.
With respect to thread locals - I'm really not sure how else to implement the interaction with the steel engine if not through a thread local, and any callbacks that I set up are done on a thread local queue that tokio is processing.
Can you elaborate on:
I also don't want to constrain interaction with plugins to a single thread
Does this mean you want the engine to be utilizing multiple threads? Or you want the main loop to be able to be moved to different threads (or both)?
As it stands, the engine implementation is not thread safe, which would make this rather tricky to implement. I don't know if tokio is actively moving the main loop to another thread or not, as I haven't run into any strange behavior with the interaction between the editor and the engine up to this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To put it another way - if we didn't want to have the main thread interact with the scripting engine, then it would have to take the context by value for this to work (at least, that is what I'm currently thinking)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If tokio lets you hold the engine struct across await points, I think the main loop only ever runs on a single thread. I would like to be able to eventually make Engine
Send
so we can send it across thread. I could definitely imagine that we would like to run some plugin code in the background (or in response to some async event).
Requiring engine to be Sync
won't work since you would need to pub mutexes everywhere which would be excessively slow. But Send could be reasonable so we can have Mutex<Engine>
and share that across threads. That is usually a reasonably easy requirement. The main thing you can't do is expose something like Rc
to the user. I know you use refcounting internally (so Send
is currently not implemented) but it could be safe to unsafe impl Send for Engine
assuming that you never hand any refcounted types out to rust (or if you do make sure to use Arc
or a non-clonable reference).
I think thread_locals are quite alright for the Context
since the context would be explicitly provided everytime we call into the Engine
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that the current moment, I've added some code to have the whole editor panic if the engine gets used on another thread to see if it is happening. I have been using this as a daily driver for a few weeks like this and haven't yet run into it. So while I agree thread local isn't the best, it also seems to be acting as intended, which I think is just by coincidence
module.register_fn("editor-cursor", Editor::cursor); | ||
|
||
module.register_fn("cx->cursor", |cx: &mut Context| cx.editor.cursor()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is an instance of exposing too many implementation details.
As I said in the other comment, context should be somekind of hidden global (which can have different capabilities depending on callsite).
The user should only know that he can do X/Y at a certain callsite. the existance of context and editor struct is an implementation detail that plugins should not be aware of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So just to clarify - don't expose the editor struct at all, any details go directly through the context?
I'm fine with that, just confirming the intent here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah don't expose theses structs at all. These should not be editor-cursor
and cx->cursor
but instead just active-cursor
and the internal structs are handled in the background.
|
||
// TODO: | ||
// Position related functions. These probably should be defined alongside the actual impl for Custom in the core crate | ||
module.register_fn("Position::new", helix_core::Position::new); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are the naming conventions in steel? These look like rust paths to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There aren't any explicit naming conventions, although in general I prefer kebab case. At some points I tend to adopt Rust naming conventions in order to make interaction with the system more obvious. This is certainly open for discussion and I don't have any strong opinions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I am not sure how I fell about using rust naming conventions. I think syntactically it feels nice to stay a bit with conventions typically used in scheme/lisp. For example racket conventions could work well: https://docs.racket-lang.org/style/Textual_Matters.html
I think in this case the name new
is not needed at all and we could just call it (position row col)
following the naming conventions for constructors in racket (https://docs.racket-lang.org/reference/define-struct.html)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that works for me!
This comment was marked as off-topic.
This comment was marked as off-topic.
this PR is not intended to discuss the choice of plugin language. We already made our choice after lengthy discussion. This PR is only intended for reviewing and discussing this particular implementation. I will mark such comments offtopic |
As an aside question, I recall that we were talking about sandboxing on Matrix a while back. Is this still planned/implemented here? |
Not fully, right now there are no restrictions what a VM van do but I talked about this a while ago with Mathew and it should be possible to add capability based security to plygins in the future. This would entail a one VM per plugin approach (and how to handle interop in that case) but those should be solvable problems (the same problems would have occurred with any sandboxing including wasm) |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
…mplementation in the future
* Add send-lsp-notification Steel binding - Add send_custom_notification method to LSP Client for arbitrary notifications - Implement send_arbitrary_lsp_notification Steel function following existing patterns - Register Steel binding with comprehensive documentation and examples - Enable fire-and-forget LSP notifications from Steel scripts - Supports arbitrary method names and parameters like textDocument/didShowCompletion - Follows same error handling and language server discovery as send-lsp-command Enables usage like: (send-lsp-notification "copilot-ls" "textDocument/didShowCompletion" (hash "item" (hash "insertText" "suggestion"))) * Fix send-lsp-notification implementation
Very excited, great stuff Paras ❤️ |
…ly update the default features for helix-term to include steel
Hi, just wanted to share a small plugin that I was personnaly missing helix-discord-rpc which just allows you to show that you are Thank you mattwparas for your work and support |
if we're adding to the plugin list, I made modeline.hx to support basic emacs and vim modelines. Currently requires a custom branch that allows regex matching on ropes but presumably it could be retrofitted to use a dylib for regex instead. Thanks again mattwparas for your hard work! Super exciting to see this PR coming to a close |
I'm trying to make "Goto definition" decompile a java class (like IntelliJ or nvim-jdtls does):
It took me a few hours to find out that the client needs to set [language-server.jdtls.config]
extendedClientCapabilities.classFileContentsSupport = true This allows the server to respond with something like: {
"jsonrpc": "2.0",
"id": 2,
"result": [
{
"uri": "jdt://contents/java.base/java.util/ArrayList.class?=karls-languages/%5C/opt%5C/homebrew%5C/Cellar%5C/openjdk%5C@21%5C/21.0.7%5C/libexec%5C/openjdk.jdk%5C/Contents%5C/Home%5C/lib%5C/jrt-fs.jar%60java.base=/javadoc_location=/https:%5C/%5C/docs.oracle.com%5C/en%5C/java%5C/javase%5C/21%5C/docs%5C/api%5C/=/%3Cjava.util(ArrayList.class",
"range": {
"start": {
"line": 167,
"character": 11
},
"end": {
"line": 167,
"character": 20
}
}
}
]
} As you can see, the URL scheme is
So my question is: is it possible to intercept the server's response using a plugin? If so, we could parse the Thanks, |
To add to the plugin list, i started working on a c# plugin. This is still very barebones though. |
Sharing hx-tmux-navigator might be helpful for anyone using tmux+helix. It allows you to use the same keybindings to navigate within and between helix splits and tmux panes. |
Curious: where should we discussed "wishlist" etc for plugins? For example, I would love to have a plugin that lets me integrate ClaudeCode and helix (like what exists between eg VSC and helix, i.e. make CC aware of which file I am in, where I am in the file, what my selection is etc). Should there be an issue / discussion for this, or should we just discuss it here (but this may clutter?)? :) |
I think there is steel discord somewhere 🤔. |
you can also write an LSP server thats a wrapper around jdtls that forwards messages between it and the client, except for GotoDefinition which it intercepts and updates before forwarding a response |
I thought that I would share this after working on it a bit more, but I haven't found any time to work on it for a while, so I'll share it now. It's a plugin called select-project.hx, and it allows you to set helix's working directory. @mattwparas, I was wondering how you would close the current picker and open the file picker when the user selects a project. |
Notes:
Opening this just to track progress on the effort and gather some feedback. There is still work to be done but I would like to gather some opinions on the direction before I continue more.
You can see my currently functioning helix config here and there are instructions listed in the
STEEL.md
file. The main repo for steel lives here, however much documentation is in works and will be added soon.The bulk of the implementation lies in the
engine.rs
andscheme.rs
files.Design
Given prior conversation about developing a custom language implementation, I attempted to make the integration with Steel as agnostic of the engine as possible to keep that door open.
The interface I ended up with (which is subject to change and would love feedback on) is the following:
If you can implement this, the engine should be able to be embedded within Helix. On top of that, I believe what I have allows the coexistence of multiple scripting engines, with a built in priority for resolving commands / configurations / etc.
As a result, Steel here is entirely optional and also remains completely backwards compatible with the existing toml configuration. Steel is just another layer on the existing configuration chain, and as such will be applied last. This applies to both the
config.toml
and thelanguages.toml
. Keybindings can be defined via Steel as well, and these can be buffer specific, language specific, or global. Themes can also be defined from Steel code and enabled, although this is not as rigorously tested and is a relatively recent addition. Otherwise, I have been using this as my daily driver to develop for the last few months.I opted for a two tiered approach, centered around a handful of design ideas that I'd like feedback on:
The first, there is a
init.scm
and ahelix.scm
file - thehelix.scm
module is where you define any commands that you would like to use at all. Any function exposed via that module is eligible to be used as a typed command or via a keybinding. For example:This would then make the command
:shell
available, and it will just replace the%
with the current file. The documentation listed in the@doc
doc comment will also pop up explaining what the command does:Once the
helix.scm
module isrequire
'd - then theinit.scm
file is run. One thing to note is that thehelix.scm
module does not have direct access to a running helix context. It must act entirely stateless of anything related to the helix context object. Runninginit.scm
gives access to a helix object, currently defined as*helix.cx*
. This is something I'm not sure I particularly love, as it makes async function calls a bit odd - I think it might make more sense to make the helix context just a global inside of a module. This would also save the hassle that every function exposed has to accept acx
parameter - this ends up with a great deal of boilerplate that I don't love. Consider the following:Every function call to helix built ins requires passing in the
cx
object - I think just having them be able to reference the global behind the scenes would make this a bit ergonomic. The integration with the helix runtime would make sure whether that variable actually points to a legal context, since we pass this in via reference, so it is only alive for the duration of the call to the engine.Async functions
Steel has support for async functions, and has successfully been integrated with the tokio runtime used within helix, however it requires constructing manually the callback function yourself, rather than elegantly being able to use something like
await
. More to come on this, since the eventual design will depend on the decision to use a local context variable vs a global one.Built in functions
The basic built in functions are first all of the function that are typed and static - i.e. everything here:
However, these functions don't return values so aren't particularly useful for anything but their side effects to the editor state. As a result, I've taken the liberty of defining functions as I've needed/wanted them. Some care will need to be decided what those functions actually exposed are.
Examples
Here are some examples of plugins that I have developed using Steel:
File tree
Source can be found here
filetree.webm
Recent file picker
Source can be found here
recent-files.webm
This persists your recent files between sessions.
Scheme indent
Since steel is a scheme, there is a relatively okay scheme indent mode that only applied on
.scm
files, which can be found here. The implementation requires a little love, but worked enough for me to use helix to write scheme code 😄Terminal emulator
I did manage to whip up a terminal emulator, however paused the development of it while focusing on other things. When I get it back into working shape, I will post a video of it here. I am not sure what the status is with respect to a built in terminal emulator, but the one I got working did not attempt to do complete emulation, but rather just maintained a shell to interact with non-interactively (e.g. don't try to launch helix in it, you'll have a bad time 😄 )
Steel as a choice for a language
I understand that there is skepticism around something like Steel, however I have been working diligently on improving it. My current projects include shoring up the documentation, and working on an LSP for it to make development easier - but I will do that in parallel with maintaining this PR. If Steel is not chosen and a different language is picked, in theory the API I've exposed should do the trick at least with matching the implementation behavior that I've outlined here.
Pure rust plugins
As part of this, I spent some time trying to expose a C ABI from helix to do rust to rust plugins directly in helix without a scripting engine, with little success. Steel supports loading dylibs over a stable abi (will link to documentation once I've written it). I used this to develop the proof of concept terminal emulator. So, you might not be a huge fan of scheme code, but in theory you can write mostly Rust and use Steel as glue if you'd like - you would just be limited to the abi compatible types.
System compatibility
I develop off of Linux and Mac - but have not tested on windows. I have access to a windows system, and will get around to testing on that when the time comes.