Skip to content

Commit

Permalink
Name the Value conversion functions more clearly (#11851)
Browse files Browse the repository at this point in the history
# Description
This PR renames the conversion functions on `Value` to be more consistent.
It follows the Rust [API guidelines](https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv) for ad-hoc conversions.
The conversion functions on `Value` now come in a few forms:
- `coerce_{type}` takes a `&Value` and attempts to convert the value to
`type` (e.g., `i64` are converted to `f64`). This is the old behavior of
some of the `as_{type}` functions -- these functions have simply been
renamed to better reflect what they do.
- The new `as_{type}` functions take a `&Value` and returns an `Ok`
result only if the value is of `type` (no conversion is attempted). The
returned value will be borrowed if `type` is non-`Copy`, otherwise an
owned value is returned.
- `into_{type}` exists for non-`Copy` types, but otherwise does not
attempt conversion just like `as_type`. It takes an owned `Value` and
always returns an owned result.
- `coerce_into_{type}` has the same relationship with `coerce_{type}` as
`into_{type}` does with `as_{type}`.
- `to_{kind}_string`: conversion to different string formats (debug,
abbreviated, etc.). Only two of the old string conversion functions were
removed, the rest have been renamed only.
- `to_{type}`: other conversion functions. Currently, only `to_path`
exists. (And `to_string` through `Display`.)

This table summaries the above:
| Form | Cost | Input Ownership | Output Ownership | Converts `Value`
case/`type` |
| ---------------------------- | ----- | --------------- |
---------------- | -------- |
| `as_{type}` | Cheap | Borrowed | Borrowed/Owned | No |
| `into_{type}` | Cheap | Owned | Owned | No |
| `coerce_{type}` | Cheap | Borrowed | Borrowed/Owned | Yes |
| `coerce_into_{type}` | Cheap | Owned | Owned | Yes |
| `to_{kind}_string` | Expensive | Borrowed | Owned | Yes |
| `to_{type}` | Expensive | Borrowed | Owned | Yes |

# User-Facing Changes
Breaking API change for `Value` in `nu-protocol` which is exposed as
part of the plugin API.
  • Loading branch information
IanManske committed Feb 17, 2024
1 parent 360ebeb commit 1c49ca5
Show file tree
Hide file tree
Showing 117 changed files with 846 additions and 688 deletions.
20 changes: 9 additions & 11 deletions crates/nu-cli/src/commands/commandline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,12 @@ impl Command for Commandline {
_input: PipelineData,
) -> Result<PipelineData, ShellError> {
if let Some(cmd) = call.opt::<Value>(engine_state, stack, 0)? {
let span = cmd.span();
let cmd = cmd.coerce_into_string()?;
let mut repl = engine_state.repl_state.lock().expect("repl state mutex");

if call.has_flag(engine_state, stack, "cursor")? {
let cmd_str = cmd.as_string()?;
match cmd_str.parse::<i64>() {
match cmd.parse::<i64>() {
Ok(n) => {
repl.cursor_pos = if n <= 0 {
0usize
Expand All @@ -90,22 +91,19 @@ impl Command for Commandline {
return Err(ShellError::CantConvert {
to_type: "int".to_string(),
from_type: "string".to_string(),
span: cmd.span(),
help: Some(format!(
r#"string "{cmd_str}" does not represent a valid int"#
)),
span,
help: Some(format!(r#"string "{cmd}" does not represent a valid int"#)),
})
}
}
} else if call.has_flag(engine_state, stack, "append")? {
repl.buffer.push_str(&cmd.as_string()?);
repl.buffer.push_str(&cmd);
} else if call.has_flag(engine_state, stack, "insert")? {
let cmd_str = cmd.as_string()?;
let cursor_pos = repl.cursor_pos;
repl.buffer.insert_str(cursor_pos, &cmd_str);
repl.cursor_pos += cmd_str.len();
repl.buffer.insert_str(cursor_pos, &cmd);
repl.cursor_pos += cmd.len();
} else {
repl.buffer = cmd.as_string()?;
repl.buffer = cmd;
repl.cursor_pos = repl.buffer.len();
}
Ok(Value::nothing(call.head).into_pipeline_data())
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-cli/src/commands/keybindings_listen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ pub fn print_events(engine_state: &EngineState) -> Result<Value, ShellError> {
let o = match v {
Value::Record { val, .. } => val
.iter()
.map(|(x, y)| format!("{}: {}", x, y.into_string("", config)))
.map(|(x, y)| format!("{}: {}", x, y.to_expanded_string("", config)))
.collect::<Vec<String>>()
.join(", "),

Expand Down
2 changes: 1 addition & 1 deletion crates/nu-cli/src/completions/command_completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl CommandCompletion {
if let Some(paths) = paths {
if let Ok(paths) = paths.as_list() {
for path in paths {
let path = path.as_string().unwrap_or_default();
let path = path.coerce_string().unwrap_or_default();

if let Ok(mut contents) = std::fs::read_dir(path) {
while let Some(Ok(item)) = contents.next() {
Expand Down
6 changes: 3 additions & 3 deletions crates/nu-cli/src/completions/completer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ pub fn map_value_completions<'a>(
) -> Vec<Suggestion> {
list.filter_map(move |x| {
// Match for string values
if let Ok(s) = x.as_string() {
if let Ok(s) = x.coerce_string() {
return Some(Suggestion {
value: s,
description: None,
Expand Down Expand Up @@ -507,7 +507,7 @@ pub fn map_value_completions<'a>(
// Match `value` column
if it.0 == "value" {
// Convert the value to string
if let Ok(val_str) = it.1.as_string() {
if let Ok(val_str) = it.1.coerce_string() {
// Update the suggestion value
suggestion.value = val_str;
}
Expand All @@ -516,7 +516,7 @@ pub fn map_value_completions<'a>(
// Match `description` column
if it.0 == "description" {
// Convert the value to string
if let Ok(desc_str) = it.1.as_string() {
if let Ok(desc_str) = it.1.coerce_string() {
// Update the suggestion value
suggestion.description = Some(desc_str);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-cli/src/completions/custom_completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl Completer for CustomCompletion {
},
match_algorithm: match options.get("completion_algorithm") {
Some(option) => option
.as_string()
.coerce_string()
.ok()
.and_then(|option| option.try_into().ok())
.unwrap_or(MatchAlgorithm::Prefix),
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-cli/src/completions/dotnu_completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl Completer for DotNuCompletion {
.into_iter()
.flat_map(|it| {
it.iter().map(|x| {
x.as_path()
x.to_path()
.expect("internal error: failed to convert lib path")
})
})
Expand Down
4 changes: 2 additions & 2 deletions crates/nu-cli/src/eval_cmds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub fn evaluate_commands(
let (block, delta) = {
if let Some(ref t_mode) = table_mode {
let mut config = engine_state.get_config().clone();
config.table_mode = t_mode.as_string()?.parse().unwrap_or_default();
config.table_mode = t_mode.coerce_string()?.parse().unwrap_or_default();
engine_state.set_config(config);
}

Expand Down Expand Up @@ -59,7 +59,7 @@ pub fn evaluate_commands(
Ok(pipeline_data) => {
let mut config = engine_state.get_config().clone();
if let Some(t_mode) = table_mode {
config.table_mode = t_mode.as_string()?.parse().unwrap_or_default();
config.table_mode = t_mode.coerce_string()?.parse().unwrap_or_default();
}
crate::eval_file::print_table_or_error(engine_state, stack, pipeline_data, &mut config)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/nu-cli/src/eval_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ fn print_or_exit(pipeline_data: PipelineData, engine_state: &mut EngineState, co
std::process::exit(1);
}

let out = item.into_string("\n", config) + "\n";
let out = item.to_expanded_string("\n", config) + "\n";
let _ = stdout_write_all_and_flush(out).map_err(|err| eprintln!("{err}"));
}
}
4 changes: 2 additions & 2 deletions crates/nu-cli/src/menus/help_completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl NuHelpCompleter {

if !sig.named.is_empty() {
long_desc.push_str(&get_flags_section(Some(&*self.0.clone()), sig, |v| {
v.into_string_parsable(", ", &self.0.config)
v.to_parsable_string(", ", &self.0.config)
}))
}

Expand All @@ -73,7 +73,7 @@ impl NuHelpCompleter {
let opt_suffix = if let Some(value) = &positional.default_value {
format!(
" (optional, default: {})",
&value.into_string_parsable(", ", &self.0.config),
&value.to_parsable_string(", ", &self.0.config),
)
} else {
(" (optional)").to_string()
Expand Down
6 changes: 4 additions & 2 deletions crates/nu-cli/src/menus/menu_completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,12 @@ fn convert_to_suggestions(
Value::Record { val, .. } => {
let text = val
.get("value")
.and_then(|val| val.as_string().ok())
.and_then(|val| val.coerce_string().ok())
.unwrap_or_else(|| "No value key".to_string());

let description = val.get("description").and_then(|val| val.as_string().ok());
let description = val
.get("description")
.and_then(|val| val.coerce_string().ok());

let span = match val.get("span") {
Some(Value::Record { val: span, .. }) => {
Expand Down
3 changes: 1 addition & 2 deletions crates/nu-cli/src/nu_highlight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,9 @@ impl Command for NuHighlight {
};

input.map(
move |x| match x.as_string() {
move |x| match x.coerce_into_string() {
Ok(line) => {
let highlights = highlighter.highlight(&line, line.len());

Value::string(highlights.render_simple(), head)
}
Err(err) => Value::error(err, head),
Expand Down
Loading

0 comments on commit 1c49ca5

Please sign in to comment.