Properly fire events for universal variable changes

Fixes #1929

3 files changed, 24 insertions, 17 deletions

diff --git a/env.cpp b/env.cpp
index 1b1b0d98..13aacf57 100644
--- a/env.cpp
+++ b/env.cpp
@@ -611,8 +611,6 @@ int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t var_mode)
     bool has_changed_new = false;
     int done=0;
 
-    int is_universal = 0;
-
     if (val && contains(key, L"PWD", L"HOME"))
     {
         /* Canoncalize our path; if it changes, recurse and try again. */
@@ -698,8 +696,6 @@ int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t var_mode)
                 mark_changed_exported();
             }
         }
-        is_universal = 1;
-
     }
     else
     {
@@ -764,7 +760,6 @@ int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t var_mode)
 
                 uvars()->set(key, val, exportv);
                 env_universal_barrier();
-                is_universal = 1;
 
                 done = 1;
 
@@ -813,18 +808,15 @@ int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t var_mode)
         }
     }
 
-    if (!is_universal)
-    {
-        event_t ev = event_t::variable_event(key);
-        ev.arguments.reserve(3);
-        ev.arguments.push_back(L"VARIABLE");
-        ev.arguments.push_back(L"SET");
-        ev.arguments.push_back(key);
+    event_t ev = event_t::variable_event(key);
+    ev.arguments.reserve(3);
+    ev.arguments.push_back(L"VARIABLE");
+    ev.arguments.push_back(L"SET");
+    ev.arguments.push_back(key);
 
-        //  debug( 1, L"env_set: fire events on variable %ls", key );
-        event_fire(&ev);
-        //  debug( 1, L"env_set: return from event firing" );
-    }
+    //  debug( 1, L"env_set: fire events on variable %ls", key );
+    event_fire(&ev);
+    //  debug( 1, L"env_set: return from event firing" );
 
     react_to_variable_change(key);
 
@@ -899,7 +891,6 @@ int env_remove(const wcstring &key, int var_mode)
             ev.arguments.push_back(L"VARIABLE");
             ev.arguments.push_back(L"ERASE");
             ev.arguments.push_back(key);
-
             event_fire(&ev);
 
             erased = 1;
@@ -914,6 +905,11 @@ int env_remove(const wcstring &key, int var_mode)
         if (erased)
         {
             env_universal_barrier();
+            event_t ev = event_t::variable_event(key);
+            ev.arguments.push_back(L"VARIABLE");
+            ev.arguments.push_back(L"ERASE");
+            ev.arguments.push_back(key);
+            event_fire(&ev);
         }
     }
 
diff --git a/tests/test3.in b/tests/test3.in
index b162939d..51d545c6 100644
--- a/tests/test3.in
+++ b/tests/test3.in
@@ -174,6 +174,15 @@ else
 end
 
 set -eU __fish_test_universal_variables_variable_foo
+function watch_foo --on-variable __fish_test_universal_variables_variable_foo
+    echo Foo change detected
+end
+
+set -U __fish_test_universal_variables_variable_foo 1234
+set -eU __fish_test_universal_variables_variable_foo
+
+functions -e watch_foo
+
 
 # test erasing variables without a specified scope
 
diff --git a/tests/test3.out b/tests/test3.out
index d7dd2c43..a6d5d3ac 100644
--- a/tests/test3.out
+++ b/tests/test3.out
@@ -13,6 +13,8 @@ Test 12 pass
 Test 13 pass
 Test 14 pass
 Test 15 pass
+Foo change detected
+Foo change detected
 Test 16 pass
 Testing Universal Startup
 1