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

enable auto-completion #588

Merged
merged 6 commits into from Jun 12, 2023
Merged

Conversation

roipoussiere
Copy link
Contributor

@roipoussiere roipoussiere commented Jun 9, 2023

Now uiwjs/react-codemirror#458 is merged, the auto-completion seems to work pretty well, so maybe we can enable it?
follows #427, fix #426

@roipoussiere
Copy link
Contributor Author

update: css fix to avoid this glitch:

image

@yaxu
Copy link
Member

yaxu commented Jun 9, 2023

It would be good to have an option for this, personally I'd prefer it default off.

@felixroos
Copy link
Collaborator

thanks for reviving the autocomplete :) besides the now merged PR (uiwjs/react-codemirror#458), there is one more thing still not done, as mentioned in #427 :

todo: be context aware, show methods only after a dot, show top level functions only on top level etc..
see https://codemirror.net/examples/autocompletion/ (Completing from Syntax)

without this, the autocomplete is pretty dumb, as it suggests any name at any place, so many things that get suggested don't actually work, for example:

s("bd sd").s

this will suggest samples, which is not a pattern method, so it will throw an error. (I haven't tested this specific case but I remember that it did things like that).

maybe it makes sense to add this before merging? it could be a bit tricky to decide how to decide which goes what, and how to annotate it.
but we could also add it as an opt-in experimental feature as it is now..

@roipoussiere
Copy link
Contributor Author

roipoussiere commented Jun 9, 2023

but we could also add it as an opt-in experimental feature as it is now..

Yep, I'm adding the button in the settings, so I can disable the feature by default until the problem you mention is not solved.

@roipoussiere
Copy link
Contributor Author

It would be good to have an option for this, personally I'd prefer it default off.

Done

@felixroos
Copy link
Collaborator

I'll merge this now with the current state, although it's currently sometimes suggesting things that are wrong, it's not on by default and already has value as-is.

@felixroos felixroos merged commit 665cf6a into tidalcycles:main Jun 12, 2023
1 check passed
@roipoussiere roipoussiere deleted the enable_autocomplete branch June 12, 2023 23:02
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.

repl: autocomplete
3 participants