search

AlecAivazis / survey

A golang library for building interactive and accessible prompts with full support for windows and posix terminals.
MIT License
4.09k stars 352 forks source link

Fix regression in PR #301. #322

Closed infalmo closed 3 years ago

infalmo commented 3 years ago

Fix #319

infalmo commented 3 years ago

Regarding #320, I feel running t.Parallel() should be done in each test function, rather than the RunTest function. However, if there is a strong reason to NOT do the same, I can fix the issue by wrapping each test in TestOnInterruptFunc inside another t.Run().

infalmo commented 3 years ago

Also, isn't the minor version supposed to be incremented, rather than the patch (I believe you're using semver) ?

AlecAivazis commented 3 years ago

I reverted the other PR and published onto the v2.2.x branch in order to correct version issue. So at the very least we will need to bring the original changes into this PR.

That being said, I'm not sure the change makes a lot of sense. Thinking about what @mislav said, i agree that this change is pretty unnecessary given that the caller can just look to see if the returned error is an interrupt? Especially considering that the error is returned either way, i can't really see the weight of an addition to the top-level API being worth it.

I'm inclined to close this PR but I want to give you a change to explain if I am missing something.

infalmo commented 3 years ago

See https://github.com/AlecAivazis/survey/pull/323#issuecomment-748615810

Also, if top level API's are a problem, I could remove the top level API and leave only the Question stuct level API.

infalmo commented 3 years ago

This PR is being closed in correspondence to https://github.com/AlecAivazis/survey/pull/323#issuecomment-749516182.