Skip to content

Commit

Permalink
Plugin explicit flags (#11581)
Browse files Browse the repository at this point in the history
# Description
#11492 fixed flags for builtin commands but I missed that plugins don't
use the same `has_flag` that builtins do. This PR addresses this.

Unfortunately this means that return value of `has_flag` needs to change
from `bool` to `Result<bool, ShellError>` to produce an error when
explicit value is not a boolean (just like in case of `has_flag` for
builtin commands. It is not possible to check this in
`EvaluatedCall::try_from_call` because

# User-Facing Changes
Passing explicit values to flags of plugin commands (like `--flag=true`
`--flag=false`) should work now.
BREAKING: changed return value of `EvaluatedCall::has_flag` method from
`bool` to `Result<bool, ShellError>`

# Tests + Formatting
Added tests and updated documentation and examples
  • Loading branch information
NotLebedev committed Jan 22, 2024
1 parent 415ebf2 commit 092d496
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 16 deletions.
77 changes: 67 additions & 10 deletions crates/nu-plugin/src/protocol/evaluated_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ impl EvaluatedCall {
})
}

/// Indicates whether named parameter is present in the arguments
///
/// Typically this method would be used on a flag parameter, a named parameter
/// that does not take a value.
/// Check if a flag (named parameter that does not take a value) is set
/// Returns Ok(true) if flag is set or passed true value
/// Returns Ok(false) if flag is not set or passed false value
/// Returns Err if passed value is not a boolean
///
/// # Examples
/// Invoked as `my_command --foo`:
Expand All @@ -72,7 +72,7 @@ impl EvaluatedCall {
/// # None
/// # )],
/// # };
/// assert!(call.has_flag("foo"));
/// assert!(call.has_flag("foo").unwrap());
/// ```
///
/// Invoked as `my_command --bar`:
Expand All @@ -88,16 +88,73 @@ impl EvaluatedCall {
/// # None
/// # )],
/// # };
/// assert!(!call.has_flag("foo"));
/// assert!(!call.has_flag("foo").unwrap());
/// ```
///
/// Invoked as `my_command --foo=true`:
/// ```
/// # use nu_protocol::{Spanned, Span, Value};
/// # use nu_plugin::EvaluatedCall;
/// # let null_span = Span::new(0, 0);
/// # let call = EvaluatedCall {
/// # head: null_span,
/// # positional: Vec::new(),
/// # named: vec![(
/// # Spanned { item: "foo".to_owned(), span: null_span},
/// # Some(Value::bool(true, Span::unknown()))
/// # )],
/// # };
/// assert!(call.has_flag("foo").unwrap());
/// ```
///
/// Invoked as `my_command --foo=false`:
/// ```
/// # use nu_protocol::{Spanned, Span, Value};
/// # use nu_plugin::EvaluatedCall;
/// # let null_span = Span::new(0, 0);
/// # let call = EvaluatedCall {
/// # head: null_span,
/// # positional: Vec::new(),
/// # named: vec![(
/// # Spanned { item: "foo".to_owned(), span: null_span},
/// # Some(Value::bool(false, Span::unknown()))
/// # )],
/// # };
/// assert!(!call.has_flag("foo").unwrap());
/// ```
///
/// Invoked with wrong type as `my_command --foo=1`:
/// ```
/// # use nu_protocol::{Spanned, Span, Value};
/// # use nu_plugin::EvaluatedCall;
/// # let null_span = Span::new(0, 0);
/// # let call = EvaluatedCall {
/// # head: null_span,
/// # positional: Vec::new(),
/// # named: vec![(
/// # Spanned { item: "foo".to_owned(), span: null_span},
/// # Some(Value::int(1, Span::unknown()))
/// # )],
/// # };
/// assert!(call.has_flag("foo").is_err());
/// ```
pub fn has_flag(&self, flag_name: &str) -> bool {
pub fn has_flag(&self, flag_name: &str) -> Result<bool, ShellError> {
for name in &self.named {
if flag_name == name.0.item {
return true;
return match &name.1 {
Some(Value::Bool { val, .. }) => Ok(*val),
None => Ok(true),
Some(result) => Err(ShellError::CantConvert {
to_type: "bool".into(),
from_type: result.get_type().to_string(),
span: result.span(),
help: Some("".into()),
}),
};
}
}

false
Ok(false)
}

/// Returns the [`Value`] of an optional named argument
Expand Down Expand Up @@ -340,7 +397,7 @@ mod test {
let name: Option<f64> = call.get_flag("name").unwrap();
assert_eq!(name, Some(1.0));

assert!(call.has_flag("flag"));
assert!(call.has_flag("flag").unwrap());

let required: f64 = call.req(0).unwrap();
assert!((required - 1.0).abs() < f64::EPSILON);
Expand Down
2 changes: 1 addition & 1 deletion crates/nu_plugin_example/src/example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl Example {
// Keep this in mind when designing your plugin signatures
let a: i64 = call.req(0)?;
let b: String = call.req(1)?;
let flag = call.has_flag("flag");
let flag = call.has_flag("flag")?;
let opt: Option<i64> = call.opt(2)?;
let named: Option<String> = call.get_flag("named")?;
let rest: Vec<String> = call.rest(3)?;
Expand Down
6 changes: 3 additions & 3 deletions crates/nu_plugin_inc/src/nu/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ impl Plugin for Inc {

self.cell_path = cell_path;

if call.has_flag("major") {
if call.has_flag("major")? {
self.for_semver(SemVerAction::Major);
}
if call.has_flag("minor") {
if call.has_flag("minor")? {
self.for_semver(SemVerAction::Minor);
}
if call.has_flag("patch") {
if call.has_flag("patch")? {
self.for_semver(SemVerAction::Patch);
}

Expand Down
4 changes: 2 additions & 2 deletions crates/nu_plugin_query/src/query_web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ pub fn parse_selector_params(call: &EvaluatedCall, input: &Value) -> Result<Valu
Some(q2) => q2,
None => "".to_string(),
};
let as_html = call.has_flag("as-html");
let as_html = call.has_flag("as-html")?;
let attribute = call.get_flag("attribute")?.unwrap_or_default();
let as_table: Value = call
.get_flag("as-table")?
.unwrap_or_else(|| Value::nothing(head));

let inspect = call.has_flag("inspect");
let inspect = call.has_flag("inspect")?;

if !&query.is_empty() && ScraperSelector::parse(&query).is_err() {
return Err(LabeledError {
Expand Down
13 changes: 13 additions & 0 deletions tests/plugins/core_inc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,16 @@ fn semversion_without_passing_field() {
assert_eq!(actual.out, "0.1.4");
})
}

#[test]
fn explicit_flag() {
Playground::setup("plugin_inc_test_6", |dirs, _| {
let actual = nu_with_plugins!(
cwd: dirs.test(),
plugin: ("nu_plugin_inc"),
"'0.1.2' | inc --major=false --minor=true --patch=false"
);

assert_eq!(actual.out, "0.2.0");
})
}

0 comments on commit 092d496

Please sign in to comment.