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

Default values for string Input prompt are not passed to validators #84

Closed
agc93 opened this issue Nov 14, 2020 · 9 comments · Fixed by #85
Closed

Default values for string Input prompt are not passed to validators #84

agc93 opened this issue Nov 14, 2020 · 9 comments · Fixed by #85
Assignees
Labels
bug Something isn't working

Comments

@agc93
Copy link

agc93 commented Nov 14, 2020

If a defaultValue is specified in a Prompt.Input<string> call that also specifies validators, pressing Enter on the prompt sends an empty string to the validators, not the default value. For example:

var userName = Sharprompt.Prompt.Input<string>("Please enter a name", "default", validators: new[] { Sharprompt.Validations.Validators.Required()});

will fail unless providing a non-default value, since an empty string is passed to validators, rather than the default value specified by defaultValue.


I've created an oversimplified repro in this repo

@shibayan shibayan added the bug Something isn't working label Nov 26, 2020
@shibayan shibayan self-assigned this Nov 26, 2020
@shibayan shibayan added this to the v2.1.0 Release milestone Nov 26, 2020
@shibayan
Copy link
Owner

Thanks for the report. This is a bug and I will fix it.

@shibayan
Copy link
Owner

Fixed v2.1.0-preview2

@jods4
Copy link

jods4 commented Dec 7, 2020

@shibayan this doesn't impact only the string Input.
For example the Confirm input clearly shows a default N value in UI:

Are you sure? (y/N)

But if you proceed with hitting ENTER the validator says a value is required.

@shibayan
Copy link
Owner

shibayan commented Dec 7, 2020

@jods4
Thanks for the feedback. In Confirm, the "(y/N)" display is now fixed value, so that's how it appears.
https://github.com/shibayan/Sharprompt/blob/master/Sharprompt/Forms/Confirm.cs#L66

Are the following mappings commonly used? I'm not familiar with it so please let me know if you know.

  • (y/N) = default value is false
  • (Y/n) = default value is true
  • (y/n) = not default value (required input)

@jods4
Copy link

jods4 commented Dec 7, 2020

@shibayan For me (y/N) is a good english default for a confirm.
My gripe is that when I hit ENTER it doesn't work because the default is not passed to the validator, as for the text. input

@shibayan
Copy link
Owner

shibayan commented Dec 7, 2020

There are no validators in Confirm. It is required, but can be omitted if a default value is explicitly specified (i.e., it passes by typing the Enter key).

So the behavior is correct, it's just that the message displayed is not as common as it should be.

@jods4
Copy link

jods4 commented Dec 7, 2020

It doesn't work for us, so I guess it's because the "implicit" default value is not taken into account -- or maybe the default message is just wrong given there's no default?

Displaying (y/N) is good because it shows that y or n is expected. Uppercase N means it's the default value if you just hit enter.

Are you saying that Confirm shows (y/N) although there is no default value?
So if I just explicitely add default: false it will work as intended?

@shibayan
Copy link
Owner

shibayan commented Dec 7, 2020

Are you saying that Confirm shows (y/N) although there is no default value?

Yes

So if I just explicitely add default: false it will work as intended?

Yes

@shibayan
Copy link
Owner

shibayan commented Dec 7, 2020

The message display will be corrected in #87.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants