Skip to content

Fix several completion bugs#48

Open
technomancy wants to merge 4 commits intosanel:masterfrom
technomancy:completion-fixes
Open

Fix several completion bugs#48
technomancy wants to merge 4 commits intosanel:masterfrom
technomancy:completion-fixes

Conversation

@technomancy
Copy link
Contributor

While debugging a new version of Jeejah, I encountered several bugs in Monroe's completion logic.

  • The completions request was missing the session field, which all messages should have.
  • The monroe-completion-at-point function was using cdadr to extract the "candidate" field instead of actually looking for the completion by name. This probably worked on whatever server it was originally tested with, but it would fail as soon as the response had a different number of fields from that.
  • The add-hook call for monroe-completion-at-point was not correctly making the hook changes buffer-local.

However, even after fixing these bugs, I've been unable to get completion working. I've confirmed that responses come back over nrepl like such:

{:completions [{:candidate "tail!" :type "unknown"} {:candidate "table" :type "unknown"}]
 :id "1"
 :ns ">"
 :session "459523637"
 :status ["done"]}

but Monroe still does not correctly show or insert candidates returned by the server. Not sure what's going on there.

All requests should have this field; completion in jeejah breaks
without it.
Previously this was mistakenly using cdadr to extract the completion
candidates, which would only work by accident if the response message
did not include any different fields than expected.
This was passing in 'local in the wrong argument, so it did not have
the desired effect.
@sanel
Copy link
Owner

sanel commented Nov 18, 2025

Can you try pls with [1] version? I've merged couple of patches before, mostly related to multiple connection support and weird things happens from time to time. I'm loosing content with the last master (no evaluations) and it's a bit hard to debug, so I was planning to revert stuff to [1] and apply other patches done in the mean time. Thoughts?

[1] 508f5ed

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.

2 participants