vhaudiquet/pkh
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

new: validate wizard defaults like typed answers

Browse Source 
ask_text accepted its default on Enter without running the question's
validator, so a probed upstream version like 1.0-2 or v1.0 sailed
through the whole questionnaire and crashed resolve() at the end, and
an invalid git-derived maintainer default (e.g. 'Name <>') was accepted
verbatim. ask_text now takes the validator and applies it to both typed
answers and the offered default — a default that fails validation is
withheld and an invalid answer re-asks — and all question call sites
(incl. the maintainer loop) route through it.
This commit is contained in:
vhaudiquet
2026-09-17 17:00:22 +02:00
parent 70e375a34d
commit d5b76ec8d8
1 changed files with 171 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/new/questions.rs
+171 -68
View File
@@ -250,7 +250,10 @@ async fn run_wizard(mut cli: NewCli) -> Result<NewOptions, Box<dyn Error>> {
let mut origin = packaged_dir.as_deref().and_then(GitOrigin::detect);
// 4. Upstream version: probed project version, then the tag HEAD sits
// on, then `<lasttag>+git<YYYYMMDD>.<hash>`, then 0.1.0.
// on, then `<lasttag>+git<YYYYMMDD>.<hash>`, then 0.1.0. A raw probe
// that fails `validate_upstream_version` (e.g. `1.0-2` or `v1.0`) is
// not offered as the default — the same validator `resolve` applies
// decides, so Enter can never accept it and crash late.
if cli.upstream_version.is_none() {
let default = probe
.as_ref()
@@ -259,9 +262,8 @@ async fn run_wizard(mut cli: NewCli) -> Result<NewOptions, Box<dyn Error>> {
.or_else(|| origin.as_ref().and_then(GitOrigin::git_version))
.unwrap_or_else(|| "0.1.0".to_string());
let revision = cli.revision.unwrap_or(1);
let answer = ask_text("Upstream version", &default, move |version: &str| {
options::validate_upstream_version(version, revision)
})?;
let validate = move |version: &str| options::validate_upstream_version(version, revision);
let answer = ask_text("Upstream version", &default, validate)?;
cli.upstream_version = Some(answer.clone());
// The typed version names an existing tag HEAD is not on: offer to
@@ -334,31 +336,25 @@ async fn run_wizard(mut cli: NewCli) -> Result<NewOptions, Box<dyn Error>> {
.as_ref()
.and_then(|p| p.description.clone())
.unwrap_or_default();
loop {
let answer = ask_text(
"One-line description",
&default,
required_answer("the description"),
)?;
if !answer.trim().is_empty() {
cli.description = Some(answer);
break;
}
log::warn!("A one-line description is required to scaffold a package");
}
let validate = required_answer("the description");
let answer = ask_text("One-line description", &default, validate)?;
cli.description = Some(answer);
}
// 7. Homepage.
// 7. Homepage (blank skips; a probed default must still be a valid URL).
if cli.homepage.is_none() {
let default = probe
.as_ref()
.and_then(|p| p.homepage.clone())
.unwrap_or_default();
let answer = ask_text(
"Homepage (blank to skip)",
&default,
options::validate_homepage,
)?;
let validate = |url: &str| {
if url.is_empty() {
Ok(())
} else {
options::validate_homepage(url)
}
};
let answer = ask_text("Homepage (blank to skip)", &default, validate)?;
if !answer.is_empty() {
cli.homepage = Some(answer);
}
@@ -378,17 +374,8 @@ async fn run_wizard(mut cli: NewCli) -> Result<NewOptions, Box<dyn Error>> {
options.contains(&answer.to_string())
})?;
if answer == LICENSE_OTHER {
let license = loop {
let candidate = ask_text(
"License (SPDX identifier)",
&custom_default,
required_answer("the license identifier"),
)?;
if !candidate.trim().is_empty() {
break candidate;
}
log::warn!("A license identifier is required when picking the free-text entry");
};
let validate = required_answer("the license identifier");
let license = ask_text("License (SPDX identifier)", &custom_default, validate)?;
cli.license = Some(license);
} else {
cli.license = Some(answer);
@@ -396,43 +383,41 @@ async fn run_wizard(mut cli: NewCli) -> Result<NewOptions, Box<dyn Error>> {
}
// 9. Command name (skipped for the empty template, where nothing is
// installed).
// installed). The probed default must pass the required-answer check
// too, or the question is asked without one.
if cli.command.is_none() && template != TemplateId::Empty {
let default = probe
.as_ref()
.and_then(|p| p.command.clone())
.unwrap_or_else(|| cli.name.clone().unwrap_or_default());
let command = ask_text(
"Command name",
&default,
required_answer("the command name"),
)?;
cli.command = Some(if command.is_empty() {
cli.name.clone().unwrap_or_default()
} else {
command
});
let validate = required_answer("the command name");
let command = ask_text("Command name", &default, validate)?;
cli.command = Some(command);
}
// 10. Maintainer, defaulting to the DEBEMAIL/git-config identity (an
// empty answer re-asks).
// 10. Maintainer, defaulting to the DEBEMAIL/git-config identity. The
// git-derived default goes through `parse_maintainer` like typed
// input: an empty git user.email yields a `Name <>` default that is
// ignored (the question is asked without one) instead of accepted
// verbatim and only failing late in `resolve`. An empty answer
// re-asks.
if cli.maintainer.is_none() {
let default = crate::changelog::get_maintainer_info()
let candidate = crate::changelog::get_maintainer_info()
.map(|(name, email)| format!("{name} <{email}>"))
.unwrap_or_default();
let maintainer = loop {
let answer = ask_text("Maintainer", &default, |answer: &str| {
options::parse_maintainer(answer).map(|_| ())
})?;
if !answer.is_empty() {
break answer;
}
let default = if options::parse_maintainer(&candidate).is_ok() {
candidate
} else {
String::new()
};
if default.is_empty() {
log::warn!(
"Could not determine a maintainer default (no DEBFULLNAME/\
DEBEMAIL and no git user config): answer as 'Name <email>'"
);
};
cli.maintainer = Some(maintainer);
}
let validate = |answer: &str| options::parse_maintainer(answer).map(|_| ());
cli.maintainer = Some(ask_text("Maintainer", &default, validate)?);
}
// 11. Target distribution.
@@ -472,10 +457,11 @@ async fn run_wizard(mut cli: NewCli) -> Result<NewOptions, Box<dyn Error>> {
// 13. Metapackage Depends (empty template only).
if template == TemplateId::Empty && cli.depends.is_empty() {
let validate = |answer: &str| options::validate_depends(answer).map(|_| ());
let answer = ask_text(
"Depends (metapackage, comma-separated, blank for an empty base)",
"",
|answer: &str| options::validate_depends(answer).map(|_| ()),
validate,
)?;
if !answer.trim().is_empty() {
cli.depends = vec![answer];
@@ -799,17 +785,54 @@ fn select_from(
}
}
/// The default a question is offered with: a probed default must clear the
/// same `validate` bar as typed input, so an unusable probe (e.g. an
/// upstream version like `1.0-2` carrying a Debian revision) is withheld —
/// the question is asked without a default instead of offering one that
/// Enter would accept verbatim.
fn offered_default<'a>(default: &'a str, validate: &prompt::Validator) -> &'a str {
if default.is_empty() || validate(default).is_ok() {
default
} else {
""
}
}
/// One round of [`ask_text`]: an empty answer takes the (pre-validated)
/// `default`, and whatever answer is finally proposed — typed or the
/// default — must pass `validate`. `Err` carries the validation error so the
/// caller re-asks with it.
fn accept_answer(
answer: &str,
default: &str,
validate: &prompt::Validator,
) -> Result<String, String> {
let answer = if answer.is_empty() { default } else { answer };
validate(answer).map(|_| answer.to_string())
}
/// One free-text question implementing the spec's "Enter accepts the
/// default": an empty answer falls back to `default` (`Esc` keeps its
/// prompt-level meaning of restoring the default). `validate` only ever
/// sees non-empty answers — the empty one is accepted by the prompt loop so
/// it can take the default path; callers that require an answer re-check
/// the result.
/// default": an empty answer falls back to the default (`Esc` keeps its
/// prompt-level meaning of restoring it). Both the typed answer and the
/// default go through `validate`: a probed default that fails validation is
/// never offered ([`offered_default`]), and an empty answer without a valid
/// default is treated like any other invalid answer (re-ask with the
/// validation error) — probe data can never bypass validation and only blow
/// up later in [`options::resolve`].
fn ask_text(
label: &str,
default: &str,
validate: impl Fn(&str) -> Result<(), String> + 'static,
) -> Result<String, Box<dyn Error>> {
// The offered default must clear the same bar as typed input: an
// unusable probe is withheld and the question is asked without one.
let default = offered_default(default, &validate);
// Enter/Esc (and the non-TTY fallback) return the default without
// validation, so pre-decide what an empty answer yields: the default
// itself (it passed the bar above), or the rejection it shares with any
// invalid answer when no valid default exists. Computed here because
// `validate` moves into the prompt wrapper below.
let empty_answer = accept_answer("", default, &validate);
let accept_empty = move |answer: &str| {
if answer.is_empty() {
Ok(())
@@ -817,12 +840,20 @@ fn ask_text(
validate(answer)
}
};
let answer = prompt::text(label, default, Some(&accept_empty))?;
Ok(if answer.is_empty() {
default.to_string()
} else {
answer
})
loop {
let answer = prompt::text(label, default, Some(&accept_empty))?;
// Typed non-empty answers were already validated by the prompt; only
// an empty one resolves to the default, pre-decided above.
let answer = if answer.is_empty() {
empty_answer.clone()
} else {
Ok(answer)
};
match answer {
Ok(answer) => return Ok(answer),
Err(error) => log::warn!("{error}"),
}
}
}
/// A validator requiring a non-empty answer.
@@ -1404,6 +1435,78 @@ mod tests {
assert!(required_answer("x")("ok").is_ok());
}
/// A probed upstream-version default must clear the same
/// [`options::validate_upstream_version`] bar that `resolve` applies: a
/// raw probe carrying a Debian revision or a `v` prefix fails it, so the
/// wizard withholds it instead of offering it for blind Enter-acceptance
/// and crashing late in `resolve` (regression).
#[test]
fn probed_version_defaults_fail_upstream_validation() {
assert!(options::validate_upstream_version("1.0-2", 1).is_err());
assert!(options::validate_upstream_version("v1.0", 1).is_err());
assert!(options::validate_upstream_version("1.0.0", 1).is_ok());
let validate = |version: &str| options::validate_upstream_version(version, 1);
// Invalid probes are not offered as the default...
assert_eq!(offered_default("1.0-2", &validate), "");
assert_eq!(offered_default("v1.0", &validate), "");
// ...clean probes keep theirs, and no default stays none.
assert_eq!(offered_default("1.0.0", &validate), "1.0.0");
assert_eq!(offered_default("", &validate), "");
}
/// [`accept_answer`] routes typed answers and Enter-taken defaults
/// through the same validation: valid typed input accepted verbatim,
/// invalid typed input rejected (re-ask), an empty answer taking a valid
/// default, and an empty answer with an invalid or missing default
/// rejected like any other invalid answer.
#[test]
fn accept_answer_validates_typed_answers_and_defaults() {
let validate = |answer: &str| {
if answer == "ok" {
Ok(())
} else {
Err(format!("not ok: {answer}"))
}
};
assert_eq!(accept_answer("ok", "ok", &validate), Ok("ok".to_string()));
assert!(accept_answer("bad", "ok", &validate).is_err());
// An empty answer takes the default, which must itself validate: an
// invalid (or missing) default is rejected, not taken.
assert_eq!(accept_answer("", "ok", &validate), Ok("ok".to_string()));
assert!(accept_answer("", "bad", &validate).is_err());
assert!(accept_answer("", "", &validate).is_err());
// The upstream-version scenario end to end: Enter on an invalid
// probed default (offered as none) fails instead of taking it
// verbatim, while a valid default is taken.
let version = |v: &str| options::validate_upstream_version(v, 1);
assert!(accept_answer("", "1.0-2", &version).is_err());
assert_eq!(
accept_answer("", "1.0.0", &version),
Ok("1.0.0".to_string())
);
}
/// The git-derived maintainer default goes through
/// [`options::parse_maintainer`] before it is offered: an empty git
/// email yields a `Name <>` default that parses to an error and is
/// ignored, instead of being accepted verbatim and only failing late in
/// `resolve` (regression).
#[test]
fn maintainer_default_validated_by_parse_maintainer() {
assert!(
options::parse_maintainer("Jane Doe <jane@example.com>").is_ok(),
"a well-formed identity is offered as the default"
);
// Empty git user.email (or DEBEMAIL): `Name <>` must not parse.
assert!(options::parse_maintainer("Jane Doe <>").is_err());
// An empty git user.name: `<>` (or `<email>`) must not parse either.
assert!(options::parse_maintainer(" <jane@example.com>").is_err());
}
#[test]
fn select_labels_carry_their_own_separator() {
// prompt::select renders `> <label><answer>` verbatim; a label