diff --git a/src/args.rs b/src/args.rs index 78ed79ee..f567ad49 100644 --- a/src/args.rs +++ b/src/args.rs @@ -55,6 +55,11 @@ impl Args<'_> { /// .unwrap_stdout(); /// assert_eq!(r, "-f"); /// ``` + /// + /// Note to self: shell passes "" as a parameter in situations like foo `--bar TAB`, bpaf + /// completion stubs adopt this conventions add pass it along. This is needed so completer can + /// tell the difference between `--bar` being completed or an argument to it in the example + /// above. #[cfg(feature = "autocomplete")] #[must_use] pub fn set_comp(mut self, rev: usize) -> Self { diff --git a/src/complete_gen.rs b/src/complete_gen.rs index 10ee608d..3fdd0041 100644 --- a/src/complete_gen.rs +++ b/src/complete_gen.rs @@ -166,7 +166,7 @@ impl State { } impl Complete { - pub(crate) fn push_shell(&mut self, op: ShellComp, depth: usize) { + pub(crate) fn push_shell(&mut self, op: ShellComp, is_argument: bool, depth: usize) { self.comps.push(Comp::Shell { extra: CompExtra { depth, @@ -174,6 +174,7 @@ impl Complete { help: None, }, script: op, + is_argument, }); } @@ -224,10 +225,7 @@ pub(crate) struct CompExtra { #[derive(Clone, Debug)] pub(crate) enum Comp { /// short or long flag - Flag { - extra: CompExtra, - name: ShortLong, - }, + Flag { extra: CompExtra, name: ShortLong }, /// argument + metadata Argument { @@ -256,12 +254,15 @@ pub(crate) enum Comp { Metavariable { extra: CompExtra, meta: &'static str, + /// AKA not positional is_argument: bool, }, Shell { extra: CompExtra, script: ShellComp, + /// AKA not positional + is_argument: bool, }, } @@ -348,6 +349,9 @@ fn pair_to_os_string<'a>(pair: (&'a Arg, &'a OsStr)) -> Option<(&'a Arg, &'a str Some((pair.0, pair.1.to_str()?)) } +/// What is the preceeding item, if any +/// +/// Mostly is there to tell if we are trying to complete and argument or not... #[derive(Debug, Copy, Clone)] enum Prefix<'a> { NA, @@ -374,7 +378,7 @@ impl State { // try get a current item to complete - must be non-virtual right most one // value must be present here, and can fail only for non-utf8 values // can't do much completing with non-utf8 values since bpaf needs to print them to stdout - let (_, lit) = items.next()?; + let (cur, lit) = items.next()?; // For cases like "-k=val", "-kval", "--key=val", "--key val" // last value is going to be either Arg::Word or Arg::ArgWord @@ -389,13 +393,18 @@ impl State { _ => (false, lit), }; + let is_named = match cur { + Arg::Short(_, _, _) | Arg::Long(_, _, _) => true, + Arg::ArgWord(_) | Arg::Word(_) | Arg::PosWord(_) => false, + }; + let prefix = match preceeding { Some((Arg::Short(s, true, _os), _lit)) => Prefix::Short(*s), Some((Arg::Long(l, true, _os), _lit)) => Prefix::Long(l.as_str()), _ => Prefix::NA, }; - let (items, shell) = comp.complete(lit, pos_only, prefix); + let (items, shell) = comp.complete(lit, pos_only, is_named, prefix); Some(match comp.output_rev { 0 => render_test(&items, &shell, full_lit), @@ -480,10 +489,9 @@ impl Comp { fn only_value(&self) -> bool { match self { Comp::Flag { .. } | Comp::Argument { .. } | Comp::Command { .. } => false, - Comp::Metavariable { is_argument, .. } | Comp::Value { is_argument, .. } => { - *is_argument - } - Comp::Shell { .. } => true, + Comp::Metavariable { is_argument, .. } + | Comp::Value { is_argument, .. } + | Comp::Shell { is_argument, .. } => *is_argument, } } fn is_pos(&self) -> bool { @@ -500,6 +508,7 @@ impl Complete { &self, arg: &str, pos_only: bool, + is_named: bool, prefix: Prefix, ) -> (Vec, Vec) { let mut items: Vec = Vec::new(); @@ -588,7 +597,9 @@ impl Complete { } Comp::Shell { script, .. } => { - shell.push(*script); + if !is_named { + shell.push(*script); + } } } } diff --git a/src/complete_shell.rs b/src/complete_shell.rs index 91a83df9..d009b817 100644 --- a/src/complete_shell.rs +++ b/src/complete_shell.rs @@ -97,8 +97,8 @@ where let depth = args.depth(); if let Some(comp) = args.comp_mut() { for ci in comp_items { - if ci.is_metavar().is_some() { - comp.push_shell(self.op, depth); + if let Some(is_argument) = ci.is_metavar() { + comp.push_shell(self.op, is_argument, depth); } else { comp.push_comp(ci); } diff --git a/src/structs.rs b/src/structs.rs index baf59fc4..695d7ec2 100644 --- a/src/structs.rs +++ b/src/structs.rs @@ -368,29 +368,54 @@ fn this_or_that_picks_first( }; #[cfg(feature = "autocomplete")] - let len_a = args_a.len(); - - #[cfg(feature = "autocomplete")] - let len_b = args_b.len(); - - #[cfg(feature = "autocomplete")] - if let (Some(a), Some(b)) = (args_a.comp_mut(), args_b.comp_mut()) { - // if both parsers managed to consume the same amount - including 0, keep - // results from both, otherwise keep results from one that consumed more - let (keep_a, keep_b) = match res { - Ok((true, _)) => (true, false), - Ok((false, _)) => (false, true), - Err(_) => match len_a.cmp(&len_b) { - std::cmp::Ordering::Less => (true, false), - std::cmp::Ordering::Equal => (true, true), - std::cmp::Ordering::Greater => (false, true), - }, - }; - if keep_a { - comp_stash.extend(a.drain_comps()); + { + let mut keep_a = true; + let mut keep_b = true; + if args_a.len() != args_b.len() { + // If neither parser consumed anything - both can produce valid completions, otherwise + // look for the first "significant" consume and keep that parser + // + // This is needed to preserve completion from a choice between a positional and a flag + // See https://github.com/pacak/bpaf/issues/303 for more details + if let (Some(_), Some(_)) = (args_a.comp_mut(), args_b.comp_mut()) { + 'check: for (ix, arg) in args_a.items.iter().enumerate() { + // During completion process named and unnamed arguments behave + // different - `-` and `--` are positional arguments, but we want to produce + // named items too. An empty string is also a special type of item that + // gets passed when user starts completion without passing any actual data. + // + // All other strings are either valid named items or valid positional items + // those are hopefully follow the right logic for being parsed/not parsed + if ix + 1 == args_a.items.len() { + let os = arg.os_str(); + if os.is_empty() || os == "-" || os == "--" { + break 'check; + } + } + if let (Some(a), Some(b)) = (args_a.present(ix), args_b.present(ix)) { + match (a, b) { + (false, true) => { + keep_b = false; + break 'check; + } + (true, false) => { + keep_a = false; + break 'check; + } + _ => {} + } + } + } + } } - if keep_b { - comp_stash.extend(b.drain_comps()); + + if let (Some(a), Some(b)) = (args_a.comp_mut(), args_b.comp_mut()) { + if keep_a { + comp_stash.extend(a.drain_comps()); + } + if keep_b { + comp_stash.extend(b.drain_comps()); + } } } diff --git a/tests/chezmoi.rs b/tests/chezmoi.rs new file mode 100644 index 00000000..411a1568 --- /dev/null +++ b/tests/chezmoi.rs @@ -0,0 +1,231 @@ +use bpaf::*; +use std::{path::PathBuf, str::FromStr}; + +#[derive(Copy, Clone, Debug)] +pub enum Style { + /// Program is in PATH + InPath, + /// Program is in .utils of chezmoi source state + InSrc, +} + +impl FromStr for Style { + type Err = &'static str; + + fn from_str(s: &str) -> Result { + match s { + "path" => Ok(Style::InPath), + "src" => Ok(Style::InSrc), + _ => Err("Not valid"), + } + } +} + +/// Parser for `--style` +fn style() -> impl Parser