diff --git a/src/env.cpp b/src/env.cpp index abbd4b083..959d20d76 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -1364,8 +1364,8 @@ env_var_t env_get(const wcstring &key, env_mode_flags_t mode) { if (!history) { history = &history_t::history_with_name(history_session_id()); } - wcstring result; - if (history) history->get_string_representation(&result, ARRAY_SEP_STR); + wcstring_list_t result; + if (history) history->get_history(result); return env_var_t(L"history", result); } else if (key == L"status") { return env_var_t(L"status", to_string(proc_get_last_status())); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 1bb340c99..fd80f6e2c 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -3079,12 +3079,12 @@ void history_tests_t::test_history_merge(void) { } // Everyone should also have items in the same order (#2312) - wcstring string_rep; - hists[0]->get_string_representation(&string_rep, L"\n"); + wcstring_list_t hist_vals1; + hists[0]->get_history(hist_vals1); for (size_t i = 0; i < count; i++) { - wcstring string_rep2; - hists[i]->get_string_representation(&string_rep2, L"\n"); - do_test(string_rep == string_rep2); + wcstring_list_t hist_vals2; + hists[i]->get_history(hist_vals2); + do_test(hist_vals1 == hist_vals2); } // Add some more per-history items. diff --git a/src/history.cpp b/src/history.cpp index 71c678563..3f6863621 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -842,15 +842,12 @@ void history_t::set_valid_file_paths(const wcstring_list_t &valid_file_paths, } } -void history_t::get_string_representation(wcstring *result, const wcstring &separator) { +void history_t::get_history(wcstring_list_t &result) { scoped_lock locker(lock); - bool first = true; - - std::unordered_set seen; - // If we have a pending item, we skip the first encountered (i.e. last) new item. bool next_is_pending = this->has_pending_item; + std::unordered_set seen; // Append new items. Note that in principle we could use const_reverse_iterator, but we do not // because reverse_iterator is not convertible to const_reverse_iterator. See @@ -863,12 +860,7 @@ void history_t::get_string_representation(wcstring *result, const wcstring &sepa continue; } - // Skip duplicates. - if (!seen.insert(iter->str()).second) continue; - - if (!first) result->append(separator); - result->append(iter->str()); - first = false; + if (seen.insert(iter->str()).second) result.push_back(iter->str()); } // Append old items. @@ -879,12 +871,7 @@ void history_t::get_string_representation(wcstring *result, const wcstring &sepa const history_item_t item = decode_item(mmap_start + offset, mmap_length - offset, mmap_type); - // Skip duplicates. - if (!seen.insert(item.str()).second) continue; - - if (!first) result->append(separator); - result->append(item.str()); - first = false; + if (seen.insert(item.str()).second) result.push_back(item.str()); } } diff --git a/src/history.h b/src/history.h index 9ff339a32..4bdf752b1 100644 --- a/src/history.h +++ b/src/history.h @@ -257,9 +257,9 @@ class history_t { // Incorporates the history of other shells into this history. void incorporate_external_changes(); - // Gets all the history into a string. This is intended for the $history environment variable. + // Gets all the history into a list. This is intended for the $history environment variable. // This may be long! - void get_string_representation(wcstring *result, const wcstring &separator); + void get_history(wcstring_list_t &result); // Sets the valid file paths for the history item with the given identifier. void set_valid_file_paths(const wcstring_list_t &valid_file_paths, history_identifier_t ident); diff --git a/tests/history.expect b/tests/history.expect index 593691699..4c7c9630c 100644 --- a/tests/history.expect +++ b/tests/history.expect @@ -30,7 +30,7 @@ expect_prompt send "echo start1; builtin history; echo end1\r" expect_prompt -re {start1\r\nend1\r\n} { puts "empty history detected as expected" -} unmatched { +} timeout { puts stderr "empty history not detected as expected" } @@ -39,7 +39,7 @@ expect_prompt -re {start1\r\nend1\r\n} { send "echo start2; builtin history; echo end2\r" expect_prompt -re {start2\r\necho start1; builtin history; echo end1\r\nend2\r\n} { puts "first history command detected as expected" -} unmatched { +} timeout { puts stderr "first history command not detected as expected" } @@ -53,7 +53,7 @@ expect_prompt -re {start2\r\necho start1; builtin history; echo end1\r\nend2\r\n send "history search echo start\r" expect_prompt -re {\r\necho start1.*\r\necho start2} { puts "history function explicit search succeeded" -} unmatched { +} timeout { puts stderr "history function explicit search failed" } @@ -62,7 +62,7 @@ expect_prompt -re {\r\necho start1.*\r\necho start2} { send "history -p 'echo start'\r" expect_prompt -re {\r\necho start2.*\r\necho start1} { puts "history function implicit search succeeded" -} unmatched { +} timeout { puts stderr "history function implicit search failed" } @@ -71,7 +71,7 @@ expect_prompt -re {\r\necho start2.*\r\necho start1} { send "history search --show-time='# %F %T%n' --prefix 'echo start'\r" expect_prompt -re {\r\n# \d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d\r\necho start2; .*\r\n# \d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d\r\necho start1; } { puts "history function implicit search with timestamps succeeded" -} unmatched { +} timeout { puts stderr "history function implicit search with timestamps failed" } @@ -90,14 +90,14 @@ expect_prompt send "history search --exact 'echo goodbye'\r" expect_prompt -re {\r\necho goodbye\r\n} { puts "history function explicit exact search 'echo goodbye' succeeded" -} unmatched { +} timeout { puts stderr "history function explicit exact search 'echo goodbye' failed" } send "history search --exact 'echo hello'\r" expect_prompt -re {\r\necho hello\r\n} { puts "history function explicit exact search 'echo hello' succeeded" -} unmatched { +} timeout { puts stderr "history function explicit exact search 'echo hello' failed" } @@ -106,7 +106,7 @@ expect_prompt -re {\r\necho hello\r\n} { send "history search --exact 'echo hell'\r" expect_prompt -re {history search --exact 'echo hell'\r\n} { puts "history function explicit exact search 'echo hell' succeeded" -} unmatched { +} timeout { puts stderr "history function explicit exact search 'echo hell' failed" } @@ -117,7 +117,7 @@ expect -re {history delete -e -C 'echo hello'\r\n} send "echo count hello (history search -e -C 'echo hello' | wc -l | string trim)\r" expect -re {\r\ncount hello 0\r\n} { puts "history function explicit exact delete 'echo hello' succeeded" -} unmatched { +} timeout { puts stderr "history function explicit exact delete 'echo hello' failed" } @@ -139,13 +139,21 @@ expect -re {Deleting history entry 1: "echo hello AGAIN"\r\n} send "echo count AGAIN (history search -e -C 'echo hello AGAIN' | wc -l | string trim)\r" expect -re {\r\ncount AGAIN 0\r\n} { puts "history function explicit prefix delete 'echo hello AGAIN' succeeded" -} unmatched { +} timeout { puts stderr "history function explicit prefix delete 'echo hello AGAIN' failed" } send "echo count again (history search -e -C 'echo hello again' | wc -l | string trim)\r" expect -re {\r\ncount again 1\r\n} { puts "history function explicit exact search 'echo hello again' succeeded" -} unmatched { +} timeout { puts stderr "history function explicit exact search 'echo hello again' failed" } + +# Verify that the $history var has the expected content. +send "echo history2=\$history\[2\]\r" +expect -re {\r\nhistory2=echo count AGAIN .*\r\n} { + puts "history\[2\] had the correct data" +} timeout { + puts stderr "history\[2\] had the wrong data" +} diff --git a/tests/history.expect.out b/tests/history.expect.out index 7fe87edd7..6e495d4fa 100644 --- a/tests/history.expect.out +++ b/tests/history.expect.out @@ -9,3 +9,4 @@ history function explicit exact search 'echo hell' succeeded history function explicit exact delete 'echo hello' succeeded history function explicit prefix delete 'echo hello AGAIN' succeeded history function explicit exact search 'echo hello again' succeeded +history[2] had the correct data