diff --git a/Cargo.lock b/Cargo.lock index 44528cf..00458ee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1200,7 +1200,7 @@ dependencies = [ [[package]] name = "rona" -version = "2.33.0" +version = "2.34.0" dependencies = [ "arboard", "assert_cmd", diff --git a/Cargo.toml b/Cargo.toml index 50d59b2..5808172 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ license = "Apache-2.0 OR MIT" name = "rona" readme = "README.md" repository = "https://github.com/rona-rs/rona" -version = "2.33.0" +version = "2.34.0" [[bin]] doc = true diff --git a/README.md b/README.md index 2f14857..02e4e0c 100644 --- a/README.md +++ b/README.md @@ -474,12 +474,14 @@ This replaces the need for a separate tool when a project requires additional st | `kind` | Prefetch result | Behaviour | | -------- | ----------------------------- | ------------------------------------------------------------ | -| `select` | non-empty list | Select from list + `(none)` (if optional) + `Other (enter manually)` | +| `select` | non-empty list | Fuzzy picker over the candidates, plus `(none)` when the field is optional. Typing filters the list and offers the typed text as a new value, so a value the list does not hold is one Enter away. | | `select` | empty | Falls back to a free-text prompt | | `text` | non-empty list from `command` | Same as `select` with non-empty list | | `text` | 0–1 values from `branch` | Free-text prompt with the extracted value as the default | | `text` | nothing | Plain free-text prompt | +**Entering a value the list does not hold**: the candidates are suggestions, not a closed set. Type the value you want; the list filters as you type, and as soon as nothing matches, a `Create "..."` row takes its place. Pressing Enter accepts it, so a new scope costs the same keystrokes as an existing one. Arrow keys still walk the list, Esc cancels the prompt. + When a field is skipped (optional + user chose `(none)`), the variable is simply absent. Use a conditional block in your template to handle this cleanly: `{?scope}({scope}){/scope}`. #### Prompt order @@ -544,7 +546,6 @@ $ Select scope > api auth (none) - Other (enter manually) $ Ticket reference (PROJ-42) > PROJ-42 @@ -575,7 +576,7 @@ feat: Add login endpoint [PROJ-42] #### Example: static select options (no prefetch) -If you just want a fixed list without any prefetching, omit the `prefetch` block and list `kind = "select"` — but note that without prefetch, an empty candidate list causes the prompt to fall back to a free-text input. For a true fixed list, provide the options via `prefetch.command` using a shell command like `echo`: +If you just want a fixed list without any prefetching, omit the `prefetch` block and list `kind = "select"` — but note that without prefetch, an empty candidate list causes the prompt to fall back to a free-text input. Provide the options via `prefetch.command` using a shell command like `echo`, and add a `validation` regex when only those options may be accepted, since the picker otherwise takes any value the user types: ```toml [[extra_fields]] diff --git a/src/commands/generate.rs b/src/commands/generate.rs index 41eb081..e1bf09b 100644 --- a/src/commands/generate.rs +++ b/src/commands/generate.rs @@ -199,8 +199,14 @@ fn build_commit_message( let template = commit_template(config); - // Validate the template, including any extra field variable names. - let extra_names: Vec<&str> = extra_values.keys().map(String::as_str).collect(); + // Validate the template against every declared extra field, not just the answered ones: a + // skipped optional field is absent from `extra_values`, but its `{?field}` block stays valid. + let extra_names: Vec<&str> = config + .project_config + .commit_extra_fields + .iter() + .map(|field| field.name.as_str()) + .collect(); if let Err(e) = validate_template(template, &extra_names) { println!( "{} Template validation error: {e}", @@ -317,6 +323,33 @@ mod tests { Ok(()) } + /// REGRESSION TEST: skipping an optional extra field used to invalidate the whole template, + /// because validation only knew the fields that were answered, and the message fell back to + /// the built-in format. A declared field keeps its `{?field}` block valid either way. + #[test] + fn test_a_skipped_extra_field_keeps_the_template_valid() -> TestResult { + let template = "{commit_type}{?scope}({scope}){/scope}: {message}"; + + validate_template(template, &["scope"])?; + + let variables = TemplateVariables { + commit_number: None, + commit_type: "feat".to_string(), + branch_name: "main".to_string(), + message: "Add login".to_string(), + date: "2024-01-15".to_string(), + time: "14:30:00".to_string(), + author: "Test User".to_string(), + email: "test@example.com".to_string(), + }; + + // The user skipped the scope, so it is absent from the values. + let result = process_template(template, &variables, &HashMap::new())?; + + assert_eq!(result, "feat: Add login"); + Ok(()) + } + #[test] fn test_fallback_message_without_a_commit_number() { let message = fallback_message(None, Some("fix"), "bugfix", "Fix issue"); diff --git a/src/extra_fields.rs b/src/extra_fields.rs index c865074..3995ab1 100644 --- a/src/extra_fields.rs +++ b/src/extra_fields.rs @@ -5,7 +5,7 @@ use std::collections::{HashMap, HashSet}; -use dialoguer::{FuzzySelect, Input}; +use dialoguer::Input; use regex::Regex; use serde::{Deserialize, Serialize}; @@ -21,7 +21,7 @@ pub enum FieldKind { /// Free-form text input. #[default] Text, - /// Selection from a list, with an "Other (enter manually)" fallback. + /// Fuzzy pick from the candidate list, or a value typed on the spot. Select, } @@ -157,9 +157,6 @@ fn extract_matches<'a>( results } -const NONE_OPTION: &str = "(none)"; -const OTHER_OPTION: &str = "Other (enter manually)"; - /// Prompt the user for an extra field value. /// /// Returns `None` when the field is optional and the user chose to skip it. @@ -202,7 +199,7 @@ pub fn prompt_extra_field(field: &ExtraField) -> Result> { )); if use_select { - prompt_as_select(field, prompt_text, candidates, validator_regex) + prompt_as_select(field, prompt_text, &candidates, validator_regex) } else { // Branch prefetch: the single extracted value becomes the text default let default_owned = candidates.into_iter().next(); @@ -215,31 +212,35 @@ pub fn prompt_extra_field(field: &ExtraField) -> Result> { } } +/// Offers the prefetched candidates without closing the door on a value they do not hold: the +/// picker filters the list as the user types and turns unmatched text into a new value, so adding +/// a scope costs no more keystrokes than reusing one. fn prompt_as_select( field: &ExtraField, prompt_text: &str, - candidates: Vec, + candidates: &[String], validator_regex: Option, ) -> Result> { - let mut options = candidates; - if !field.required { - options.push(NONE_OPTION.to_string()); - } - options.push(OTHER_OPTION.to_string()); - - let index = FuzzySelect::with_theme(&crate::theme::prompt_theme()) - .with_prompt(prompt_text) - .items(&options) - .default(0) - .interact_opt() - .map_err(|_| RonaError::UserCancelled)? - .ok_or(RonaError::UserCancelled)?; - - match options[index].as_str() { - s if s == NONE_OPTION => Ok(None), - s if s == OTHER_OPTION => prompt_as_text(field, prompt_text, None::<&str>, validator_regex), - value => Ok(Some(value.to_string())), - } + let required = field.required; + let pattern = field.validation.clone(); + + let validate = move |value: &str| -> std::result::Result<(), String> { + if required && value.trim().is_empty() { + return Err("This field is required.".to_string()); + } + if let Some(ref re) = validator_regex + && !value.is_empty() + && !re.is_match(value) + { + return Err(format!( + "Must match pattern: {}", + pattern.as_deref().unwrap_or("") + )); + } + Ok(()) + }; + + crate::prompt::select_or_create(prompt_text, candidates, !required, &validate) } fn prompt_as_text( diff --git a/src/prompt.rs b/src/prompt.rs index 47958f3..a5dc571 100644 --- a/src/prompt.rs +++ b/src/prompt.rs @@ -6,13 +6,17 @@ //! //! The prompts for config-declared extra fields live in [`crate::extra_fields`], next to the //! declarations and the prefetch that feed them; this module drives them in the configured -//! order via [`fields`]. +//! order via [`fields`], and lends them the [`select_or_create`] picker for their candidate lists. + +mod picker; use std::{collections::HashMap, fmt::Display}; use dialoguer::{Confirm, FuzzySelect, Input, MultiSelect}; use regex::Regex; +pub(crate) use picker::select_or_create; + use crate::{ errors::{Result, RonaError}, extra_fields::{BuiltInFieldConfig, ExtraField, prompt_extra_field}, diff --git a/src/prompt/picker.rs b/src/prompt/picker.rs new file mode 100644 index 0000000..20edcce --- /dev/null +++ b/src/prompt/picker.rs @@ -0,0 +1,455 @@ +//! Fuzzy picker that also accepts a value the list does not hold. +//! +//! The candidates of a select field are suggestions, not a closed set: typing a value that is +//! not in the list offers it as a `Create "..."` row, so a brand new scope costs the same +//! keystrokes as an existing one instead of a detour through a manual-entry option. +//! +//! `dialoguer` has no such prompt ([`dialoguer::FuzzySelect`] can only return one of the items it +//! was built with), so this module drives the terminal itself. Every line is still rendered +//! through the shared [`crate::theme::prompt_theme`], so the picker looks like the other prompts. + +use dialoguer::{ + console::{Key, Term, truncate_str}, + theme::{ColorfulTheme, Theme}, +}; + +use crate::errors::{Result, RonaError}; + +/// Label of the row that skips an optional field. +const NONE_LABEL: &str = "(none)"; + +/// A row of the picker, and what accepting it means. +#[derive(Debug, Clone, PartialEq, Eq)] +enum Row { + /// One of the candidates, accepted as it is. + Candidate(String), + /// The typed text, accepted as a new value. + Create(String), + /// Skip the field entirely; only offered to optional fields. + Skip, +} + +impl Row { + /// Text shown for the row. + fn label(&self) -> String { + match self { + Self::Candidate(value) => value.clone(), + Self::Create(value) => format!("Create \"{value}\""), + Self::Skip => NONE_LABEL.to_string(), + } + } + + /// Value the row yields, or `None` when it skips the field. + fn value(&self) -> Option<&str> { + match self { + Self::Candidate(value) | Self::Create(value) => Some(value), + Self::Skip => None, + } + } +} + +/// Builds the rows shown for `query`, best match first. +/// +/// An empty query lists every candidate in prefetch order, followed by the skip row when the field +/// is optional. A non-empty query keeps the matching candidates and appends a `Create` row unless a +/// candidate already holds that exact text: the best match keeps the selection, and once the filter +/// has emptied the list the `Create` row is the only one left, so a new value is a plain Enter away. +fn rows(candidates: &[String], query: &str, allow_skip: bool) -> Vec { + if query.is_empty() { + let mut rows: Vec = candidates.iter().cloned().map(Row::Candidate).collect(); + if allow_skip { + rows.push(Row::Skip); + } + return rows; + } + + let mut matches: Vec<(usize, i32, &String)> = candidates + .iter() + .enumerate() + .filter_map(|(position, candidate)| { + score(candidate, query).map(|score| (position, score, candidate)) + }) + .collect(); + + // Best score first, ties broken by the order the prefetch returned them in. + matches.sort_by(|(left_pos, left_score, _), (right_pos, right_score, _)| { + right_score.cmp(left_score).then(left_pos.cmp(right_pos)) + }); + + let mut rows: Vec = matches + .into_iter() + .map(|(_, _, candidate)| Row::Candidate(candidate.clone())) + .collect(); + + if !candidates.iter().any(|candidate| candidate == query) { + rows.push(Row::Create(query.to_string())); + } + + rows +} + +/// Scores `candidate` against `query`, or `None` when it does not match at all. +/// +/// Whole-string equality beats a prefix, a prefix beats a substring, and a substring beats a +/// scattered subsequence; within a rank the shorter or earlier-matching candidate wins. +fn score(candidate: &str, query: &str) -> Option { + let candidate = candidate.to_lowercase(); + let query = query.to_lowercase(); + let length = i32::try_from(candidate.chars().count()).unwrap_or(i32::MAX); + + if candidate == query { + return Some(4000); + } + + if let Some(position) = candidate.find(&query) { + let position = i32::try_from(position).unwrap_or(i32::MAX); + return Some(if position == 0 { + 3000 - length + } else { + 2000 - position + }); + } + + is_subsequence(&candidate, &query).then(|| 1000 - length) +} + +/// Whether every char of `needle` appears in `haystack`, in order. +fn is_subsequence(haystack: &str, needle: &str) -> bool { + let mut haystack = haystack.chars(); + needle + .chars() + .all(|wanted| haystack.any(|current| current == wanted)) +} + +/// Byte offset of the `index`-th char, or the end of the string. +fn byte_position(text: &str, index: usize) -> usize { + text.char_indices() + .nth(index) + .map_or(text.len(), |(offset, _)| offset) +} + +/// Turns a rendering failure into an error, since the theme writes into a `String`. +fn render_error(error: std::fmt::Error) -> RonaError { + RonaError::InvalidInput(format!("Failed to render prompt: {error}")) +} + +/// Terminal state of one running picker. +struct Picker<'a> { + /// Terminal the prompt is drawn on; stderr, like every other prompt. + term: Term, + /// Shared prompt theme, used for every rendered line. + theme: ColorfulTheme, + /// Label shown before the typed text. + prompt: &'a str, + /// Values offered by the prefetch, in the order it returned them. + candidates: &'a [String], + /// Whether the skip row is offered. + allow_skip: bool, + /// Text typed so far, which both filters the list and feeds the `Create` row. + query: String, + /// Caret position in `query`, counted in chars. + cursor: usize, + /// Index of the highlighted row. + selected: usize, + /// Index of the first rendered row, so a long list can scroll. + offset: usize, + /// Validation message shown under the list, cleared as soon as the query changes. + error: Option, + /// Number of lines the last frame wrote, so the next one can clear them. + drawn: usize, +} + +impl Picker<'_> { + /// Runs the prompt, restoring the cursor whatever the outcome. + fn run( + &mut self, + validate: &dyn Fn(&str) -> std::result::Result<(), String>, + ) -> Result> { + self.term.hide_cursor()?; + let outcome = self.interact(validate); + let _ = self.term.show_cursor(); + outcome + } + + /// Draws the list and handles keys until the user accepts a row or cancels. + fn interact( + &mut self, + validate: &dyn Fn(&str) -> std::result::Result<(), String>, + ) -> Result> { + loop { + let rows = rows(self.candidates, &self.query, self.allow_skip); + self.selected = self.selected.min(rows.len().saturating_sub(1)); + self.render(&rows)?; + + match self.term.read_key()? { + Key::Escape | Key::CtrlC => { + self.clear()?; + return Err(RonaError::UserCancelled); + } + Key::Enter => { + let Some(row) = rows.get(self.selected) else { + continue; + }; + let Some(value) = row.value() else { + self.report(NONE_LABEL)?; + return Ok(None); + }; + match validate(value) { + Ok(()) => { + let value = value.to_string(); + self.report(&value)?; + return Ok(Some(value)); + } + Err(message) => self.error = Some(message), + } + } + Key::ArrowUp | Key::BackTab => self.move_selection(-1, rows.len()), + Key::ArrowDown | Key::Tab => self.move_selection(1, rows.len()), + Key::ArrowLeft => self.cursor = self.cursor.saturating_sub(1), + Key::ArrowRight => self.cursor = (self.cursor + 1).min(self.query.chars().count()), + Key::Home => self.cursor = 0, + Key::End => self.cursor = self.query.chars().count(), + Key::Backspace if self.cursor > 0 => { + self.cursor -= 1; + let at = byte_position(&self.query, self.cursor); + self.query.remove(at); + self.reset_filter(); + } + Key::Del if self.cursor < self.query.chars().count() => { + let at = byte_position(&self.query, self.cursor); + self.query.remove(at); + self.reset_filter(); + } + Key::Char(typed) if !typed.is_ascii_control() => { + let at = byte_position(&self.query, self.cursor); + self.query.insert(at, typed); + self.cursor += 1; + self.reset_filter(); + } + _ => {} + } + } + } + + /// Moves the highlight by `delta`, wrapping around the list. + fn move_selection(&mut self, delta: isize, length: usize) { + if length == 0 { + return; + } + let length = isize::try_from(length).unwrap_or(isize::MAX); + let current = isize::try_from(self.selected).unwrap_or(0); + self.selected = usize::try_from((current + delta).rem_euclid(length)).unwrap_or(0); + } + + /// Restarts the list from the top; the typed text just changed, so the old state is stale. + fn reset_filter(&mut self) { + self.selected = 0; + self.offset = 0; + self.error = None; + } + + /// Redraws the prompt line, the visible rows, and any validation message. + fn render(&mut self, rows: &[Row]) -> Result<()> { + let (height, width) = self.term.size(); + // Two lines are kept for the prompt and a validation message. + let visible = usize::from(height).saturating_sub(2).max(1); + let width = usize::from(width).max(1); + + if rows.len() <= visible { + self.offset = 0; + } else if self.selected < self.offset { + self.offset = self.selected; + } else if self.selected >= self.offset + visible { + self.offset = self.selected + 1 - visible; + } + + let mut lines: Vec = Vec::with_capacity(visible + 2); + + let mut header = String::new(); + self.theme + .format_fuzzy_select_prompt( + &mut header, + self.prompt, + &self.query, + byte_position(&self.query, self.cursor), + ) + .map_err(render_error)?; + lines.push(header); + + for (index, row) in rows.iter().enumerate().skip(self.offset).take(visible) { + let mut line = String::new(); + self.theme + .format_select_prompt_item(&mut line, &row.label(), index == self.selected) + .map_err(render_error)?; + lines.push(line); + } + + if let Some(message) = &self.error { + let mut line = String::new(); + self.theme + .format_error(&mut line, message) + .map_err(render_error)?; + lines.push(line); + } + + self.term.clear_last_lines(self.drawn)?; + for line in &lines { + // A wrapped line would take two rows and break the clearing above. + self.term.write_line(&truncate_str(line, width, "..."))?; + } + self.term.flush()?; + self.drawn = lines.len(); + + Ok(()) + } + + /// Erases the frame currently on screen. + fn clear(&mut self) -> Result<()> { + self.term.clear_last_lines(self.drawn)?; + self.drawn = 0; + self.term.flush()?; + Ok(()) + } + + /// Replaces the frame with the single line echoing the answer, as the other prompts do. + fn report(&mut self, value: &str) -> Result<()> { + self.clear()?; + let mut line = String::new(); + self.theme + .format_input_prompt_selection(&mut line, self.prompt, value) + .map_err(render_error)?; + self.term.write_line(&line)?; + self.term.flush()?; + Ok(()) + } +} + +/// Asks the user to pick one of `candidates` or to type a value of their own. +/// +/// Typing filters the list; Enter takes the highlighted row, which is the `Create "..."` row as +/// soon as the typed text matches no candidate. `allow_skip` adds the `(none)` row that optional +/// fields need, and returns `Ok(None)` when the user takes it. `validate` guards both a picked +/// candidate and a created value: its message is shown under the list and the prompt stays open. +/// +/// # Errors +/// * If the terminal is not interactive +/// * If the user cancels with Esc or Ctrl-C +pub(crate) fn select_or_create( + prompt: &str, + candidates: &[String], + allow_skip: bool, + validate: &dyn Fn(&str) -> std::result::Result<(), String>, +) -> Result> { + let term = Term::stderr(); + if !term.is_term() { + return Err(RonaError::UserCancelled); + } + + Picker { + term, + theme: crate::theme::prompt_theme(), + prompt, + candidates, + allow_skip, + query: String::new(), + cursor: 0, + selected: 0, + offset: 0, + error: None, + drawn: 0, + } + .run(validate) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn candidates() -> Vec { + ["api", "auth", "cli", "parser"] + .iter() + .map(|value| (*value).to_string()) + .collect() + } + + fn labels(rows: &[Row]) -> Vec { + rows.iter().map(Row::label).collect() + } + + #[test] + fn test_empty_query_lists_every_candidate_in_order() { + let rows = rows(&candidates(), "", false); + + assert_eq!(labels(&rows), vec!["api", "auth", "cli", "parser"]); + } + + #[test] + fn test_empty_query_appends_the_skip_row_when_optional() { + let rows = rows(&candidates(), "", true); + + assert_eq!(rows.last(), Some(&Row::Skip)); + } + + #[test] + fn test_typed_text_is_offered_as_a_new_value() { + let rows = rows(&candidates(), "billing", true); + + // Nothing matches, so the create row is the only one and Enter accepts it. + assert_eq!(rows, vec![Row::Create("billing".to_string())]); + } + + #[test] + fn test_matches_come_before_the_create_row() { + let rows = rows(&candidates(), "a", false); + + assert_eq!(labels(&rows), vec!["api", "auth", "parser", "Create \"a\""]); + } + + #[test] + fn test_no_create_row_when_a_candidate_holds_the_text() { + let rows = rows(&candidates(), "api", false); + + assert_eq!(labels(&rows), vec!["api"]); + } + + #[test] + fn test_skip_row_is_hidden_once_the_user_types() { + let rows = rows(&candidates(), "a", true); + + assert!(!rows.contains(&Row::Skip)); + } + + #[test] + fn test_exact_match_outranks_prefix_and_substring() { + let rows = rows(&["parser".to_string(), "par".to_string()], "par", false); + + assert_eq!(labels(&rows), vec!["par", "parser"]); + } + + #[test] + fn test_matching_ignores_case() { + let rows = rows(&["API".to_string()], "api", false); + + // The candidate still matches, but the differently-cased text stays creatable: a scope + // typed in another case is a value of its own. + assert_eq!(labels(&rows), vec!["API", "Create \"api\""]); + } + + #[test] + fn test_subsequence_matches_rank_last() { + let rows = rows(&["auth".to_string(), "ah".to_string()], "ah", false); + + assert_eq!(labels(&rows), vec!["ah", "auth"]); + } + + #[test] + fn test_score_rejects_a_non_match() { + assert!(score("api", "zz").is_none()); + } + + #[test] + fn test_byte_position_handles_multibyte_text() { + assert_eq!(byte_position("éa", 1), 2); + assert_eq!(byte_position("éa", 9), 3); + } +}