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

Support suggestions and autocompletion in textinput #407

Merged
merged 10 commits into from Aug 21, 2023

Conversation

toadle
Copy link
Contributor

@toadle toadle commented Aug 14, 2023

Continuation of discussion from here: #140

@toadle
Copy link
Contributor Author

toadle commented Aug 14, 2023

@maaslalani In my implementation here: #140 there was an id in the Model of textinput to check if we complete the right textinput. That is now completely gone from the model. Do we still need that?

@maaslalani
Copy link
Member

This is awesome @toadle! Thank you so much!

@toadle
Copy link
Contributor Author

toadle commented Aug 15, 2023

@maaslalani You think we can get this merged?

There was one thing that I was still thinking about: If there are several suggestions populated

  • "Hello World"
  • "Hello You"
  • "Foo bar"

and you currently have "Hello" typed in, you can switch through all three. Ending up in the last situation where there is nothing to complete when having "Foo bar" selected, because "Hello" can not be completed to "Foo bar".

Should the autocomplete work in a way where the input also looks for the next possible suggestion if the current one can not be used to complete? Effectively switching the current selected suggestion.

The otherwise to do it would be to always switch the possible suggestions externally.

I kind of have the feeling that it should cause it'll be more useful that way. But I also would like to get this merged ;-)

@toadle
Copy link
Contributor Author

toadle commented Aug 15, 2023

Anyway, forget my last comment. I just fixed it here: 2d16f32

Should be good to go. I hope we can merge this, so that it doesn't get stale again :-)

@toadle
Copy link
Contributor Author

toadle commented Aug 15, 2023

Little preview:

CleanShot 2023-08-16 at 00 02 06

@meowgorithm
Copy link
Member

@toadle so what you're saying is that you're building a bubble tea integration called toadle-copilot, right?

@toadle
Copy link
Contributor Author

toadle commented Aug 16, 2023

@meowgorithm Unsure if pun or not :-)?!

@meowgorithm
Copy link
Member

@toadle Totally joking around. This looks awesome!

@toadle
Copy link
Contributor Author

toadle commented Aug 18, 2023

@maaslalani What do you say?

@maaslalani
Copy link
Member

Going to try and get this merged today! Thanks so much for all your work @toadle.

@toadle
Copy link
Contributor Author

toadle commented Aug 18, 2023

@maaslalani I saw that you removed the Msg to set suggestions in the recent commit. Please remember that suggestions most likely will get stemmet asynchronously by the result of another cmd. We discussed that over on the other PR. Would you still set them via a func-call then?

@maaslalani
Copy link
Member

@maaslalani I saw that you removed the Msg to set suggestions in the recent commit. Please remember that suggestions most likely will get stemmet asynchronously by the result of another cmd. We discussed that over on the other PR. Would you still set them via a func-call then?

Yep, I will add the asynchronous setting back since many people may get their autocompletion data from an API or external source. But we should definitely also allow setting the suggestions statically since a lot of people may already have the values beforehand when creating the textinput bubble.

@maaslalani
Copy link
Member

Hey @toadle looks like this PR will need some adjustments to get double width runes working: I can handle that soon!

image

@toadle
Copy link
Contributor Author

toadle commented Aug 18, 2023

@maaslalani OK, that double-width rune thing is weird. I tried to fix that myself just now but always end up with the same problem. I'll also have a look if I can fix that.

I pushed a little other change where I expose a function that can help customize to complete behaviour (e.g. if you want to complete just up to the next "space" or "-"). I think it'd be good to have that - I can think of several use-cases.

also: accepting suggestions does not overwrite the already given input, rather appends
@toadle
Copy link
Contributor Author

toadle commented Aug 19, 2023

@maaslalani Finally understood how []rune vs. string works in Go. Refactored everything to work with []rune - much better now. Should work fine with "charm热爱开源" and sorts now.

@maaslalani
Copy link
Member

Amazing @toadle! I will take a look and merge this soon 🎉
Thanks again for all your work!

@maaslalani
Copy link
Member

@toadle after doing a bit of testing it's definitely possible to use the SetSuggestions stuff with custom commands on the developer end. So I say we merge this as is.

For example, a user can do the following:

type setSuggestionsMsg []string

func loadSuggestions() tea.Msg {
	time.Sleep(5 * time.Second) // Fetch from a data source.
	return setSuggestionsMsg(pokemon[0])
}

// ... Update method.
	case setSuggestionsMsg:
		m.textInput.SetSuggestions(msg)

Since the user can implement a basic Cmd for asynchronous data / completion fetching I think this is enough to enable this functionality for now. If necessary / requested we can add a first class API for async completion but I think the current way is more customizable for the developer.

@maaslalani
Copy link
Member

Also tested with double width runes and everything is working great!

@toadle
Copy link
Contributor Author

toadle commented Aug 21, 2023

@maaslalani If that works too i think it is also fine to have it work like this. In my tests it somehow did not work, but my implementation already differs quite substantially from the one here. So the error might be in my side.

@maaslalani maaslalani merged commit eda8912 into charmbracelet:master Aug 21, 2023
9 checks passed
@toadle toadle deleted the autocomplete branch August 21, 2023 20:56
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.

None yet

3 participants