Relax history autosuggestion and highlighting if cd is wrapped

For implementation reasons, we special-case cd in several ways
1. it gets different completions (handle_as_special_cd)
2. when highlighting, we honor CDPATH
3. we discard autosuggestions from history that don't have valid path arguments

There are some third-party tools like zoxide that redefine cd ("function cd
--wraps ...; ...; end"). We can't support this in general but let's try to
make an effort.

zoxide tries to be a superset of cd, so special case 1 is still
valid but 2 and 3 are not, because zoxide accepts some paths
that cd doesn't accept.

Let's add a hack to detect when "cd" actually means something else by checking
if there is any --wraps argument.

A cleaner solution is definitely possible but more effort.

Closes #10719
This commit is contained in:
Johannes Altmanninger 2024-09-14 08:02:07 +02:00
parent 5b04f221a3
commit 5432ee1aa9
4 changed files with 21 additions and 9 deletions

View file

@ -17,7 +17,7 @@ fish 3.8.0 (released ???)
10308 10321 10338 10348 10349 10355 10357 10379 10381 10388 10389 10390 10395 10398 10400 10403 10308 10321 10338 10348 10349 10355 10357 10379 10381 10388 10389 10390 10395 10398 10400 10403
10404 10407 10408 10409 10411 10412 10417 10418 10427 10429 10438 10439 10440 10441 10442 10443 10404 10407 10408 10409 10411 10412 10417 10418 10427 10429 10438 10439 10440 10441 10442 10443
10445 10448 10450 10451 10456 10457 10462 10463 10464 10466 10467 10474 10481 10490 10492 10494 10445 10448 10450 10451 10456 10457 10462 10463 10464 10466 10467 10474 10481 10490 10492 10494
10499 10503 10505 10508 10509 10510 10511 10512 10513 10518 10547 10499 10503 10505 10508 10509 10510 10511 10512 10513 10518 10547 10719
The entirety of fish's C++ code has been ported to Rust (:issue:`9512`). The entirety of fish's C++ code has been ported to Rust (:issue:`9512`).
This means a large change in dependencies and how to build fish. This means a large change in dependencies and how to build fish.

View file

@ -4,7 +4,7 @@ use std::{
mem, mem,
sync::{ sync::{
atomic::{self, AtomicUsize}, atomic::{self, AtomicUsize},
Mutex, Mutex, MutexGuard,
}, },
time::{Duration, Instant}, time::{Duration, Instant},
}; };
@ -2448,7 +2448,7 @@ pub fn complete_load(cmd: &wstr, parser: &Parser) -> bool {
// See issue #2466. // See issue #2466.
if function::load(cmd, parser) { if function::load(cmd, parser) {
// We autoloaded something; check if we have a --wraps. // We autoloaded something; check if we have a --wraps.
loaded_new |= !complete_get_wrap_targets(cmd).is_empty(); loaded_new |= complete_wrap_map().get(cmd).is_some();
} }
// It's important to NOT hold the lock around completion loading. // It's important to NOT hold the lock around completion loading.
@ -2564,6 +2564,11 @@ pub fn complete_remove_wrapper(command: WString, target_to_remove: &wstr) -> boo
result result
} }
/// Returns a list of wrap targets for a given command.
pub fn complete_wrap_map() -> MutexGuard<'static, HashMap<WString, Vec<WString>>> {
wrapper_map.lock().unwrap()
}
/// Returns a list of wrap targets for a given command. /// Returns a list of wrap targets for a given command.
pub fn complete_get_wrap_targets(command: &wstr) -> Vec<WString> { pub fn complete_get_wrap_targets(command: &wstr) -> Vec<WString> {
if command.is_empty() { if command.is_empty() {

View file

@ -5,7 +5,7 @@
use crate::ast::{self, Node}; use crate::ast::{self, Node};
use crate::autoload::Autoload; use crate::autoload::Autoload;
use crate::common::{assert_sync, escape, valid_func_name, FilenameRef}; use crate::common::{assert_sync, escape, valid_func_name, FilenameRef};
use crate::complete::complete_get_wrap_targets; use crate::complete::complete_wrap_map;
use crate::env::{EnvStack, Environment}; use crate::env::{EnvStack, Environment};
use crate::event::{self, EventDescription}; use crate::event::{self, EventDescription};
use crate::global_safety::RelaxedAtomicBool; use crate::global_safety::RelaxedAtomicBool;
@ -416,9 +416,11 @@ impl FunctionProperties {
} }
// Output wrap targets. // Output wrap targets.
for wrap in complete_get_wrap_targets(name) { if let Some(wrapped_cmds) = complete_wrap_map().get(name) {
out.push_str(" --wraps="); for wrapped_cmd in wrapped_cmds {
out.push_utfstr(&escape(&wrap)); out.push_str(" --wraps=");
out.push_utfstr(&escape(wrapped_cmd));
}
} }
if !desc.is_empty() { if !desc.is_empty() {

View file

@ -10,6 +10,7 @@ use crate::common::{
unescape_string, valid_var_name, valid_var_name_char, UnescapeFlags, ASCII_MAX, unescape_string, valid_var_name, valid_var_name_char, UnescapeFlags, ASCII_MAX,
EXPAND_RESERVED_BASE, EXPAND_RESERVED_END, EXPAND_RESERVED_BASE, EXPAND_RESERVED_END,
}; };
use crate::complete::complete_wrap_map;
use crate::env::Environment; use crate::env::Environment;
use crate::expand::{ use crate::expand::{
expand_one, expand_tilde, expand_to_command_and_args, ExpandFlags, ExpandResultCode, expand_one, expand_tilde, expand_to_command_and_args, ExpandFlags, ExpandResultCode,
@ -316,6 +317,10 @@ fn autosuggest_parse_command(
None None
} }
pub fn is_veritable_cd(expanded_command: &wstr) -> bool {
expanded_command == L!("cd") && complete_wrap_map().get(L!("cd")).is_none()
}
/// Given an item `item` from the history which is a proposed autosuggestion, return whether the /// Given an item `item` from the history which is a proposed autosuggestion, return whether the
/// autosuggestion is valid. It may not be valid if e.g. it is attempting to cd into a directory /// autosuggestion is valid. It may not be valid if e.g. it is attempting to cd into a directory
/// which does not exist. /// which does not exist.
@ -333,7 +338,7 @@ pub fn autosuggest_validate_from_history(
}; };
// We handle cd specially. // We handle cd specially.
if parsed_command == "cd" && !cd_dir.is_empty() { if is_veritable_cd(&parsed_command) && !cd_dir.is_empty() {
if expand_one(&mut cd_dir, ExpandFlags::FAIL_ON_CMDSUBST, ctx, None) { if expand_one(&mut cd_dir, ExpandFlags::FAIL_ON_CMDSUBST, ctx, None) {
if string_prefixes_string(&cd_dir, L!("--help")) if string_prefixes_string(&cd_dir, L!("--help"))
|| string_prefixes_string(&cd_dir, L!("-h")) || string_prefixes_string(&cd_dir, L!("-h"))
@ -1354,7 +1359,7 @@ impl<'s> Highlighter<'s> {
// Color arguments and redirections. // Color arguments and redirections.
// Except if our command is 'cd' we have special logic for how arguments are colored. // Except if our command is 'cd' we have special logic for how arguments are colored.
let is_cd = expanded_cmd == "cd"; let is_cd = is_veritable_cd(&expanded_cmd);
let mut is_set = expanded_cmd == "set"; let mut is_set = expanded_cmd == "set";
// If we have seen a "--" argument, color all options from then on as normal arguments. // If we have seen a "--" argument, color all options from then on as normal arguments.
let mut have_dashdash = false; let mut have_dashdash = false;