From f4d9ddd3ad2c21553dddc46e62fd23ed0d51f9ec Mon Sep 17 00:00:00 2001
From: 132ikl <132@ikl.sh>
Date: Mon, 26 Feb 2024 13:14:19 -0500
Subject: [PATCH] Fix completions for directories with hidden files (#11921)

# Description
Attempting to complete a directory with hidden files could cause a
variety of issues. When Rust parses the partial path to be completed
into components, it removes the trailing `.` since it interprets this to
mean "the current directory", but in the case of the completer we
actually want to treat the trailling `.` as a literal `.`. This PR fixes
this by adding a `.` back into the Path components if the last character
of the path is a `.` AND the path is longer than 1 character (eg., not
just a ".", since that correctly gets interpreted as Component::CurDir).

Here are some things this fixes:
- Panic when tab completing for hidden files in a directory with hidden
files (ex. `ls test/.`)
- Panic when tab completing a directory with only hidden files (since
the common prefix ends with a `.`, causing the previous issue)
- Mishandling of tab completing hidden files in directory (ex. `ls
~/.<TAB>` lists all files instead of just hidden files)
- Trailing `.` being inexplicably removed when tab completing a
directory without hidden files

While testing for this PR I also noticed there is a similar issue when
completing with `..` (ex. `ls ~/test/..<TAB>`) which is not fixed by
this PR (edit: see #11922).

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
N/A

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->
Added a hidden-files-within-directories test to the `file_completions`
test.

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
---
 crates/nu-cli/src/completions/completion_common.rs    | 11 ++++++++++-
 crates/nu-cli/tests/completions.rs                    | 10 ++++++++++
 .../.hidden_folder/{.gitkeep => .hidden_subfile}      |  0
 3 files changed, 20 insertions(+), 1 deletion(-)
 rename tests/fixtures/completions/.hidden_folder/{.gitkeep => .hidden_subfile} (100%)

diff --git a/crates/nu-cli/src/completions/completion_common.rs b/crates/nu-cli/src/completions/completion_common.rs
index 3d12f2aea4..9b5a022fd2 100644
--- a/crates/nu-cli/src/completions/completion_common.rs
+++ b/crates/nu-cli/src/completions/completion_common.rs
@@ -5,6 +5,7 @@ use nu_path::home_dir;
 use nu_protocol::engine::{EngineState, Stack};
 use nu_protocol::{engine::StateWorkingSet, Span};
 use nu_utils::get_ls_colors;
+use std::ffi::OsStr;
 use std::path::{is_separator, Component, Path, PathBuf, MAIN_SEPARATOR as SEP};
 
 fn complete_rec(
@@ -112,7 +113,15 @@ pub fn complete_item(
             get_ls_colors(ls_colors_env_str)
         });
     let mut original_cwd = OriginalCwd::None;
-    let mut components = Path::new(&partial).components().peekable();
+    let mut components_vec: Vec<Component> = Path::new(&partial).components().collect();
+
+    // Path components that end with a single "." get normalized away,
+    // so if the partial path ends in a literal "." we must add it back in manually
+    if partial.ends_with('.') && partial.len() > 1 {
+        components_vec.push(Component::Normal(OsStr::new(".")));
+    };
+    let mut components = components_vec.into_iter().peekable();
+
     let mut cwd = match components.peek().cloned() {
         Some(c @ Component::Prefix(..)) => {
             // windows only by definition
diff --git a/crates/nu-cli/tests/completions.rs b/crates/nu-cli/tests/completions.rs
index 57b0dc509f..2914ee503e 100644
--- a/crates/nu-cli/tests/completions.rs
+++ b/crates/nu-cli/tests/completions.rs
@@ -247,6 +247,16 @@ fn file_completions() {
 
     // Match the results
     match_suggestions(expected_paths, suggestions);
+
+    // Test completions for hidden files
+    let target_dir = format!("ls {}/.", folder(dir.join(".hidden_folder")));
+    let suggestions = completer.complete(&target_dir, target_dir.len());
+
+    let expected_paths: Vec<String> =
+        vec![file(dir.join(".hidden_folder").join(".hidden_subfile"))];
+
+    // Match the results
+    match_suggestions(expected_paths, suggestions);
 }
 
 #[test]
diff --git a/tests/fixtures/completions/.hidden_folder/.gitkeep b/tests/fixtures/completions/.hidden_folder/.hidden_subfile
similarity index 100%
rename from tests/fixtures/completions/.hidden_folder/.gitkeep
rename to tests/fixtures/completions/.hidden_folder/.hidden_subfile