Teach env_universal_remove to directly report whether the variable was

successfully removed.

env.cpp

@@ -917,7 +917,7 @@ int env_remove(const wcstring &key, int var_mode)
             !(var_mode & ENV_GLOBAL) &&
             !(var_mode & ENV_LOCAL))
     {
-        erased = ! env_universal_remove(key.c_str());
+        erased = env_universal_remove(key);
     }
 
     react_to_variable_change(key);

env_universal.cpp

@@ -105,20 +105,12 @@ void env_universal_set(const wcstring &name, const wcstring &value, bool exportv
     env_universal_barrier();
 }
 
-int env_universal_remove(const wchar_t *name)
+bool env_universal_remove(const wcstring &name)
 {
-    int res;
-
     if (!s_env_univeral_inited)
-        return 1;
+        return false;
 
-    CHECK(name, 1);
-
-    const wcstring name_str = name;
-    // TODO: shouldn't have to call get() here, should just have remove return success
-    res = env_universal_common_get(name_str).missing();
-    env_universal_common_remove(name_str);
-    return res;
+    return env_universal_common_remove(name);
 }
 
 void env_universal_get_names(wcstring_list_t &lst,

env_universal.h

@@ -37,9 +37,9 @@ void env_universal_set(const wcstring &name, const wcstring &val, bool exportv);
 /**
    Erase a universal variable
 
-   \return zero if the variable existed, and non-zero if the variable did not exist
+   \return true if the variable existed, and false if the variable did not exist
 */
-int env_universal_remove(const wchar_t *name);
+bool env_universal_remove(const wcstring &name);
 
 /**
    Read all available messages from the server.

env_universal_common.cpp

@@ -113,9 +113,9 @@ void env_universal_common_init(void (*cb)(fish_message_type_t type, const wchar_
 /**
    Remove variable with specified name
 */
-void env_universal_common_remove(const wcstring &name)
+bool env_universal_common_remove(const wcstring &name)
 {
-    default_universal_vars().remove(name);
+    return default_universal_vars().remove(name);
 }
 
 /**
@@ -359,25 +359,21 @@ void env_universal_t::set(const wcstring &key, const wcstring &val, bool exportv
     this->set_internal(key, val, exportv, true /* overwrite */);
 }
 
-void env_universal_t::remove_internal(const wcstring &key, bool overwrite)
+bool env_universal_t::remove_internal(const wcstring &key)
 {
     ASSERT_IS_LOCKED(lock);
-    if (! overwrite && this->modified.find(key) != modified.end())
-    {
-        /* This value has been modified and we're not overwriting it. Skip it. */
-        return;
-    }
     size_t erased = this->vars.erase(key);
-    if (erased > 0 && overwrite)
+    if (erased > 0)
     {
         this->modified.insert(key);
     }
+    return erased > 0;
 }
 
-void env_universal_t::remove(const wcstring &key)
+bool env_universal_t::remove(const wcstring &key)
 {
     scoped_lock locker(lock);
-    this->remove_internal(key, true);
+    return this->remove_internal(key);
 }
 
 wcstring_list_t env_universal_t::get_names(bool show_exported, bool show_unexported) const

env_universal_common.h

@@ -60,15 +60,9 @@ void env_universal_common_get_names(wcstring_list_t &lst,
 void env_universal_common_set(const wchar_t *key, const wchar_t *val, bool exportv);
 
 /**
-   Remove the specified variable.
-
-   This function operate agains the local copy of all universal
-   variables, it does not communicate with any other process.
-
-   Do not call this function. Create a message to do it. This function
-   is only to be used when fishd is dead.
+   Remove the specified variable. Returns true if it was removed, false if it was not found.
 */
-void env_universal_common_remove(const wcstring &key);
+bool env_universal_common_remove(const wcstring &key);
 
 /**
    Get the value of the variable with the specified name
@@ -113,7 +107,7 @@ class env_universal_t
     void parse_message_internal(wchar_t *msg, callback_data_list_t *callbacks);
     
     void set_internal(const wcstring &key, const wcstring &val, bool exportv, bool overwrite);
-    void remove_internal(const wcstring &name, bool overwrite);
+    bool remove_internal(const wcstring &name);
     
     /* Functions concerned with saving */
     bool open_and_acquire_lock(const wcstring &path, int *out_fd);
@@ -139,8 +133,8 @@ public:
     /* Sets a variable */
     void set(const wcstring &key, const wcstring &val, bool exportv);
     
-    /* Removes a variable */
-    void remove(const wcstring &name);
+    /* Removes a variable. Returns true if it was found, false if not. */
+    bool remove(const wcstring &name);
     
     /* Gets variable names */
     wcstring_list_t get_names(bool show_exported, bool show_unexported) const;