From 5432ee1aa95d9fdd4eae6eb4ff0a723daa0fc4f3 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 14 Sep 2024 08:02:07 +0200 Subject: [PATCH] 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 --- CHANGELOG.rst | 2 +- src/complete.rs | 9 +++++++-- src/function.rs | 10 ++++++---- src/highlight.rs | 9 +++++++-- 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b7b730ad2..44a0be74b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -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 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 - 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`). This means a large change in dependencies and how to build fish. diff --git a/src/complete.rs b/src/complete.rs index cc13efc31..99833adbe 100644 --- a/src/complete.rs +++ b/src/complete.rs @@ -4,7 +4,7 @@ use std::{ mem, sync::{ atomic::{self, AtomicUsize}, - Mutex, + Mutex, MutexGuard, }, time::{Duration, Instant}, }; @@ -2448,7 +2448,7 @@ pub fn complete_load(cmd: &wstr, parser: &Parser) -> bool { // See issue #2466. if function::load(cmd, parser) { // 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. @@ -2564,6 +2564,11 @@ pub fn complete_remove_wrapper(command: WString, target_to_remove: &wstr) -> boo result } +/// Returns a list of wrap targets for a given command. +pub fn complete_wrap_map() -> MutexGuard<'static, HashMap>> { + wrapper_map.lock().unwrap() +} + /// Returns a list of wrap targets for a given command. pub fn complete_get_wrap_targets(command: &wstr) -> Vec { if command.is_empty() { diff --git a/src/function.rs b/src/function.rs index 8b29d9f53..6f4408dd3 100644 --- a/src/function.rs +++ b/src/function.rs @@ -5,7 +5,7 @@ use crate::ast::{self, Node}; use crate::autoload::Autoload; 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::event::{self, EventDescription}; use crate::global_safety::RelaxedAtomicBool; @@ -416,9 +416,11 @@ impl FunctionProperties { } // Output wrap targets. - for wrap in complete_get_wrap_targets(name) { - out.push_str(" --wraps="); - out.push_utfstr(&escape(&wrap)); + if let Some(wrapped_cmds) = complete_wrap_map().get(name) { + for wrapped_cmd in wrapped_cmds { + out.push_str(" --wraps="); + out.push_utfstr(&escape(wrapped_cmd)); + } } if !desc.is_empty() { diff --git a/src/highlight.rs b/src/highlight.rs index f5031a709..356ca941c 100644 --- a/src/highlight.rs +++ b/src/highlight.rs @@ -10,6 +10,7 @@ use crate::common::{ unescape_string, valid_var_name, valid_var_name_char, UnescapeFlags, ASCII_MAX, EXPAND_RESERVED_BASE, EXPAND_RESERVED_END, }; +use crate::complete::complete_wrap_map; use crate::env::Environment; use crate::expand::{ expand_one, expand_tilde, expand_to_command_and_args, ExpandFlags, ExpandResultCode, @@ -316,6 +317,10 @@ fn autosuggest_parse_command( 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 /// autosuggestion is valid. It may not be valid if e.g. it is attempting to cd into a directory /// which does not exist. @@ -333,7 +338,7 @@ pub fn autosuggest_validate_from_history( }; // 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 string_prefixes_string(&cd_dir, L!("--help")) || string_prefixes_string(&cd_dir, L!("-h")) @@ -1354,7 +1359,7 @@ impl<'s> Highlighter<'s> { // Color arguments and redirections. // 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"; // If we have seen a "--" argument, color all options from then on as normal arguments. let mut have_dashdash = false;