You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Describe the bug
TextPrompt.Validate() does not chain with itself. Only the last call to Validate() will run. Any other validators are ignored. This is clear in TextPromptExtensions where obj.Validator is simply replaced each time Validate() is called.
To Reproduce
Chain multiple Validate() calls together. Only the last call will run, regardless of any previous calls.
Expected behavior
If Validate() is used multiple times, each Validator should run. Otherwise, comments should be revised to make it clear that only 1 call to Validate() is allowed.
Additional context
XML comments in TextPromptExtensions indicate that Validate() should chain, and a comment in TextPrompt says "Run all validators". To me, this indicates that multiple validators are allowed and should work as expected.
Multiple Validate() calls would be useful to separate validation logic, and especially to tailor validation errors more specifically. With a single call, multiple checks all have to run at once and the error message must suggest all possible errors.
Semantics are debatable: should validation stop on the first failure, or run all validators and present all failures? Could be configurable.
I am willing to implement this and open a PR if this is a desirable change.
Please upvote 👍 this issue if you are interested in it.
The text was updated successfully, but these errors were encountered:
// regex & IsValidDateString() are defined elsewhere
var StartingDate = AnsiConsole.Prompt(
new TextPrompt<string>("Enter starting date as MMDD, MMDDHH, MMDDHHmm, or yyyyMMDDHHmm)):")
.Validate(s => true, "Early true")
.Validate(s => false, "Early false")
.Validate(s => regex.IsMatch(s), "Date string is unparseable")
.Validate(s => true, "Middle true")
.Validate(s => false, "Middle false")
.Validate(s => IsValidDateString(s, out startingDateActual), "Date string represents an invalid date")
.Validate(s => true, "Late true")
.Validate(s => false, "Late false")
);
The above will always reject any input with error message "Late false". Everything else will be ignored. Removing the very last Validate() will always return Success regardless of the input string. Removing the last 2 Validate() will return the expected result and error message (if appropriate) from IsValidDateString().
I was able to work around it like so, but this is not desirable because the error message has to report all possible problems rather than a single one:
var StartingDate = AnsiConsole.Prompt(
new TextPrompt<string>("Enter starting date as MMDD, MMDDHH, MMDDHHmm, or yyyyMMDDHHmm)):")
.Validate(s => regex.IsMatch(s) && IsValidDateString(s, out startingDateActual),
"Date is unparseable or represents an invalid date")
);
Information
Describe the bug
TextPrompt.Validate() does not chain with itself. Only the last call to Validate() will run. Any other validators are ignored. This is clear in TextPromptExtensions where obj.Validator is simply replaced each time Validate() is called.
To Reproduce
Chain multiple Validate() calls together. Only the last call will run, regardless of any previous calls.
Expected behavior
If Validate() is used multiple times, each Validator should run. Otherwise, comments should be revised to make it clear that only 1 call to Validate() is allowed.
Additional context
XML comments in TextPromptExtensions indicate that Validate() should chain, and a comment in TextPrompt says "Run all validators". To me, this indicates that multiple validators are allowed and should work as expected.
Multiple Validate() calls would be useful to separate validation logic, and especially to tailor validation errors more specifically. With a single call, multiple checks all have to run at once and the error message must suggest all possible errors.
Semantics are debatable: should validation stop on the first failure, or run all validators and present all failures? Could be configurable.
I am willing to implement this and open a PR if this is a desirable change.
Please upvote 👍 this issue if you are interested in it.
The text was updated successfully, but these errors were encountered: