Skip to content
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

Add completions to Common Lisp client #517

Merged
merged 2 commits into from
Jul 28, 2023
Merged

Conversation

russtoku
Copy link
Contributor

I hope this helps a little to resolve issue #466.

Some issues:

  • The candidates are identified as TEXT. I haven't figured out how to at least annotate them as symbols or something that would indicate that they are from from Swank.
  • The candidate list returned from Swank is based the first letter typed. 'lo' and 'la' would share the same list of candidates.
  • If you trigger omnicompletion after typing say 'lo', you'll get an error message but the completion pop-up window is not affected.

Suggestions welcome.

@LordMZTE
Copy link

This is awesome! Apart from what you stated already, I have 3 more criticizms:

  • The Conjure output window is spammed a little during completion. It'd be great if output from completions wasn't sent to it.
  • An error occurs when completing a package, for example triggering a completion here: uiop:| (where | represents the cursor) causes this error to appear:
Error executing vim.schedule lua callback: ...ker/opt/conjure/lua/conjure/client/common-lisp/swank.lua:173: too many results to unpack
stack traceback:
	[C]: in function 'unpack'
	...ker/opt/conjure/lua/conjure/client/common-lisp/swank.lua:173: in function 'dispatch'
	...ker/opt/conjure/lua/conjure/client/common-lisp/swank.lua:213: in function 'parse_separated_list'
	...ker/opt/conjure/lua/conjure/client/common-lisp/swank.lua:232: in function 'parse_result'
	...ker/opt/conjure/lua/conjure/client/common-lisp/swank.lua:249: in function ''
	vim/_editor.lua: in function <vim/_editor.lua:0>
  • This may be a limitation with swank, but currently, completions are shown which won't work due to being declared in another package. For example, your code would suggest run-program whereas that function needs to be refered to by uiop:run-program.

Thanks for you great work! This is really good already!

@Olical
Copy link
Owner

Olical commented Jul 28, 2023

Tried it locally and I got some completions! I had to trigger omnicomplete though with <c-x><c-o> which I found odd.

@Olical
Copy link
Owner

Olical commented Jul 28, 2023

A fun error if I trigger omnicompletion with no text on the line:
image

I think there's an unpack somewhere in our lua and it's being asked to unpack too many values. I didn't know there was a limit so The More You Know! I'll see if I can find it.

@Olical Olical merged commit d75f12f into Olical:develop Jul 28, 2023
1 check passed
@Olical
Copy link
Owner

Olical commented Jul 28, 2023

I've replaced the unpack in question with (str.join (a.map string.char stack)) and it can now handle any size. So we should avoid using unpack for unbounded tables or at least tables we think could get very large. Merged to develop!

@russtoku
Copy link
Contributor Author

Yes, that unpack was the cause of the error. Thanks for that fix.
I'm still trying to work through some of the short-comings mentioned but I'm glad we've gotten thing this far.

@russtoku russtoku deleted the dev-466 branch July 28, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants