Skip to content

Commit

Permalink
Universal variable callbacks should only be announced for changed
Browse files Browse the repository at this point in the history 
values, not every value. Also support erase notifications.
  • Loading branch information
@ridiculousfish
ridiculousfish committed Jun 16, 2014
1 parent 6277a2e commit 3513ce3
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 42 deletions.
7 changes: 7 additions & 0 deletions env.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
126 changes: 91 additions & 35 deletions env_universal_common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/* Skip modified values */
if (this->modified.find(key) != this->modified.end())
{
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;
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;
}

/* 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<wcstring>::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)
Expand All @@ -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;
}
}
Expand All @@ -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)
Expand Down Expand Up @@ -721,13 +779,6 @@ bool env_universal_t::sync(callback_data_list_t *callbacks)
return false;
}

#if 0
for (std::set<wcstring>::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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand All @@ -845,25 +899,27 @@ 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 );

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' ')
Expand All @@ -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
Expand Down
15 changes: 10 additions & 5 deletions env_universal_common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
typedef enum
{
SET,
SET_EXPORT
SET_EXPORT,
ERASE
} fish_message_type_t;

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
62 changes: 60 additions & 2 deletions fish_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 3513ce3

Please sign in to comment.