-
Notifications
You must be signed in to change notification settings - Fork 280
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
[wip] improvement: autocomplete functions #2883
base: main
Are you sure you want to change the base?
[wip] improvement: autocomplete functions #2883
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I see there is an issue with some code_completion requests not resolving. It ends up that the autocomplete feels flaky.
export const AUTOCOMPLETER = new DeferredRequestRegistry<
Omit<CodeCompletionRequest, "id">,
CompletionResultMessage | null
>(
"autocomplete-result",
async (requestId, req) => {
await sendCodeCompletionRequest({
id: requestId,
...req,
});
},
// We don't care about previous requests
// so we just resolve them with an empty response.
// { resolveExistingRequests: () => null }, // for example, commenting this out improves it. Existing reqs are handled
); Playing around with this code, I can see the autocomplete not working 100% of the time. Still looking into why. fig = px.bar(x=["a", "b", "c"], y=[1, 3, 2])
fig.upda |
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.
Thanks for exploring this — our completion could definitely use improvements. Left a comment.
@@ -60,7 +60,6 @@ export const Autocompleter = { | |||
info: () => constructCompletionInfoNode(option.completion_info), | |||
}; | |||
}), | |||
validFor: /^\w*$/, |
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 concerned about the performance hit from this.
Instead of deleting, can we fix this regex? From looking at the video in your issue, I wonder if /^\w+$/
would be better.
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.
thanks @akshayka , you are right about the performance hit.
I've been a little stuck because any regex doesn't work well with other libraries, eg polars df
df = pl.DataFrame({
"a": [1,2]
})
df.clo# <---- cursor here doesn't autocomplete
I don't know why I'm so stuck unfortunately 🥲, anyways i modified this regex for a minor improvement.
if (tooltip) { | ||
return null; | ||
} |
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.
another thing @akshayka , this removal introduces some side effects. It's not ideal.
df = pl.DataFrame({
"a": [1,2],# <--- cursor here will pop up an autocomplete for Dataframe which overrides Enter key
})
I am keen to revert this, an ideal solution is to display as just a tooltip, it would take me too long to implement imo :/. (unless you have some pointers, getting pretty lost haha)
📝 Summary
Fixes #2877
def abc(# <---- when the cursor is here
🔍 Description of Changes
There are still some odd / inconsistent behaviours, so this fix isn't foolproof.
📋 Checklist
📜 Reviewers
@akshayka OR @mscolnick