From ce9f95128a27749b13a3049fc0dd3f0112fd0010 Mon Sep 17 00:00:00 2001 From: David Adam Date: Fri, 30 Jun 2023 05:39:03 +0800 Subject: [PATCH] type/command: implement optimisation for --all This was present in the C++ version for command, though never for type. Checking over all elements of PATH can be slow on some platforms eg WSL2, so only do that when used with `--all`. Based on discussion in https://github.com/fish-shell/fish-shell/pull/9856 --- fish-rust/src/builtins/command.rs | 16 ++++++++++------ fish-rust/src/builtins/type.rs | 11 +++++++++-- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/fish-rust/src/builtins/command.rs b/fish-rust/src/builtins/command.rs index 89ffed8e6..3aaf314f7 100644 --- a/fish-rust/src/builtins/command.rs +++ b/fish-rust/src/builtins/command.rs @@ -5,8 +5,8 @@ use crate::builtins::shared::{ STATUS_CMD_OK, STATUS_CMD_UNKNOWN, STATUS_INVALID_ARGS, }; use crate::ffi::parser_t; -use crate::path::path_get_paths; -use crate::wchar::{wstr, WString, L}; +use crate::path::{path_get_path, path_get_paths}; +use crate::wchar::{wstr, L}; use crate::wgetopt::{wgetopter_t, wopt, woption, woption_argument_t}; use crate::wutil::sprintf; @@ -71,10 +71,14 @@ pub fn r#command( let mut res = false; let optind = w.woptind; for arg in argv.iter().take(argc).skip(optind) { - // TODO: This always gets all paths, and then skips a bunch. - // For the common case, we want to get just the one path. - // Port this over once path.cpp is. - let paths: Vec = path_get_paths(arg, &*parser.get_vars()); + let paths = if opts.all { + path_get_paths(arg, &*parser.get_vars()) + } else { + match path_get_path(arg, &*parser.get_vars()) { + Some(p) => vec![p], + None => vec![], + } + }; for path in paths.iter() { res = true; diff --git a/fish-rust/src/builtins/type.rs b/fish-rust/src/builtins/type.rs index 2e6553cee..7cc392d8b 100644 --- a/fish-rust/src/builtins/type.rs +++ b/fish-rust/src/builtins/type.rs @@ -14,7 +14,7 @@ use crate::ffi::{ function_get_definition_file, function_get_definition_lineno, function_get_props_autoload, function_is_copy, }; -use crate::path::path_get_paths; +use crate::path::{path_get_path, path_get_paths}; use crate::wchar::{wstr, WString, L}; use crate::wchar_ffi::WCharFromFFI; use crate::wchar_ffi::WCharToFFI; @@ -190,7 +190,14 @@ pub fn r#type( } } - let paths: Vec = path_get_paths(arg, &*parser.get_vars()); + let paths = if opts.all { + path_get_paths(arg, &*parser.get_vars()) + } else { + match path_get_path(arg, &*parser.get_vars()) { + Some(p) => vec![p], + None => vec![], + } + }; for path in paths.iter() { found += 1;