cygwin: improve handling of .exe file extension

- Prefer the command name without `.exe` since the extension is optional
when launching application on Windows...
- ... but if the user started to type the extension, then use it.
- If there is no description and/or completion for `foo.exe` then
use those for `foo`

Closes #12100

CHANGELOG.rst

@@ -12,6 +12,8 @@ Interactive improvements
 - When typing immediately after starting fish, the first prompt is now rendered correctly.
 - Completion accuracy was improved for file paths containing ``=`` or ``:`` (:issue:`5363`).
 - Prefix-matching completions are now shown even if they don't have the case typed by the user (:issue:`7944`).
+- On Cygwin/MSYS, command name completion will favor the non-exe name (``foo``) unless the user started typing the extension
+- When using the exe name (``foo.exe``), fish will use to the description and completions for ``foo`` if there are none for ``foo.exe``
 
 Improved terminal support
 -------------------------

doc_src/cmds/complete.rst

@@ -103,6 +103,31 @@ When erasing completions, it is possible to either erase all completions for a s
 
 When ``complete`` is called without anything that would define or erase completions (options, arguments, wrapping, ...), it shows matching completions instead. So ``complete`` without any arguments shows all loaded completions, ``complete -c foo`` shows all loaded completions for ``foo``. Since completions are :ref:`autoloaded <syntax-function-autoloading>`, you will have to trigger them first.
 
