Simplify callback_data_t

SET_EXPORT no longer makes sense; remove it.

src/env.cpp

@@ -608,28 +608,16 @@ static void react_to_variable_change(const wchar_t *op, const wcstring &key) {
 
 /// Universal variable callback function. This function makes sure the proper events are triggered
 /// when an event occurs.
-static void universal_callback(fish_message_type_t type, const wchar_t *name) {
+static void universal_callback(const callback_data_t &cb) {
-    const wchar_t *op;
+    const wchar_t *op = cb.is_erase() ? L"ERASE" : L"SET";
 
-    switch (type) {
+    react_to_variable_change(op, cb.key);
-        case SET:
-        case SET_EXPORT: {
-            op = L"SET";
-            break;
-        }
-        case ERASE: {
-            op = L"ERASE";
-            break;
-        }
-    }
-
-    react_to_variable_change(op, name);
     vars_stack().mark_changed_exported();
 
-    event_t ev = event_t::variable_event(name);
+    event_t ev = event_t::variable_event(cb.key);
     ev.arguments.push_back(L"VARIABLE");
     ev.arguments.push_back(op);
-    ev.arguments.push_back(name);
+    ev.arguments.push_back(cb.key);
     event_fire(&ev);
 }
 
@@ -720,10 +708,9 @@ void misc_init() {
     }
 }
 
-static void env_universal_callbacks(callback_data_list_t &callbacks) {
+static void env_universal_callbacks(const callback_data_list_t &callbacks) {
-    for (size_t i = 0; i < callbacks.size(); i++) {
+    for (const callback_data_t &cb : callbacks) {
-        const callback_data_t &data = callbacks.at(i);
+        universal_callback(cb);
-        universal_callback(data.type, data.key.c_str());
     }
 }
 

src/env_universal_common.cpp

@@ -74,6 +74,9 @@
 /// Small note about not editing ~/.fishd manually. Inserted at the top of all .fishd files.
 #define SAVE_MSG "# This file contains fish universal variable definitions.\n"
 
+/// The different types of messages found in the fishd file.
+enum class uvar_message_type_t { set, set_export };
+
 static wcstring get_machine_identifier();
 
 /// return a list of paths where the uvars file has been historically stored.
@@ -148,7 +151,7 @@ static bool append_utf8(const wcstring &input, std::string *receiver, std::strin
 
 /// Creates a file entry like "SET fish_color_cwd:FF0". Appends the result to *result (as UTF8).
 /// Returns true on success. storage may be used for temporary storage, to avoid allocations.
-static bool append_file_entry(fish_message_type_t type, const wcstring &key_in,
+static bool append_file_entry(uvar_message_type_t type, const wcstring &key_in,
                               const wcstring &val_in, std::string *result, std::string *storage) {
     assert(storage != NULL);
     assert(result != NULL);
@@ -158,7 +161,7 @@ static bool append_file_entry(fish_message_type_t type, const wcstring &key_in,
     const size_t result_length_on_entry = result->size();
 
     // Append header like "SET "
-    result->append(type == SET ? SET_MBS : SET_EXPORT_MBS);
+    result->append(type == uvar_message_type_t::set ? SET_MBS : SET_EXPORT_MBS);
     result->push_back(' ');
 
     // Append variable name like "fish_color_cwd".
@@ -300,7 +303,7 @@ void env_universal_t::generate_callbacks(const var_table_t &new_vars,
 
         // If the value is not present in new_vars, it has been erased.
         if (new_vars.find(key) == new_vars.end()) {
-            callbacks.push_back(callback_data_t(ERASE, key, L""));
+            callbacks.push_back(callback_data_t(key, none()));
         }
     }
 
@@ -319,8 +322,7 @@ void env_universal_t::generate_callbacks(const var_table_t &new_vars,
         if (existing == this->vars.end() || existing->second.exports() != new_entry.exports() ||
             existing->second != new_entry) {
             // Value has changed.
-            callbacks.push_back(callback_data_t(new_entry.exports() ? SET_EXPORT : SET, key,
+            callbacks.push_back(callback_data_t(key, new_entry.as_string()));
-                                                new_entry.as_string()));
         }
     }
 }
@@ -407,8 +409,9 @@ bool env_universal_t::write_to_fd(int fd, const wcstring &path) {
         // variable; soldier on.
         const wcstring &key = iter->first;
         const env_var_t &var = iter->second;
-        append_file_entry(var.exports() ? SET_EXPORT : SET, key, encode_serialized(var.as_list()),
+        append_file_entry(
-                          &contents, &storage);
+            var.exports() ? uvar_message_type_t::set_export : uvar_message_type_t::set, key,
+            encode_serialized(var.as_list()), &contents, &storage);
 
         // Go to next.
         ++iter;

src/env_universal_common.h

@@ -13,17 +13,19 @@
 #include "env.h"
 #include "wutil.h"
 
-/// The different types of messages found in the fishd file.
-typedef enum { SET, SET_EXPORT, ERASE } fish_message_type_t;
-
 /// Callback data, reflecting a change in universal variables.
 struct callback_data_t {
-    fish_message_type_t type;
+    // The name of the variable.
     wcstring key;
-    wcstring val;
 
-    callback_data_t(fish_message_type_t t, wcstring k, wcstring v)
+    // The value of the variable, or none if it is erased.
-        : type(t), key(std::move(k)), val(std::move(v)) {}
+    maybe_t<wcstring> val;
+
+    /// Construct from a key and maybe a value.
+    callback_data_t(wcstring k, maybe_t<wcstring> v) : key(std::move(k)), val(std::move(v)) {}
+
+    /// \return whether this callback represents an erased variable.
+    bool is_erase() const { return !val.has_value(); }
 };
 
 typedef std::vector<struct callback_data_t> callback_data_list_t;

src/fish_tests.cpp

@@ -2915,15 +2915,12 @@ static void test_universal_callbacks() {
 
     // Should see exactly three changes.
     do_test(callbacks.size() == 3);
-    do_test(callbacks.at(0).type == SET);
     do_test(callbacks.at(0).key == L"alpha");
-    do_test(callbacks.at(0).val == L"2");
+    do_test(callbacks.at(0).val == wcstring{L"2"});
-    do_test(callbacks.at(1).type == SET_EXPORT);
     do_test(callbacks.at(1).key == L"beta");
-    do_test(callbacks.at(1).val == L"1");
+    do_test(callbacks.at(1).val == wcstring{L"1"});
-    do_test(callbacks.at(2).type == ERASE);
     do_test(callbacks.at(2).key == L"delta");
-    do_test(callbacks.at(2).val == L"");
+    do_test(callbacks.at(2).val == none());
     (void)system("rm -Rf test/fish_uvars_test/");
 }