From 6489ef5ac0d5c270bbc366f36fb2dc4d1617ee50 Mon Sep 17 00:00:00 2001 From: Fabian Boehm Date: Tue, 8 Aug 2023 20:12:05 +0200 Subject: [PATCH] Rewrite builtin functions in rust --- fish-rust/src/builtins/functions.rs | 422 ++++++++++++++++++++++++++++ fish-rust/src/builtins/mod.rs | 1 + fish-rust/src/builtins/shared.rs | 1 + src/builtin.cpp | 5 +- src/builtin.h | 1 + tests/checks/functions.fish | 39 +++ tests/pexpects/generic.py | 5 + 7 files changed, 473 insertions(+), 1 deletion(-) create mode 100644 fish-rust/src/builtins/functions.rs diff --git a/fish-rust/src/builtins/functions.rs b/fish-rust/src/builtins/functions.rs new file mode 100644 index 000000000..8fc4c7618 --- /dev/null +++ b/fish-rust/src/builtins/functions.rs @@ -0,0 +1,422 @@ +use super::prelude::*; +use crate::common::escape_string; +use crate::common::reformat_for_screen; +use crate::common::valid_func_name; +use crate::common::{EscapeFlags, EscapeStringStyle}; +use crate::event::{self}; +use crate::ffi::colorize_shell; +use crate::function; +use crate::parser_keywords::parser_keywords_is_reserved; +use crate::termsize::termsize_last; + +struct FunctionsCmdOpts<'args> { + print_help: bool, + erase: bool, + list: bool, + show_hidden: bool, + query: bool, + copy: bool, + report_metadata: bool, + no_metadata: bool, + verbose: bool, + handlers: bool, + handlers_type: Option<&'args wstr>, + description: Option<&'args wstr>, +} + +impl Default for FunctionsCmdOpts<'_> { + fn default() -> Self { + Self { + print_help: false, + erase: false, + list: false, + show_hidden: false, + query: false, + copy: false, + report_metadata: false, + no_metadata: false, + verbose: false, + handlers: false, + handlers_type: None, + description: None, + } + } +} + +const NO_METADATA_SHORT: char = 2 as char; + +const SHORT_OPTIONS: &wstr = L!(":Ht:Dacd:ehnqv"); +#[rustfmt::skip] +const LONG_OPTIONS: &[woption] = &[ + wopt(L!("erase"), woption_argument_t::no_argument, 'e'), + wopt(L!("description"), woption_argument_t::required_argument, 'd'), + wopt(L!("names"), woption_argument_t::no_argument, 'n'), + wopt(L!("all"), woption_argument_t::no_argument, 'a'), + wopt(L!("help"), woption_argument_t::no_argument, 'h'), + wopt(L!("query"), woption_argument_t::no_argument, 'q'), + wopt(L!("copy"), woption_argument_t::no_argument, 'c'), + wopt(L!("details"), woption_argument_t::no_argument, 'D'), + wopt(L!("no-details"), woption_argument_t::no_argument, NO_METADATA_SHORT), + wopt(L!("verbose"), woption_argument_t::no_argument, 'v'), + wopt(L!("handlers"), woption_argument_t::no_argument, 'H'), + wopt(L!("handlers-type"), woption_argument_t::required_argument, 't'), +]; + +/// Parses options to builtin function, populating opts. +/// Returns an exit status. +fn parse_cmd_opts<'args>( + opts: &mut FunctionsCmdOpts<'args>, + optind: &mut usize, + argv: &mut [&'args wstr], + parser: &mut parser_t, + streams: &mut io_streams_t, +) -> Option { + let cmd = L!("function"); + let print_hints = false; + let mut w = wgetopter_t::new(SHORT_OPTIONS, LONG_OPTIONS, argv); + while let Some(opt) = w.wgetopt_long() { + match opt { + 'v' => opts.verbose = true, + 'e' => opts.erase = true, + 'D' => opts.report_metadata = true, + NO_METADATA_SHORT => opts.no_metadata = true, + 'd' => { + opts.description = Some(w.woptarg.unwrap()); + } + 'n' => opts.list = true, + 'a' => opts.show_hidden = true, + 'h' => opts.print_help = true, + 'q' => opts.query = true, + 'c' => opts.copy = true, + 'H' => opts.handlers = true, + 't' => { + opts.handlers = true; + opts.handlers_type = Some(w.woptarg.unwrap()); + } + ':' => { + builtin_missing_argument(parser, streams, cmd, argv[w.woptind - 1], print_hints); + return STATUS_INVALID_ARGS; + } + '?' => { + builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1], print_hints); + return STATUS_INVALID_ARGS; + } + other => { + panic!("Unexpected retval from wgetopt_long: {}", other); + } + } + } + + *optind = w.woptind; + STATUS_CMD_OK +} + +pub fn functions( + parser: &mut parser_t, + streams: &mut io_streams_t, + args: &mut [&wstr], +) -> Option { + let cmd = args[0]; + + let mut opts = FunctionsCmdOpts::default(); + let mut optind = 0; + let retval = parse_cmd_opts(&mut opts, &mut optind, args, parser, streams); + if retval != STATUS_CMD_OK { + return retval; + } + // Shadow our args with the positionals + let args = &args[optind..]; + + if opts.print_help { + builtin_print_error_trailer(parser, streams, cmd); + return STATUS_CMD_OK; + } + + let describe = opts.description.is_some(); + if [describe, opts.erase, opts.list, opts.query, opts.copy] + .into_iter() + .filter(|b| *b) + .count() + > 1 + { + streams.err.append(wgettext_fmt!(BUILTIN_ERR_COMBO, cmd)); + builtin_print_error_trailer(parser, streams, cmd); + return STATUS_INVALID_ARGS; + } + + if opts.report_metadata && opts.no_metadata { + streams.err.append(wgettext_fmt!(BUILTIN_ERR_COMBO, cmd)); + builtin_print_error_trailer(parser, streams, cmd); + return STATUS_INVALID_ARGS; + } + + if opts.erase { + for arg in args { + function::remove(arg); + } + // Historical - this never failed? + return STATUS_CMD_OK; + } + + if let Some(desc) = opts.description { + if args.len() != 1 { + streams.err.append(wgettext_fmt!( + "%ls: Expected exactly one function name\n", + cmd + )); + builtin_print_error_trailer(parser, streams, cmd); + return STATUS_INVALID_ARGS; + } + let current_func = args[0]; + + if !function::exists(current_func, parser) { + streams.err.append(wgettext_fmt!( + "%ls: Function '%ls' does not exist\n", + cmd, + current_func + )); + builtin_print_error_trailer(parser, streams, cmd); + return STATUS_CMD_ERROR; + } + + function::set_desc(current_func, desc.into(), parser); + return STATUS_CMD_OK; + } + + if opts.report_metadata { + if args.len() != 1 { + streams.err.append(wgettext_fmt!( + BUILTIN_ERR_ARG_COUNT2, + cmd, + // This error is + // functions: --details: expected 1 arguments; got 2 + // The "--details" was "argv[optind - 1]" in the C++ + // which would just give the last option. + // This is broken because you could do `functions --details --verbose foo bar`, and it would error about "--verbose". + "--details", + 1, + args.len() + )); + return STATUS_INVALID_ARGS; + } + let props = function::get_props_autoload(args[0], parser); + let def_file = if let Some(p) = props.as_ref() { + if let Some(cpf) = &p.copy_definition_file { + cpf.as_ref().to_owned() + } else if let Some(df) = &p.definition_file { + df.as_ref().to_owned() + } else { + L!("stdin").to_owned() + } + } else { + L!("n/a").to_owned() + }; + streams.out.append(def_file + L!("\n")); + + if opts.verbose { + let copy_place = match props.as_ref() { + Some(p) if p.copy_definition_file.is_some() => { + if let Some(df) = &p.definition_file { + df.as_ref().to_owned() + } else { + L!("stdin").to_owned() + } + } + Some(p) if p.is_autoload.load() => L!("autoloaded").to_owned(), + Some(p) if !p.is_autoload.load() => L!("not-autoloaded").to_owned(), + _ => L!("n/a").to_owned(), + }; + streams.out.append(copy_place + L!("\n")); + let line = if let Some(p) = props.as_ref() { + p.definition_lineno() + } else { + 0 + }; + streams.out.append(sprintf!("%d\n", line)); + + let shadow = match props.as_ref() { + Some(p) if p.shadow_scope => L!("scope-shadowing").to_owned(), + Some(p) if !p.shadow_scope => L!("no-scope-shadowing").to_owned(), + _ => L!("n/a").to_owned(), + }; + streams.out.append(shadow + L!("\n")); + + let desc = match props.as_ref() { + Some(p) if !p.description.is_empty() => escape_string( + &p.description, + EscapeStringStyle::Script(EscapeFlags::NO_PRINTABLES | EscapeFlags::NO_QUOTED), + ), + Some(p) if p.description.is_empty() => L!("").to_owned(), + _ => L!("n/a").to_owned(), + }; + streams.out.append(desc + L!("\n")); + } + // Historical - this never failed? + return STATUS_CMD_OK; + } + + if opts.handlers { + // Empty handlers-type is the same as "all types". + if !opts.handlers_type.unwrap_or(L!("")).is_empty() + && !event::EVENT_FILTER_NAMES.contains(&opts.handlers_type.unwrap()) + { + streams.err.append(wgettext_fmt!( + "%ls: Expected generic | variable | signal | exit | job-id for --handlers-type\n", + cmd + )); + return STATUS_INVALID_ARGS; + } + event::print(streams, opts.handlers_type.unwrap_or(L!(""))); + return STATUS_CMD_OK; + } + + if opts.query && args.is_empty() { + return STATUS_CMD_ERROR; + } + + if opts.list || args.is_empty() { + let mut names = function::get_names(opts.show_hidden); + names.sort(); + if streams.out_is_terminal() { + let mut buff = WString::new(); + let mut first: bool = true; + for name in names { + if !first { + buff.push_utfstr(L!(", ")); + } + buff.push_utfstr(&name); + first = false; + } + streams + .out + .append(reformat_for_screen(&buff, &termsize_last())); + } else { + for name in names { + streams.out.append(name + "\n"); + } + } + return STATUS_CMD_OK; + } + + if opts.copy { + if args.len() != 2 { + streams.err.append(wgettext_fmt!( + "%ls: Expected exactly two names (current function name, and new function name)\n", + cmd + )); + builtin_print_error_trailer(parser, streams, cmd); + return STATUS_INVALID_ARGS; + } + let current_func = args[0]; + let new_func = args[1]; + + if !function::exists(current_func, parser) { + streams.err.append(wgettext_fmt!( + "%ls: Function '%ls' does not exist\n", + cmd, + current_func + )); + builtin_print_error_trailer(parser, streams, cmd); + return STATUS_CMD_ERROR; + } + + if !valid_func_name(new_func) || parser_keywords_is_reserved(new_func) { + streams.err.append(wgettext_fmt!( + "%ls: Illegal function name '%ls'\n", + cmd, + new_func + )); + builtin_print_error_trailer(parser, streams, cmd); + return STATUS_INVALID_ARGS; + } + + if function::exists(new_func, parser) { + streams.err.append(wgettext_fmt!( + "%ls: Function '%ls' already exists. Cannot create copy '%ls'\n", + cmd, + new_func, + current_func + )); + builtin_print_error_trailer(parser, streams, cmd); + return STATUS_CMD_ERROR; + } + if function::copy(current_func, new_func.into(), parser) { + return STATUS_CMD_OK; + } + return STATUS_CMD_ERROR; + } + + let mut res: c_int = STATUS_CMD_OK.unwrap(); + + let mut first = true; + for arg in args.iter() { + let Some(props) = function::get_props_autoload(arg, parser) else { + res += 1; + first = false; + continue; + }; + if opts.query { + continue; + } + if !first { + streams.out.append(L!("\n")); + }; + + let mut comment = WString::new(); + if !opts.no_metadata { + // TODO: This is duplicated in type. + // Extract this into a helper. + match props.definition_file() { + Some(path) if path == "-" => { + comment.push_utfstr(&wgettext!("Defined via `source`")) + } + Some(path) => { + comment.push_utfstr(&wgettext_fmt!( + "Defined in %ls @ line %d", + path, + props.definition_lineno() + )); + } + None => comment.push_utfstr(&wgettext_fmt!("Defined interactively")), + } + + if props.is_copy() { + match props.copy_definition_file() { + Some(path) if path == "-" => { + comment.push_utfstr(&wgettext_fmt!(", copied via `source`")) + } + Some(path) => { + comment.push_utfstr(&wgettext_fmt!( + ", copied in %ls @ line %d", + path, + props.copy_definition_lineno() + )); + } + None => comment.push_utfstr(&wgettext_fmt!(", copied interactively")), + } + } + } + + let mut def = WString::new(); + + if !comment.is_empty() { + def.push_utfstr(&sprintf!( + "# %ls\n%ls", + comment, + props.annotated_definition(arg) + )); + } else { + def = props.annotated_definition(arg); + } + + if streams.out_is_terminal() { + let col = colorize_shell(&def.to_ffi(), parser.pin()).from_ffi(); + streams.out.append(col); + } else { + streams.out.append(def); + } + first = false; + } + + return Some(res); +} diff --git a/fish-rust/src/builtins/mod.rs b/fish-rust/src/builtins/mod.rs index 110db8258..bc103161c 100644 --- a/fish-rust/src/builtins/mod.rs +++ b/fish-rust/src/builtins/mod.rs @@ -12,6 +12,7 @@ pub mod echo; pub mod emit; pub mod exit; pub mod function; +pub mod functions; pub mod math; pub mod path; pub mod printf; diff --git a/fish-rust/src/builtins/shared.rs b/fish-rust/src/builtins/shared.rs index 1bfc2b6b1..f27cb43b9 100644 --- a/fish-rust/src/builtins/shared.rs +++ b/fish-rust/src/builtins/shared.rs @@ -237,6 +237,7 @@ pub fn run_builtin( RustBuiltin::Echo => super::echo::echo(parser, streams, args), RustBuiltin::Emit => super::emit::emit(parser, streams, args), RustBuiltin::Exit => super::exit::exit(parser, streams, args), + RustBuiltin::Functions => super::functions::functions(parser, streams, args), RustBuiltin::Math => super::math::math(parser, streams, args), RustBuiltin::Path => super::path::path(parser, streams, args), RustBuiltin::Pwd => super::pwd::pwd(parser, streams, args), diff --git a/src/builtin.cpp b/src/builtin.cpp index f07f41e95..790b7fae3 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -373,7 +373,7 @@ static constexpr builtin_data_t builtin_datas[] = { {L"fg", &builtin_fg, N_(L"Send job to foreground")}, {L"for", &builtin_generic, N_(L"Perform a set of commands multiple times")}, {L"function", &builtin_generic, N_(L"Define a new function")}, - {L"functions", &builtin_functions, N_(L"List or remove functions")}, + {L"functions", &implemented_in_rust, N_(L"List or remove functions")}, {L"history", &builtin_history, N_(L"History of commands executed by user")}, {L"if", &builtin_generic, N_(L"Evaluate block if condition is true")}, {L"jobs", &builtin_jobs, N_(L"Print currently running jobs")}, @@ -549,6 +549,9 @@ static maybe_t try_get_rust_builtin(const wcstring &cmd) { if (cmd == L"exit") { return RustBuiltin::Exit; } + if (cmd == L"functions") { + return RustBuiltin::Functions; + } if (cmd == L"math") { return RustBuiltin::Math; } diff --git a/src/builtin.h b/src/builtin.h index 7a0cc3208..e084a90f5 100644 --- a/src/builtin.h +++ b/src/builtin.h @@ -123,6 +123,7 @@ enum class RustBuiltin : int32_t { Echo, Emit, Exit, + Functions, Math, Path, Printf, diff --git a/tests/checks/functions.fish b/tests/checks/functions.fish index f5f0dae1d..2d4d35154 100644 --- a/tests/checks/functions.fish +++ b/tests/checks/functions.fish @@ -9,6 +9,10 @@ end functions --details f1 f2 #CHECKERR: functions: --details: expected 1 arguments; got 2 +# Verify that it still mentions "--details" even if it isn't the last option. +functions --details --verbose f1 f2 +#CHECKERR: functions: --details: expected 1 arguments; got 2 + # ========== # Verify that `functions --details` works as expected when given the name of a # known function. @@ -185,3 +189,38 @@ functions --handlers-type signal # CHECK: SIGTERM term1 # CHECK: SIGTERM term2 # CHECK: SIGTERM term3 + +# See how --names and --all work. +# We don't want to list all of our functions here, +# so we just match a few that we know are there. +functions -n | string match cd +# CHECK: cd + +functions --names | string match __fish_config_interactive +echo $status +# CHECK: 1 + +functions --names -a | string match __fish_config_interactive +# CHECK: __fish_config_interactive + +functions --description "" +# CHECKERR: functions: Expected exactly one function name +# CHECKERR: checks/functions.fish (line {{\d+}}): +# CHECKERR: functions --description "" +# CHECKERR: ^ +# CHECKERR: (Type 'help functions' for related documentation) + +function foo --on-variable foo; end +# This should print *everything* +functions --handlers-type "" | string match 'Event *' +# CHECK: Event signal +# CHECK: Event variable +# CHECK: Event generic +functions -e foo + +functions --details --verbose thisfunctiondoesnotexist +# CHECK: n/a +# CHECK: n/a +# CHECK: 0 +# CHECK: n/a +# CHECK: n/a diff --git a/tests/pexpects/generic.py b/tests/pexpects/generic.py index 5dd0ed618..8f509aa76 100644 --- a/tests/pexpects/generic.py +++ b/tests/pexpects/generic.py @@ -64,3 +64,8 @@ expect_str("# Defined interactively\r\n") expect_str("function foo") expect_str("end") expect_prompt() + +# See that `functions` terminates +sendline("functions") +expect_re(".*fish_prompt,.*") +expect_prompt()