diff --git a/env.cpp b/env.cpp index 0b262f11b..9f057848a 100644 --- a/env.cpp +++ b/env.cpp @@ -351,6 +351,13 @@ static void universal_callback(fish_message_type_t type, const wchar_t *name, co str=L"SET"; break; } + + case ERASE: + { + str=L"ERASE"; + break; + } + default: break; } diff --git a/env_universal_common.cpp b/env_universal_common.cpp index ce4f4f9a6..49d26cdc1 100644 --- a/env_universal_common.cpp +++ b/env_universal_common.cpp @@ -364,25 +364,76 @@ wcstring_list_t env_universal_t::get_names(bool show_exported, bool show_unexpor return result; } - -void env_universal_t::erase_unmodified_values() +/* Given a variable table, generate callbacks representing the difference between our vars and the new vars */ +void env_universal_t::generate_callbacks(const var_table_t &new_vars, callback_data_list_t *callbacks) const { - /* Delete all non-modified keys. */ - var_table_t::iterator iter = vars.begin(); - while (iter != vars.end()) + assert(callbacks != NULL); + + /* Construct callbacks for erased values */ + for (var_table_t::const_iterator iter = this->vars.begin(); iter != this->vars.end(); ++iter) { const wcstring &key = iter->first; - if (modified.find(key) == modified.end()) + + /* Skip modified values */ + if (this->modified.find(key) != this->modified.end()) { - // Unmodified key. Erase the old value. - vars.erase(iter++); + continue; + } + + /* 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"")); + } + } + + /* Construct callbacks for newly inserted or changed values */ + for (var_table_t::const_iterator iter = new_vars.begin(); iter != new_vars.end(); ++iter) + { + const wcstring &key = iter->first; + + /* Skip modified values */ + if (this->modified.find(key) != this->modified.end()) + { + continue; + } + + /* See if the value has changed */ + const var_entry_t &new_entry = iter->second; + var_table_t::const_iterator existing = this->vars.find(key); + if (existing == this->vars.end() || existing->second.exportv != new_entry.exportv || existing->second.val != new_entry.val) + { + /* Value has changed */ + callbacks->push_back(callback_data_t(new_entry.exportv ? SET_EXPORT : SET, key, new_entry.val)); + } + } +} + + +void env_universal_t::acquire_variables(var_table_t *vars_to_acquire) +{ + /* Copy modified values from existing vars to vars_to_acquire */ + for (std::set::iterator iter = this->modified.begin(); iter != this->modified.end(); ++iter) + { + const wcstring &key = *iter; + var_table_t::iterator src_iter = this->vars.find(key); + if (src_iter == this->vars.end()) + { + /* The value has been deleted. */ + vars_to_acquire->erase(key); } else { - // Modified key, retain the value. - ++iter; + /* The value has been modified. Copy it over. Note we can destructively modify the source entry in vars since we are about to get rid of this->vars entirely. */ + var_entry_t &src = src_iter->second; + var_entry_t &dst = (*vars_to_acquire)[key]; + dst.val.swap(src.val); + dst.exportv = src.exportv; } } + + /* We have constructed all the callbacks and updated vars_to_acquire. Acquire it! */ + this->vars.swap(*vars_to_acquire); } void env_universal_t::load_from_fd(int fd, callback_data_list_t *callbacks) @@ -397,10 +448,18 @@ void env_universal_t::load_from_fd(int fd, callback_data_list_t *callbacks) } else { - /* Unmodified values are sourced from the file. Since we are about to read a different file, erase them */ - this->erase_unmodified_values(); - /* Read from the file. */ - this->read_message_internal(fd, callbacks); + /* Read a variables table from the file. */ + var_table_t new_vars = this->read_message_internal(fd); + + /* Announce changes */ + if (callbacks != NULL) + { + this->generate_callbacks(new_vars, callbacks); + } + + /* Acquire the new variables */ + this->acquire_variables(&new_vars); + last_read_file = current_file; } } @@ -416,7 +475,6 @@ bool env_universal_t::load_from_path(const wcstring &path, callback_data_list_t return true; } - /* OK to not use CLO_EXEC here because fishd is single threaded */ bool result = false; int fd = wopen_cloexec(path, O_RDONLY); if (fd >= 0) @@ -721,13 +779,6 @@ bool env_universal_t::sync(callback_data_list_t *callbacks) return false; } -#if 0 - for (std::set::iterator iter = modified.begin(); iter != modified.end(); ++iter) - { - fprintf(stderr, "Modified %ls\n", iter->c_str()); - } -#endif - const wcstring directory = wdirname(vars_path); bool success = true; int vars_fd = -1; @@ -797,9 +848,12 @@ bool env_universal_t::sync(callback_data_list_t *callbacks) return success; } -void env_universal_t::read_message_internal(int fd, callback_data_list_t *callbacks) +var_table_t env_universal_t::read_message_internal(int fd) { - ASSERT_IS_LOCKED(lock); + var_table_t result; + + // Temp value used to avoid repeated allocations + wcstring storage; // The line we construct (and then parse) std::string line; @@ -834,7 +888,7 @@ void env_universal_t::read_message_internal(int fd, callback_data_list_t *callba { if (utf8_to_wchar_string(line, &wide_line)) { - this->parse_message_internal(wide_line, callbacks); + env_universal_t::parse_message_internal(wide_line, &result, &storage); } line.clear(); } @@ -845,14 +899,14 @@ void env_universal_t::read_message_internal(int fd, callback_data_list_t *callba } // We make no effort to handle an unterminated last line + return result; } /** Parse message msg */ -void env_universal_t::parse_message_internal(const wcstring &msgstr, callback_data_list_t *callbacks) +void env_universal_t::parse_message_internal(const wcstring &msgstr, var_table_t *vars, wcstring *storage) { - ASSERT_IS_LOCKED(lock); const wchar_t *msg = msgstr.c_str(); // debug( 3, L"parse_message( %ls );", msg ); @@ -860,10 +914,12 @@ void env_universal_t::parse_message_internal(const wcstring &msgstr, callback_da if (msg[0] == L'#') return; - if (match(msg, SET_STR) || match(msg, SET_EXPORT_STR)) + bool is_set_export = match(msg, SET_EXPORT_STR); + bool is_set = ! is_set_export && match(msg, SET_STR); + if (is_set || is_set_export) { const wchar_t *name, *tmp; - bool exportv = match(msg, SET_EXPORT_STR); + const bool exportv = is_set_export; name = msg+(exportv?wcslen(SET_EXPORT_STR):wcslen(SET_STR)); while (name[0] == L'\t' || name[0] == L' ') @@ -872,16 +928,16 @@ void env_universal_t::parse_message_internal(const wcstring &msgstr, callback_da tmp = wcschr(name, L':'); if (tmp) { - const wcstring key(name, tmp - name); + /* Use 'storage' to hold our key to avoid allocations */ + storage->assign(name, tmp - name); + const wcstring &key = *storage; wcstring val; if (unescape_string(tmp + 1, &val, 0)) { - this->set_internal(key, val, exportv, false); - if (callbacks != NULL) - { - callbacks->push_back(callback_data_t(exportv ? SET_EXPORT:SET, key, val)); - } + var_entry_t &entry = (*vars)[key]; + entry.exportv = exportv; + entry.val.swap(val); //acquire the value } } else diff --git a/env_universal_common.h b/env_universal_common.h index c827716e1..207e4dba9 100644 --- a/env_universal_common.h +++ b/env_universal_common.h @@ -15,7 +15,8 @@ typedef enum { SET, - SET_EXPORT + SET_EXPORT, + ERASE } fish_message_type_t; /** @@ -55,9 +56,6 @@ class env_universal_t bool tried_renaming; bool load_from_path(const wcstring &path, callback_data_list_t *callbacks); void load_from_fd(int fd, callback_data_list_t *callbacks); - void erase_unmodified_values(); - - void parse_message_internal(const wcstring &msg, callback_data_list_t *callbacks); void set_internal(const wcstring &key, const wcstring &val, bool exportv, bool overwrite); bool remove_internal(const wcstring &name); @@ -71,7 +69,14 @@ class env_universal_t /* File id from which we last read */ file_id_t last_read_file; - void read_message_internal(int fd, callback_data_list_t *callbacks); + /* Given a variable table, generate callbacks representing the difference between our vars and the new vars */ + void generate_callbacks(const var_table_t &new_vars, callback_data_list_t *callbacks) const; + + /* Given a variable table, copy unmodified values into self. May destructively modified vars_to_acquire. */ + void acquire_variables(var_table_t *vars_to_acquire); + + static void parse_message_internal(const wcstring &msg, var_table_t *vars, wcstring *storage); + static var_table_t read_message_internal(int fd); public: env_universal_t(const wcstring &path); diff --git a/fish_tests.cpp b/fish_tests.cpp index 939dafb48..b29f7124a 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -2255,10 +2255,67 @@ static void test_universal() } } - if (system("rm -Rf /tmp/fish_uvars_test")) err(L"rrm failed"); + if (system("rm -Rf /tmp/fish_uvars_test")) err(L"rm failed"); putc('\n', stderr); } +static bool callback_data_less_than(const callback_data_t &a, const callback_data_t &b) { + return a.key < b.key; +} + +static void test_universal_callbacks() +{ + say(L"Testing universal callbacks"); + if (system("mkdir -p /tmp/fish_uvars_test/")) err(L"mkdir failed"); + env_universal_t uvars1(UVARS_TEST_PATH); + env_universal_t uvars2(UVARS_TEST_PATH); + + /* Put some variables into both */ + uvars1.set(L"alpha", L"1", false); + uvars1.set(L"beta", L"1", false); + uvars1.set(L"delta", L"1", false); + uvars1.set(L"epsilon", L"1", false); + uvars1.set(L"lambda", L"1", false); + uvars1.set(L"kappa", L"1", false); + uvars1.set(L"omicron", L"1", false); + + uvars1.sync(NULL); + uvars2.sync(NULL); + + /* Change uvars1 */ + uvars1.set(L"alpha", L"2", false); //changes value + uvars1.set(L"beta", L"1", true); //changes export + uvars1.remove(L"delta"); //erases value + uvars1.set(L"epsilon", L"1", false); //changes nothing + uvars1.sync(NULL); + + /* Change uvars2. It should treat its value as correct and ignore changes from uvars1. */ + uvars2.set(L"lambda", L"1", false); //same value + uvars2.set(L"kappa", L"2", false); //different value + + /* Now see what uvars2 sees */ + callback_data_list_t callbacks; + uvars2.sync(&callbacks); + + /* Sort them to get them in a predictable order */ + std::sort(callbacks.begin(), callbacks.end(), callback_data_less_than); + + /* Should see exactly two 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(1).type == SET_EXPORT); + do_test(callbacks.at(1).key == L"beta"); + do_test(callbacks.at(1).val == L"1"); + do_test(callbacks.at(2).type == ERASE); + do_test(callbacks.at(2).key == L"delta"); + do_test(callbacks.at(2).val == L""); + + + if (system("rm -Rf /tmp/fish_uvars_test")) err(L"rm failed"); +} + bool poll_notifier(universal_notifier_t *note) { bool result = false; @@ -3449,7 +3506,8 @@ int main(int argc, char **argv) if (should_test_function("complete")) test_complete(); if (should_test_function("input")) test_input(); if (should_test_function("universal")) test_universal(); - if (should_test_function("universal_notifiers")) test_universal_notifiers(); + if (should_test_function("universal")) test_universal_callbacks(); + if (should_test_function("notifiers")) test_universal_notifiers(); if (should_test_function("completion_insertions")) test_completion_insertions(); if (should_test_function("autosuggestion_combining")) test_autosuggestion_combining(); if (should_test_function("autosuggest_suggest_special")) test_autosuggest_suggest_special();