codecompanion: extension support #1115
Conversation
midischwarz12
requested review from
NotAShelf,
Soliprem and
horriblename
as code owners
| extensions = mkOption { | ||
| type = nullOr luaInline; | ||
| default = null; | ||
| description = "Extensions for codecompanion"; |
There was a problem hiding this comment.
This will be converted to nil by default. Does code-companion really support this?
There was a problem hiding this comment.
@NotAShelf I'm not 100% sure how nvf is turning these options into lua code. I couldn't find where it traverses these and generates lua. But I kind of assumed that if it was null, it wouldn't generate the luaInLine at all. I think that's what happens with the adapter options.
There was a problem hiding this comment.
I did test this with the extensions set. And it seemed to work. I probably should test this with the extensions unset as well though.
There was a problem hiding this comment.
midischwarz12
force-pushed
the
codecompanion-extensions
branch
from
0dee230 to
92d5f7b
Compare
|
Ah, okay. This is ready. But there is a merge conflict with the release notes. So I'll have to go resolve this real quick. |
midischwarz12
force-pushed
the
codecompanion-extensions
branch
from
92d5f7b to
dcde353
Compare
|
@NotAShelf Ready to merge |
| ''; | ||
| }; | ||
|
|
||
| extensions = mkOption { |
There was a problem hiding this comment.
I believe you have Fundamentally Misunderstood how setupOpts is supposed to work. Normally, the setupOpts attribute set ({}) converts to a Lua table. In the documentation section you can find how Nix primitives are converted to Lua.
In this case,adding extensions is not strictly necessary; the setupOpts table is freeform and the user can already pass
extensions = { /* nix here */}and it would be converted to the Lua expression. With type = luaInline, howevr, the user can ONLY pass inline Lua values, which defeats the purpose of using nvf. The type should be attrsOf anything, and to justify the addition of an option I'd like you to
- Add an example in
mkOption - Better utilize description.
Otherwise there is no need for this PR.
There was a problem hiding this comment.
I don't think I'm misunderstanding. I read through the docs you sent when you sent them previously. But from what I remember, there were quite a few call backs for these plugins and made more sense to just have inline lua.
I'll poke around more and look into doing this without the explicit inline extension option.
|
|
||
| [midischwarz12](https://github.com/midischwarz12): | ||
|
|
||
| - Add extension support for [codecompanion-nvim]. |
There was a problem hiding this comment.
"Add extension support is" misleading; we already hid it, it was just implicit. Better describe the change per my comment in the Nix section.
There was a problem hiding this comment.
What do you mean by "we already hid it"?
There was a problem hiding this comment.
Maybe "explicit codecompanion-nvim extension option" would be more accurate then?
3 participants
Sanity Checking
nix fmt).#nix(default package).#maximal.#docs-html(manual, must build).#docs-linkcheck(optional, please build if adding links)x86_64-linuxaarch64-linuxx86_64-darwinaarch64-darwinAdd a 👍 reaction to pull requests you find important.