From d9ebe13cb4569f66c73d9e329bdff4d0a00077b4 Mon Sep 17 00:00:00 2001
From: ridiculousfish <rf@fishshell.com>
Date: Fri, 27 Nov 2020 13:21:21 -0800
Subject: [PATCH] Reorganize and improve commenting of
 autosuggest_validate_from_history

No behavior change expected here.
---
 src/highlight.cpp | 39 ++++++++++++++++++---------------------
 src/highlight.h   |  6 +++---
 2 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/src/highlight.cpp b/src/highlight.cpp
index 5eccd8032..444b9a4d1 100644
--- a/src/highlight.cpp
+++ b/src/highlight.cpp
@@ -434,9 +434,6 @@ bool autosuggest_validate_from_history(const history_item_t &item,
                                        const operation_context_t &ctx) {
     ASSERT_IS_BACKGROUND_THREAD();
 
-    bool handled = false;
-    bool suggestion_ok = false;
-
     // Parse the string.
     wcstring parsed_command;
     wcstring cd_dir;
@@ -447,35 +444,35 @@ bool autosuggest_validate_from_history(const history_item_t &item,
         return true;
     }
 
+    // We handle cd specially.
     if (parsed_command == L"cd" && !cd_dir.empty()) {
-        // We can possibly handle this specially.
         if (expand_one(cd_dir, expand_flag::skip_cmdsubst, ctx)) {
-            handled = true;
-            bool is_help =
-                string_prefixes_string(cd_dir, L"--help") || string_prefixes_string(cd_dir, L"-h");
-            if (!is_help) {
+            if (string_prefixes_string(cd_dir, L"--help") ||
+                string_prefixes_string(cd_dir, L"-h")) {
+                // cd --help is always valid.
+                return true;
+            } else {
+                // Check the directory target, respecting CDPATH.
+                // Permit the autosuggestion if the path is valid and not our directory.
                 auto path = path_get_cdpath(cd_dir, working_directory, ctx.vars);
-                if (path && !paths_are_same_file(working_directory, *path)) {
-                    suggestion_ok = true;
-                }
+                return path && !paths_are_same_file(working_directory, *path);
             }
         }
     }
 
-    if (handled) {
-        return suggestion_ok;
-    }
-
-    // Not handled specially so handle it here.
+    // Not handled specially. Is the command valid?
     bool cmd_ok = builtin_exists(parsed_command) || function_exists_no_autoload(parsed_command) ||
                   path_get_path(parsed_command, nullptr, ctx.vars);
-
-    if (cmd_ok) {
-        const path_list_t &paths = item.get_required_paths();
-        suggestion_ok = all_paths_are_valid(paths, working_directory);
+    if (!cmd_ok) {
+        return false;
     }
 
-    return suggestion_ok;
+    // Did the historical command have arguments that look like paths, which aren't paths now?
+    if (!all_paths_are_valid(item.get_required_paths(), working_directory)) {
+        return false;
+    }
+
+    return true;
 }
 
 // Highlights the variable starting with 'in', setting colors within the 'colors' array. Returns the
diff --git a/src/highlight.h b/src/highlight.h
index 5b65eefbe..50cef0436 100644
--- a/src/highlight.h
+++ b/src/highlight.h
@@ -116,9 +116,9 @@ struct highlight_color_resolver_t {
                                       const environment_t &vars) const;
 };
 
-/// Given a command 'str' from the history, try to determine whether we ought to suggest it by
-/// specially recognizing the command. Returns true if we validated the command. If so, returns by
-/// reference whether the suggestion is valid or not.
+/// Given an item \p 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.
 bool autosuggest_validate_from_history(const history_item_t &item,
                                        const wcstring &working_directory,
                                        const operation_context_t &ctx);