-
Notifications
You must be signed in to change notification settings - Fork 122
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
fix: Fix aborting a form returning a timeout error #287
Conversation
Also added some tests for timeout, aborts and this issue.
@maaslalani Are you sure you don't have a timeout set? As the PR mentions, this only occurs when Looks like tests are failing. I'll look into it when I'm back. |
Sorry you're totally right, I was on an incorrect branch! |
I've applied the changes you requested and removed the I've fixed the data race as well and all tests now pass with |
@maaslalani Would you be able to look into this soon? |
@Sculas This looks great! Thank you! |
….mod (#60) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [github.com/charmbracelet/huh](https://togithub.com/charmbracelet/huh) | `v0.4.2` -> `v0.5.1` | [![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fcharmbracelet%2fhuh/v0.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fcharmbracelet%2fhuh/v0.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fcharmbracelet%2fhuh/v0.4.2/v0.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fcharmbracelet%2fhuh/v0.4.2/v0.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>charmbracelet/huh (github.com/charmbracelet/huh)</summary> ### [`v0.5.1`](https://togithub.com/charmbracelet/huh/compare/v0.5.0...v0.5.1) [Compare Source](https://togithub.com/charmbracelet/huh/compare/v0.5.0...v0.5.1) ### [`v0.5.0`](https://togithub.com/charmbracelet/huh/releases/tag/v0.5.0) [Compare Source](https://togithub.com/charmbracelet/huh/compare/v0.4.2...v0.5.0) ### Dynamic Forms 🪄 <img width="600" src="https://vhs.charm.sh/vhs-6FRmBjNi2aiRb4INPXwIjo.gif" alt="Country / State form with dynamic inputs running."> `huh?` forms can now react to changes in other parts of the form. Replace properties such as `Options`, `Title`, `Description` with their dynamic counterparts: `OptionsFunc`, `TitleFunc`, and `DescriptionFunc` to recompute properties values on changes when watched variables change. Let’s build a simple state / province picker. ```go var country string var state string ``` The `country` select will be static, we’ll use this value to recompute the options and title for the next input. ```go huh.NewSelect[string](). Options(huh.NewOptions("United States", "Canada", "Mexico")...). Value(&country). Title("Country"). ``` Define your `Select` with `TitleFunc` and `OptionsFunc` and bind them to the `&country` value from the previous field. Whenever the user chooses a different country, the `TitleFunc` and `OptionsFunc` will be recomputed. > \[!IMPORTANT] > We have to pass `&country` as the binding to recompute the function only when > `country` changes, otherwise we will hit the API too often. ```go huh.NewSelect[string](). Value(&state). Height(8). TitleFunc(func() string { switch country { case "United States": return "State" case "Canada": return "Province" default: return "Territory" } }, &country). OptionsFunc(func() []huh.Option[string] { opts := fetchStatesForCountry(country) return huh.NewOptions(opts...) }, &country), ``` Lastly, run the `form` with these inputs. ```go err := form.Run() if err != nil { log.Fatal(err) } ``` ##### Other Changes #### What's Changed - Form Layouts by [@​adamdottv](https://togithub.com/adamdottv) in [https://github.com/charmbracelet/huh/pull/274](https://togithub.com/charmbracelet/huh/pull/274) - Resolve conflict between select and filter by [@​MikaelFangel](https://togithub.com/MikaelFangel) in [https://github.com/charmbracelet/huh/pull/252](https://togithub.com/charmbracelet/huh/pull/252) - Add `CursorText` style by [@​nervo](https://togithub.com/nervo) in [https://github.com/charmbracelet/huh/pull/259](https://togithub.com/charmbracelet/huh/pull/259) - Adjust input width to char limit by [@​nervo](https://togithub.com/nervo) in [https://github.com/charmbracelet/huh/pull/260](https://togithub.com/charmbracelet/huh/pull/260) - Implement `WithInput` by [@​Delta456](https://togithub.com/Delta456) in [https://github.com/charmbracelet/huh/pull/271](https://togithub.com/charmbracelet/huh/pull/271) - Implement WithTimeout by [@​Delta456](https://togithub.com/Delta456) in [https://github.com/charmbracelet/huh/pull/276](https://togithub.com/charmbracelet/huh/pull/276) - Set filtering state of select by [@​PJGaetan](https://togithub.com/PJGaetan) in [https://github.com/charmbracelet/huh/pull/179](https://togithub.com/charmbracelet/huh/pull/179) - `Filtering` on `Select` and `MultiSelect` fields by [@​maaslalani](https://togithub.com/maaslalani) in [https://github.com/charmbracelet/huh/pull/283](https://togithub.com/charmbracelet/huh/pull/283) - Introduce `accessor` by [@​nervo](https://togithub.com/nervo) in [https://github.com/charmbracelet/huh/pull/263](https://togithub.com/charmbracelet/huh/pull/263) - fix: aborting a form returning a timeout error by [@​Sculas](https://togithub.com/Sculas) in [https://github.com/charmbracelet/huh/pull/287](https://togithub.com/charmbracelet/huh/pull/287) #### New Contributors - [@​MikaelFangel](https://togithub.com/MikaelFangel) made their first contribution in [https://github.com/charmbracelet/huh/pull/252](https://togithub.com/charmbracelet/huh/pull/252) - [@​nervo](https://togithub.com/nervo) made their first contribution in [https://github.com/charmbracelet/huh/pull/253](https://togithub.com/charmbracelet/huh/pull/253) - [@​shedyfreak](https://togithub.com/shedyfreak) made their first contribution in [https://github.com/charmbracelet/huh/pull/230](https://togithub.com/charmbracelet/huh/pull/230) - [@​Delta456](https://togithub.com/Delta456) made their first contribution in [https://github.com/charmbracelet/huh/pull/271](https://togithub.com/charmbracelet/huh/pull/271) - [@​abradley2](https://togithub.com/abradley2) made their first contribution in [https://github.com/charmbracelet/huh/pull/280](https://togithub.com/charmbracelet/huh/pull/280) - [@​PJGaetan](https://togithub.com/PJGaetan) made their first contribution in [https://github.com/charmbracelet/huh/pull/179](https://togithub.com/charmbracelet/huh/pull/179) - [@​Sculas](https://togithub.com/Sculas) made their first contribution in [https://github.com/charmbracelet/huh/pull/287](https://togithub.com/charmbracelet/huh/pull/287) **Full Changelog**: charmbracelet/huh@v0.4.2...v0.5.0 *** <a href="https://charm.sh/"><img alt="The Charm logo" src="https://stuff.charm.sh/charm-badge.jpg" width="400"></a> Thoughts? Questions? We love hearing from you. Feel free to reach out on [Twitter](https://twitter.com/charmcli), [The Fediverse](https://mastodon.technology/@​charm), or [Slack](https://charm.sh/slack). </details> --- ### Configuration 📅 **Schedule**: Branch creation - "* */8 * * *" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/jippi/dottie). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MjUuMSIsInVwZGF0ZWRJblZlciI6IjM3LjQyNS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJkZXBlbmRlbmNpZXMiXX0=--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This PR fixes an issue caused by #276 where aborting a form would return
ErrTimeout
instead, because whenf.timeout == 0
,time.Since(startTime) >= f.timeout
is always true. This was discussed here: #276 (comment)This is fixed by simply checking if the error returned by
bubbletea
isErrProgramKilled
, which is already handled bytea.WithContext(ctx)
, making the (erroneous) check unnecessary anyway.I've also added tests for handling timeouts, user aborts, and making sure
ErrUserAborted
is returned instead ofErrTimeout
when a timeout error occurs (the issue mentioned above).This required an extra fieldprogram
for testing to hold thetea.Program
to send messages to theForm
(batchUpdate
cannot be used here because it's managed bytea.Program
).RunWithContext(context.Context)
has been added to support this.