Skip to content
This repository has been archived by the owner on Sep 24, 2023. It is now read-only.

Fix multiple validator inconsistency #401


Copy link

@fatihdumanli fatihdumanli commented Jan 23, 2022

Steps to reproduce:

  1. Run the program below
  2. Enter a string with a length shorter than 3 when prompted. You should get Sorry, your reply was invalid: value is too short. Min length is 3. Nothing surprising so far.
  3. Now enter a string that consists of more than 20 characters. You'll see that it passes the validation.

Expected behavior: A string with more than 20 characters shouldn't satisfy the conditions to be a valid answer.

        qCityName := survey.Question{
                Name: "cityname",
                Prompt: &survey.Input{
                        Message: "Enter a city name",
                Validate: func(ans interface{}) error {
                        str, _ := ans.(string)
                        if len(str) > 20 {
                                return errors.New("Max length is 20")
                        return nil

        var cityName string
        survey.Ask([]*survey.Question{&qCityName}, &cityName, survey.WithValidator(survey.MinLength(3)))

This PR introduces a new change that rolls back the validation flow to the beginning as soon as we encounter a validation error. The current code is taking it from where it left off, ignoring the previous validators.

@fatihdumanli fatihdumanli changed the title Fatihdumanli/bugfix/fix on multiple validators Fix multiple validator inconsistency Jan 23, 2022
Copy link

@mislav mislav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, great catch and thank you for the fix!

I found both the previous approach and your revised version a bit tricky to follow, since it resulted in a lot of nested loops and changing of iteration index mid-iteration. I've pushed a more substantial change that streamlines the whole prompt + validate loop to ensure re-prompt as long as validators don't pass. I've also added a unit test. Please take a look!

@mislav mislav merged commit 099a968 into AlecAivazis:master Mar 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

2 participants