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

feature suggestion: make callbacks configurable #280

Merged
merged 3 commits into from
Jun 14, 2024

Conversation

abradley2
Copy link
Contributor

I'm using huh to make some internal tooling with neat little TUI's.

In this case, we have suite of tools that are their own individual huh forms. These can be used as top level forms, but I've also set things up in a way where forms can compose other forms that are otherwise top level. This uses a pattern commonly referred to as Nested Tea. Some of our tools are sometimes the top level form, other times they're forms within a form and this pattern makes them super composable.

It's a bit tricky, to compose them in this way, however. It often involves a line that looks sort of like this:

	if m.subFormFoo != nil && m.subFormFoo.State == huh.StateCompleted {
		cmd = tea.Batch(cmd, getData(m))
		m.subFormFoo = nil
	}
       
	switch curmsg := msg.(type) {
	case getDataSuccess:
		m.subFormBar = NewFormBar()
                cmd = tea.Batch(cmd, subFormBar.Init())

The tricky part is if-statements in the control flow of update. Missing simple statements like m.subFormFoo = nil can cause a bad loop. When callbacks are configurable things become much easier and everything can stay in the msg/update pattern:

	switch curmsg := msg.(type) {
        case subFormFooCompeted:
                cmd = tea.Batch(cmd, getData(m))
	case getDataSuccess:
		m.subFormBar = NewFormBar()
                cmd = tea.Batch(cmd, subFormBar.Init())
        case subFormBarCompleted:
               // more control flow etc,

Initializing a new form in this way is pretty straight forward

func NewFormBar(m Model) *huh.Form {
	var form *huh.Form
	form = huh.NewForm(
		// cool huh stuff here
	)
	form.CancelCmd = func() tea.Msg {
		return subFormBarCancelled{}
	}
	form.SubmitCmd = func() tea.Msg {
		return subFormBarCompleted{}
	}
	return form
}

Notice that in these cases we aren't using form.Run() to run a form because it's undesirable to have it call tea.Quit when that form is submitted.

For this reason it isn't necessary to use the RunWithCallbacks function I added here. That would be used for top level forms where we want to stay in the "TEA mode" when the form is exited.

@abradley2
Copy link
Contributor Author

abradley2 commented Jun 14, 2024

I plan on making a larger example for this during the weekend once I'm free from work, but curious to know what people think now. I'm on the fence about RunWithCallbacks being necessary for what I'm trying to do here, though I still think it'd be helpfull to have the callback fields exposed either way

@maaslalani
Copy link
Member

I plan on making a larger example for this during the weekend once I'm free from work, but curious to know what people think now. I'm on the fence about RunWithCallbacks being necessary for what I'm trying to do here, though I still think it'd be helpfull to have the callback fields exposed either way

I like exposing the SubmitCmd and CancelCmd. I think we don't need RunWithCallbacks personally. So we should take it out, if we end up having a compelling use case we can add it in another PR.

@abradley2
Copy link
Contributor Author

I plan on making a larger example for this during the weekend once I'm free from work, but curious to know what people think now. I'm on the fence about RunWithCallbacks being necessary for what I'm trying to do here, though I still think it'd be helpfull to have the callback fields exposed either way

I like exposing the SubmitCmd and CancelCmd. I think we don't need RunWithCallbacks personally. So we should take it out, if we end up having a compelling use case we can add it in another PR.

I think you're right. As of now I'm no longer using RunWithCallbacks, so I can't think of a concrete use-case. I've removed it

@maaslalani maaslalani merged commit d7db017 into charmbracelet:main Jun 14, 2024
20 checks passed
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

2 participants