From 73f344f41bd20c005a0f5b810dc20229392f2bf8 Mon Sep 17 00:00:00 2001
From: Sanne Wouda <sanne.wouda@gmail.com>
Date: Wed, 11 Mar 2015 14:14:56 +0100
Subject: [PATCH] Revert 1349d12 and properly fix #213

As suggested by @ridiculousfish, when removing autoloaded functions, add them
to a tombstones set.  These functions will never be autoloaded again in the
current shell, not even when the timestamp changes.

Tested as per comment 1 of #1033.  `~/.config/fish/functions/ls.fish` contains
the function definition.  `function -e ls` removes the redefined `ls` (and
reverts back to the built-in command).  `touch .../ls.fish` does not cause the
function to be reloaded.
---
 builtin.cpp  |  2 +-
 function.cpp | 39 ++++++++++++++++++++++++++++++---------
 function.h   |  3 ---
 3 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/builtin.cpp b/builtin.cpp
index 44f4e84ea..c5e8e46f9 100644
--- a/builtin.cpp
+++ b/builtin.cpp
@@ -1565,7 +1565,7 @@ static int builtin_functions(parser_t &parser, wchar_t **argv)
     {
         int i;
         for (i=woptind; i<argc; i++)
-            function_remove_ignore_autoload(argv[i]);
+            function_remove(argv[i]);
         return STATUS_BUILTIN_OK;
     }
     else if (desc)
diff --git a/function.cpp b/function.cpp
index 11c231146..adb164792 100644
--- a/function.cpp
+++ b/function.cpp
@@ -43,6 +43,11 @@
 typedef std::map<wcstring, function_info_t> function_map_t;
 static function_map_t loaded_functions;
 
+/**
+   Functions that shouldn't be autoloaded (anymore).
+*/
+static std::set<wcstring> function_tombstones;
+
 /* Lock for functions */
 static pthread_mutex_t functions_lock;
 
@@ -61,6 +66,8 @@ function_autoload_t::function_autoload_t() : autoload_t(L"fish_function_path", N
 {
 }
 
+static bool function_remove_ignore_autoload(const wcstring &name);
+
 /** Callback when an autoloaded function is removed */
 void function_autoload_t::command_removed(const wcstring &cmd)
 {
@@ -84,6 +91,11 @@ static int load(const wcstring &name)
     scoped_lock lock(functions_lock);
     bool was_autoload = is_autoload;
     int res;
+
+    bool no_more_autoload = function_tombstones.count(name) == 1;
+    if (no_more_autoload)
+        return 0;
+
     function_map_t::iterator iter = loaded_functions.find(name);
     if (iter !=  loaded_functions.end() && !iter->second.is_autoload)
     {
@@ -225,19 +237,28 @@ int function_exists_no_autoload(const wcstring &cmd, const env_vars_snapshot_t &
     return loaded_functions.find(cmd) != loaded_functions.end() || function_autoloader.can_load(cmd, vars);
 }
 
-bool function_remove_ignore_autoload(const wcstring &name)
+static bool function_remove_ignore_autoload(const wcstring &name)
 {
     scoped_lock lock(functions_lock);
-    bool erased = (loaded_functions.erase(name) > 0);
 
-    if (erased)
-    {
-        event_t ev(EVENT_ANY);
-        ev.function_name=name;
-        event_remove(ev);
-    }
-    return erased;
+    function_map_t::iterator iter = loaded_functions.find(name);
 
+    // not found.  not erasing.
+    if (iter == loaded_functions.end())
+        return false;
+
+    // removing an auto-loaded function.  prevent it from being
+    // auto-reloaded.
+    if (iter->second.is_autoload)
+        function_tombstones.insert(name);
+
+    loaded_functions.erase(iter);
+
+    event_t ev(EVENT_ANY);
+    ev.function_name=name;
+    event_remove(ev);
+
+    return true;
 }
 
 void function_remove(const wcstring &name)
diff --git a/function.h b/function.h
index 2f4fe8c52..ac56bd46c 100644
--- a/function.h
+++ b/function.h
@@ -105,9 +105,6 @@ void function_init();
 /** Add a function. definition_line_offset is the line number of the function's definition within its source file */
 void function_add(const function_data_t &data, const parser_t &parser, int definition_line_offset = 0);
 
-/** Removes a function from our internal table, returning true if it was found and false if not */
-bool function_remove_ignore_autoload(const wcstring &name);
-
 /**
    Remove the function with the specified name.
 */