+.. _completions-cygwin:
+
+Cygwin / MSYS2 / Windows
+------------------------
+
+On Windows, binary executables have a ``.exe`` extension, but this extension is not required when calling an application (and if the name is not ambiguous, i.e. there isn't also a script called ``myprog`` in the same directory as ``myprog.exe``).
+
+To unify completions between Windows and other OSes, on Cygwin/MSYS2/Windows, *COMMAND* does not require the ``.exe`` extension.
+Completions for ``myprog`` will also be used for ``myprog.exe`` if there are no ambiguities, i.e. if there are no completions for ``myprog.exe`` specifically.
+However, completions for ``myprog.exe`` will only be used when also using the ``.exe`` extension on the command line.
+
+In other words:
+
+::
+
+    complete -c myprog.exe ...  #1
+
+will only work for ``myprog.exe``
+
+::
+
+    complete -c myprog ...  #2
+
+can work for both ``myprog`` and ``myprog.exe``. But if both completions exist, #2 will only be used for ``myprog`` while ``myprog.exe`` will use #1.
+
 Examples
 --------
 

doc_src/completions.rst

@@ -1,7 +1,7 @@
 Writing your own completions
 ============================
 
-To specify a completion, use the ``complete`` command. ``complete`` takes as a parameter the name of the command to specify a completion for. For example, to add a completion for the program ``myprog``, start the completion command with ``complete -c myprog ...``
+To specify a completion, use the ``complete`` command. ``complete`` takes as a parameter the name of the command to specify a completion for. For example, to add a completion for the program ``myprog`` (or ``myprog.exe`` on :ref:`Cygwin/MSYS2 <completions-cygwin>`), start the completion command with ``complete -c myprog ...``
 
 For a complete description of the various switches accepted by the ``complete`` command, see the documentation for the :doc:`complete <cmds/complete>` builtin, or write ``complete --help`` inside the ``fish`` shell.
 
@@ -160,4 +160,3 @@ This wide search may be confusing. If you are unsure, your completions probably
 If you have written new completions for a common Unix command, please consider sharing your work by submitting it via the instructions in :ref:`Further help and development <more-help>`.
 
 If you are developing another program and would like to ship completions with your program, install them to the "vendor" completions directory. As this path may vary from system to system, the ``pkgconfig`` framework should be used to discover this path with the output of ``pkg-config --variable completionsdir fish``.
-

share/functions/__fish_describe_command.fish

@@ -11,9 +11,14 @@ if not type -q apropos
 end
 
 function __fish_describe_command -d "Command used to find descriptions for commands"
+    argparse exact -- $argv
+    or return 1
+    set -l suffix
+    set -q _flag_exact && set suffix '$'
+
     # $argv will be inserted directly into the awk regex, so it must be escaped
     set -l argv_regex (string escape --style=regex -- "$argv")
-    __fish_apropos ^$argv 2>/dev/null | awk -v FS=" +- +" '{
+    __fish_apropos "^$argv$suffix" 2>/dev/null | awk -v FS=" +- +" '{
 		split($1, names, ", ");
 		for (name in names)
 			if (names[name] ~ /^'"$argv_regex"'.* *\([18]\)/ ) {

src/autoload.rs

@@ -57,7 +57,7 @@ pub enum AutoloadPath {
     Path(WString),
 }
 
-enum AutoloadResult {
+pub enum AutoloadResult {
     Path(AutoloadPath),
     Loaded,
     Pending,
@@ -91,33 +91,30 @@ pub fn new(env_var_name: &'static wstr) -> Self {
     /// After returning a path, the command is marked in-progress until the caller calls
     /// mark_autoload_finished() with the same command. Note this does not actually execute any
     /// code; it is the caller's responsibility to load the file.
-    pub fn resolve_command(&mut self, cmd: &wstr, env: &dyn Environment) -> Option<AutoloadPath> {
-        match self.resolve_command_impl(
+    pub fn resolve_command(&mut self, cmd: &wstr, env: &dyn Environment) -> AutoloadResult {
+        let result = self.resolve_command_impl(
             cmd,
             env.get(self.env_var_name)
                 .as_ref()
                 .map(|var| var.as_list())
                 .unwrap_or_default(),
-        ) {
-            AutoloadResult::Path(path) => {
-                match &path {
-                    AutoloadPath::Embedded(_) => {
-                        FLOGF!(autoload, "Embedded: %s", cmd);
-                    }
-                    AutoloadPath::Path(path) => {
-                        FLOGF!(
-                            autoload,
-                            "Loading %s from var %s from path %s",
-                            cmd,
-                            self.env_var_name,
-                            path
-                        )
-                    }
-                }
-                Some(path)
+        );
+        match result {
+            AutoloadResult::Path(AutoloadPath::Embedded(_)) => {
+                FLOGF!(autoload, "Embedded: %s", cmd);
             }
-            AutoloadResult::Loaded | AutoloadResult::Pending | AutoloadResult::None => None,
-        }
+            AutoloadResult::Path(AutoloadPath::Path(ref path)) => {
+                FLOGF!(
+                    autoload,
+                    "Loading %s from var %s from path %s",
+                    cmd,
+                    self.env_var_name,
+                    path
+                );
+            }
+            AutoloadResult::Loaded | AutoloadResult::Pending | AutoloadResult::None => {}
+        };
+        result
     }
 
     /// Helper to actually perform an autoload.

src/complete.rs

@@ -11,9 +11,11 @@
 
 use crate::{
     ast::unescape_keyword,
+    autoload::AutoloadResult,
     common::charptr2wcstring,
     reader::{get_quote, is_backslashed},
     util::wcsfilecmp,
+    wcstringutil::{string_suffixes_string_case_insensitive, strip_executable_suffix},
     wutil::{LocalizableString, localizable_string},
 };
 use bitflags::bitflags;
@@ -988,16 +990,26 @@ fn complete_cmd_desc(&mut self, s: &wstr) {
             return;
         }
 
-        let lookup_cmd: WString = [
-            L!("functions -q __fish_describe_command && __fish_describe_command "),
-            &escape(cmd),
-        ]
-        .into_iter()
-        .collect();
+        // On Cygwin, if `cmd` contains part of the `.exe` extension (e.g. `lsmod.e`), we are unlikely
+        // to find a description since they are usually associated to the POSIX name (`lsmod`). So we also
+        // need to search for the stripped command (`lsmod`), and later associate the description to
+        // the missing part of the extension (`xe`)
+        let no_exe = strip_partial_executable_suffix(cmd);
 
         // First locate a list of possible descriptions using a single call to apropos or a direct
         // search if we know the location of the whatis database. This can take some time on slower
         // systems with a large set of manuals, but it should be ok since apropos is only called once.
+        // For Cygwin, also try to find the exact match for the non-exe name
+        let lookup_cmd = sprintf!(
+            "functions -q __fish_describe_command &&{ __fish_describe_command %s %s}",
+            &escape(cmd),
+            &no_exe
+                .map(|(cmd_sans_exe, _)| {
+                    sprintf!("; __fish_describe_command --exact %s", escape(cmd_sans_exe))
+                })
+                .unwrap_or_default()[..]
+        );
+
         let mut list = vec![];
         let _ = exec_subshell(
             &lookup_cmd,
@@ -1011,17 +1023,12 @@ fn complete_cmd_desc(&mut self, s: &wstr) {
         let mut lookup = BTreeMap::new();
         // A typical entry is the command name, followed by a tab, followed by a description.
         for elstr in &mut list {
-            // Skip keys that are too short.
-            if elstr.len() < cmd.len() {
-                continue;
-            }
-
-            // Skip cases without a tab, or without a description, or bizarre cases where the tab is
-            // part of the command.
+            // Skip cases without a tab, or without a description
+            // Bizarre cases where the tab is part of the command will be filtered later.
             let Some(tab_idx) = elstr.find_char('\t') else {
                 continue;
             };
-            if tab_idx + 1 >= elstr.len() || tab_idx < cmd.len() {
+            if tab_idx + 1 >= elstr.len() {
                 continue;
             }
 
@@ -1033,8 +1040,16 @@ fn complete_cmd_desc(&mut self, s: &wstr) {
             //  val = A description
             // Note an empty key is common and natural, if 'cmd' were already valid.
             let parts = elstr.as_mut_utfstr().split_at_mut(tab_idx);
-            let key = &parts.0[cmd.len()..tab_idx];
-            let (_, val) = parts.1.split_at_mut(1);
+            let key = if parts.0.len() >= cmd.len() {
+                &parts.0[cmd.len()..]
+            } else if let Some((_, comp)) = no_exe.filter(|(stripped, _)| stripped == parts.0) {
+                // On Cygwin, `cmd` might be `lsmod.e`, then key needs to be `xe`, while
+                // elstr is `lsmod\t...` (i.e. parts.0 is `lsmod`)
+                comp
+            } else {
+                continue;
+            };
+            let val = &mut parts.1[1..];
 
             // And once again I make sure the first character is uppercased because I like it that
             // way, and I get to decide these things.
@@ -1257,7 +1272,15 @@ fn complete_param_for_command(
             .iter()
             .filter_map(|(idx, completion)| {
                 let r#match = if idx.is_path { &path } else { &cmd };
-                if wildcard_match(r#match, &idx.name, false) {
+                let has_match = wildcard_match(r#match, &idx.name, false)
+                    || (
+                        // On cygwin, if we didn't have a completion for "foo.exe",
+                        // check if there is one for "foo"
+                        !idx.is_path
+                            && strip_executable_suffix(r#match)
+                                .is_some_and(|stripped| wildcard_match(stripped, &idx.name, false))
+                    );
+                if has_match {
                     // Copy all of their options into our list. Oof, this is a lot of copying.
                     let mut options = completion.get_options().to_vec();
                     // We have to copy them in reverse order to preserve legacy behavior (#9221).
@@ -2418,6 +2441,26 @@ fn completion2string(index: &CompletionEntryIndex, o: &CompleteEntryOpt) -> WStr
     out
 }
 
+/// If the cmd contains a partial executable extension, return the stripped
+/// command and missing part of the full extension.
+/// E.g. `cmd.e` -> `Some(("cmd", "xe"))``
+fn strip_partial_executable_suffix(cmd: &wstr) -> Option<(&wstr, &wstr)> {
+    if !cfg!(cygwin) {
+        return None;
+    }
+
+    [
+        // (<cmd suffix>, <completion for full ".exe">)
+        (L!(".exe"), L!("")),
+        (L!(".ex"), L!("e")),
+        (L!(".e"), L!("xe")),
+        (L!("."), L!("exe")),
+    ]
+    .into_iter()
+    .find(|(ext, _)| string_suffixes_string_case_insensitive(ext, cmd))
+    .map(|(ext, comp)| (&cmd[0..cmd.len() - ext.len()], comp))
+}
+
 /// Load command-specific completions for the specified command.
 /// Returns `true` if something new was loaded, `false` if not.
 pub fn complete_load(cmd: &wstr, parser: &Parser) -> bool {
@@ -2442,13 +2485,22 @@ pub fn complete_load(cmd: &wstr, parser: &Parser) -> bool {
         .lock()
         .expect("mutex poisoned")
         .resolve_command(cmd, EnvStack::globals());
-    if let Some(path_to_load) = path_to_load {
-        Autoload::perform_autoload(&path_to_load, parser);
-        completion_autoloader
-            .lock()
-            .expect("mutex poisoned")
-            .mark_autoload_finished(cmd);
-        loaded_new = true;
+    match path_to_load {
+        AutoloadResult::Path(path_to_load) => {
+            Autoload::perform_autoload(&path_to_load, parser);
+            completion_autoloader
+                .lock()
+                .expect("mutex poisoned")
+                .mark_autoload_finished(cmd);
+            loaded_new = true;
+        }
+        AutoloadResult::None => {
+            // On Cygwin, if we failed to find a completion for "foo.exe", try "foo"
+            if let Some(stripped) = strip_executable_suffix(cmd) {
+                loaded_new = complete_load(stripped, parser);
+            }
+        }
+        AutoloadResult::Loaded | AutoloadResult::Pending => {}
     }
     loaded_new
 }

src/function.rs

@@ -3,7 +3,7 @@
 // the parser and to some degree the builtin handling library.
 
 use crate::ast::{self, Node};
-use crate::autoload::Autoload;
+use crate::autoload::{Autoload, AutoloadResult};
 use crate::common::{FilenameRef, assert_sync, escape, valid_func_name};
 use crate::complete::complete_wrap_map;
 use crate::env::{EnvStack, Environment};
@@ -117,7 +117,7 @@ pub fn load(name: &wstr, parser: &Parser) -> bool {
     {
         let mut funcset: std::sync::MutexGuard<FunctionSet> = FUNCTION_SET.lock().unwrap();
         if funcset.allow_autoload(name) {
-            if let Some(path) = funcset
+            if let AutoloadResult::Path(path) = funcset
                 .autoloader
                 .resolve_command(name, EnvStack::globals())
             {

src/wcstringutil.rs

@@ -36,6 +36,14 @@ pub fn string_prefixes_string_maybe_case_insensitive(
     })(proposed_prefix, value)
 }
 
+/// Remove the optional executable extension if there is one
+/// Always returns None on non-Cygwin platforms
+pub fn strip_executable_suffix(path: &wstr) -> Option<&wstr> {
+    const DOT_EXE: &wstr = L!(".exe");
+    (cfg!(cygwin) && { string_suffixes_string_case_insensitive(DOT_EXE, path) })
+        .then(|| &path[..path.len() - DOT_EXE.len()])
+}
+
 /// Test if a string is a suffix of another.
 pub fn string_suffixes_string_case_insensitive(proposed_suffix: &wstr, value: &wstr) -> bool {
     let suffix_size = proposed_suffix.len();

src/wildcard.rs

@@ -1,9 +1,10 @@
 // Enumeration of all wildcard types.
 
 use libc::X_OK;
+use once_cell::sync::Lazy;
 use std::cmp::Ordering;
 use std::collections::HashSet;
-use std::fs;
+use std::os::unix::fs::MetadataExt;
 
 use crate::common::{
     UnescapeFlags, UnescapeStringStyle, WILDCARD_RESERVED_BASE, WSL, char_offset,
@@ -17,6 +18,7 @@
 use crate::wchar::prelude::*;
 use crate::wcstringutil::{
     CaseSensitivity, string_fuzzy_match_string, string_suffixes_string_case_insensitive,
+    strip_executable_suffix,
 };
 use crate::wutil::dir_iter::DirEntryType;
 use crate::wutil::{dir_iter::DirEntry, lwstat, waccess};
@@ -334,7 +336,7 @@ fn file_get_desc(
 /// up. Note that the filename came from a readdir() call, so we know it exists.
 fn wildcard_test_flags_then_complete(
     filepath: &wstr,
-    filename: &wstr,
+    mut filename: &wstr,
     wc: &wstr,
     expand_flags: ExpandFlags,
     out: &mut CompletionReceiver,
@@ -382,10 +384,32 @@ fn wildcard_test_flags_then_complete(
         .check_type()
         .map(|x| x == DirEntryType::reg)
         .unwrap_or(false);
-    if executables_only && (!is_regular_file || waccess(filepath, X_OK) != 0) {
+    let is_executable = Lazy::new(|| is_regular_file && waccess(filepath, X_OK) == 0);
+    if executables_only && !*is_executable {
         return false;
     }
 
+    let filepath_stat = Lazy::new(|| lwstat(filepath));
+
+    // For executables on Cygwin, prefer the name without the .exe, to match
+    // better with Unix names, but only if there isn't also a file without that
+    // extension and the user hasn't started to type the extension
+    if let Some(filepath_stripped) = strip_executable_suffix(filepath) {
+        let stripped_filename_len = filename.len() - (filepath.len() - filepath_stripped.len());
+        if wc.len() <= stripped_filename_len && *is_executable {
+            let stat_stripped = lwstat(filepath_stripped).map(|stat| (stat.dev(), stat.ino()));
+            let stat = filepath_stat.as_ref().map(|stat| (stat.dev(), stat.ino()));
+
+            // TODO(MSRV>=1.88) use if-let-chain
+            //   if let Ok(stat_stripped) = stat_stripped
+            //       && let Ok(stat) = stat
+            //       && stat_stripped == stat
+            if stat_stripped.is_ok() && stat.is_ok() && stat_stripped.unwrap() == stat.unwrap() {
+                filename = &filename[0..filename.len() - 4];
+            }
+        }
+    }
+
     // Compute the description.
     // This is effectively only for command completions,
     // because we disable descriptions for regular file completions.
@@ -395,8 +419,7 @@ fn wildcard_test_flags_then_complete(
             None => {
                 // We do not know it's a link from the d_type,
                 // so we will have to do an lstat().
-                let lstat: Option<fs::Metadata> = lwstat(filepath).ok();
-                if let Some(md) = &lstat {
+                if let Ok(md) = filepath_stat.as_ref() {
                     md.is_symlink()
                 } else {
                     // This file is no longer be usable, skip it.
@@ -1109,7 +1132,7 @@ pub fn wildcard_expand_string<'closure>(
 /// Test whether the given wildcard matches the string. Does not perform any I/O.
 ///
 /// \param str The string to test
-/// \param wc The wildcard to test against
+/// \param pattern The wildcard to test against
 /// \param leading_dots_fail_to_match if set, strings with leading dots are assumed to be hidden
 /// files and are not matched (default was false)
 ///

tests/checks/complete-cygwin.fish

@@ -0,0 +1,72 @@
+#RUN: fish=%fish %fish %s
+
+# REQUIRES: %fish -c "is_cygwin"
+
+mkdir dir
+echo "#!/bin/sh" >dir/foo.exe
+echo "#!/bin/sh" >dir/foo.bar
+set PATH (pwd)/dir $PATH
+
+# === Check that `complete` prefers to non-exe name, unless the user started
+# to type the extension
+complete -C"./dir/fo"
+# CHECK: ./dir/foo{{\t}}command
+# CHECK: ./dir/foo.bar{{\t}}command
+complete -C"./dir/foo."
+# CHECK: ./dir/foo.bar{{\t}}command
+# CHECK: ./dir/foo.exe{{\t}}command
+
+# === Check that foo.exe uses foo's description if it doesn't have its own
+function __fish_describe_command
+    echo -e "foo\tposix"
+end
+complete -C"./dir/foo."
+# CHECK: ./dir/foo.bar{{\t}}command
+# CHECK: ./dir/foo.exe{{\t}}Posix
+function __fish_describe_command
+    echo -e "foo\tposix"
+    echo -e "foo.exe\twindows"
+end
+complete -C"./dir/fo"
+# CHECK: ./dir/foo{{\t}}Posix
+# CHECK: ./dir/foo.bar{{\t}}command
+complete -C"./dir/foo."
+# CHECK: ./dir/foo.bar{{\t}}command
+# CHECK: ./dir/foo.exe{{\t}}Windows
+
+# === Check that if we have a non-exe and an exe file, they both show
+echo "#!/bin/sh" >dir/foo.bar.exe
+complete -C"./dir/foo.ba"
+# CHECK: ./dir/foo.bar{{\t}}command
+# CHECK: ./dir/foo.bar.exe{{\t}}command
+
+# === Check that "foo.fish" completion file is used when completing "foo.exe"
+# and there is no "foo.exe.fish"
+mkdir $__fish_config_dir/completions
+echo "complete -c foo -s a -d args; complete -c foo -s b -d bargs" >$__fish_config_dir/completions/foo.fish
+complete -C"./dir/foo.exe -"
+# CHECK: -a{{\t}}args
+# CHECK: -b{{\t}}bargs
+
+# === Check that "foo.exe.fish" is used over "foo.fish" when both are present
+# when completing "foo.exe" (but still uses "foo.fish" for "foo")
+# Note: use subshell to avoid waiting 15s for the autoload cache to become stale
+echo "complete -c foo -s c -d cargs; complete -c foo -s d -d dargs" >$__fish_config_dir/completions/foo.exe.fish
+$fish -ic 'complete -C"./dir/foo.exe -"'
+# CHECK: -c{{\t}}cargs
+# CHECK: -d{{\t}}dargs
+$fish -ic 'complete -C"./dir/foo -"'
+# CHECK: -a{{\t}}args
+# CHECK: -b{{\t}}bargs
+
+# === We only support exe=>non-exe fallback for description/args completion.
+# We do not handle the other way around
+function __fish_describe_command
+    echo -e "foo.bar.exe\twindows"
+end
+rm $__fish_config_dir/completions/foo.fish
+complete -C"./dir/foo.ba"
+# CHECK: ./dir/foo.bar{{\t}}command
+# CHECK: ./dir/foo.bar.exe{{\t}}Windows
+$fish -ic 'complete -C"./dir/foo -"'
+# nothing

tests/test_functions/is_cygwin.fish

@@ -0,0 +1,3 @@
+function is_cygwin
+    string match -qr "^(MSYS|CYGWIN)" -- (uname)
+end