Simplify history search

Remove features related to navigating forwards in history that are no
longer used.
This commit is contained in:
ridiculousfish 2018-08-12 00:30:57 -07:00
parent e51e854d8d
commit d87f00bdc9
4 changed files with 132 additions and 150 deletions

View File

@ -2728,19 +2728,17 @@ static void test_autosuggestion_combining() {
do_test(combine_command_and_autosuggestion(L"alpha", L"ALPHA") == L"alpha");
}
static void test_history_matches(history_search_t &search, size_t matches, unsigned from_line) {
size_t i;
for (i = 0; i < matches; i++) {
do_test(search.go_backwards());
static void test_history_matches(history_search_t &search, const wcstring_list_t &expected,
unsigned from_line) {
wcstring_list_t found;
while (search.go_backwards()) {
found.push_back(search.current_string());
}
// do_test_from(!search.go_backwards(), from_line);
bool result = search.go_backwards();
do_test_from(!result, from_line);
for (i = 1; i < matches; i++) {
do_test_from(search.go_forwards(), from_line);
do_test_from(expected == found, from_line);
if (expected != found) {
fprintf(stderr, "Expected %ls, found %ls\n", comma_join(expected).c_str(),
comma_join(found).c_str());
}
do_test_from(!search.go_forwards(), from_line);
}
static bool history_contains(history_t *history, const wcstring &txt) {
@ -3028,64 +3026,82 @@ static wcstring random_string() {
return result;
}
// Helper to lowercase a string.
static wcstring lower(const wcstring &s) {
wcstring result;
for (wchar_t c : s) {
result.push_back(towlower(c));
}
return result;
}
void history_tests_t::test_history() {
history_search_t searcher;
say(L"Testing history");
const wcstring_list_t items = {L"Gamma", L"beta", L"BetA", L"Beta", L"alpha",
L"AlphA", L"Alpha", L"alph", L"ALPH", L"ZZZ"};
const history_search_flags_t nocase = history_search_ignore_case;
// Populate a history.
history_t &history = history_t::history_with_name(L"test_history");
history.clear();
history.add(L"Gamma");
history.add(L"beta");
history.add(L"BetA");
history.add(L"Beta");
history.add(L"alpha");
history.add(L"AlphA");
history.add(L"Alpha");
history.add(L"alph");
history.add(L"ALPH");
history.add(L"ZZZ");
for (const wcstring &s : items) {
history.add(s);
}
// Helper to set expected items to those matching a predicate, in reverse order.
wcstring_list_t expected;
auto set_expected = [&](const std::function<bool(const wcstring &)> &filt) {
expected.clear();
for (const auto &s : items) {
if (filt(s)) expected.push_back(s);
}
std::reverse(expected.begin(), expected.end());
};
// Items matching "a", case-sensitive.
searcher = history_search_t(history, L"a");
test_history_matches(searcher, 6, __LINE__);
do_test(searcher.current_string() == L"alph");
set_expected([](const wcstring &s) { return s.find(L'a') != wcstring::npos; });
test_history_matches(searcher, expected, __LINE__);
// Items matching "alpha", case-insensitive.
searcher = history_search_t(history, L"AlPhA", HISTORY_SEARCH_TYPE_CONTAINS, false);
test_history_matches(searcher, 3, __LINE__);
do_test(searcher.current_string() == L"Alpha");
searcher = history_search_t(history, L"AlPhA", HISTORY_SEARCH_TYPE_CONTAINS, nocase);
set_expected([](const wcstring &s) { return lower(s).find(L"alpha") != wcstring::npos; });
test_history_matches(searcher, expected, __LINE__);
// Items matching "et", case-sensitive.
searcher = history_search_t(history, L"et");
test_history_matches(searcher, 3, __LINE__);
do_test(searcher.current_string() == L"Beta");
set_expected([](const wcstring &s) { return s.find(L"et") != wcstring::npos; });
test_history_matches(searcher, expected, __LINE__);
// Items starting with "be", case-sensitive.
searcher = history_search_t(history, L"be", HISTORY_SEARCH_TYPE_PREFIX, true);
test_history_matches(searcher, 1, __LINE__);
do_test(searcher.current_string() == L"beta");
searcher = history_search_t(history, L"be", HISTORY_SEARCH_TYPE_PREFIX, 0);
set_expected([](const wcstring &s) { return string_prefixes_string(L"be", s); });
test_history_matches(searcher, expected, __LINE__);
// Items starting with "be", case-insensitive.
searcher = history_search_t(history, L"be", HISTORY_SEARCH_TYPE_PREFIX, false);
test_history_matches(searcher, 3, __LINE__);
do_test(searcher.current_string() == L"Beta");
searcher = history_search_t(history, L"be", HISTORY_SEARCH_TYPE_PREFIX, nocase);
set_expected(
[](const wcstring &s) { return string_prefixes_string_case_insensitive(L"be", s); });
test_history_matches(searcher, expected, __LINE__);
// Items exactly matching "alph", case-sensitive.
searcher = history_search_t(history, L"alph", HISTORY_SEARCH_TYPE_EXACT, true);
test_history_matches(searcher, 1, __LINE__);
do_test(searcher.current_string() == L"alph");
searcher = history_search_t(history, L"alph", HISTORY_SEARCH_TYPE_EXACT, 0);
set_expected([](const wcstring &s) { return s == L"alph"; });
test_history_matches(searcher, expected, __LINE__);
// Items exactly matching "alph", case-insensitive.
searcher = history_search_t(history, L"alph", HISTORY_SEARCH_TYPE_EXACT, false);
test_history_matches(searcher, 2, __LINE__);
do_test(searcher.current_string() == L"ALPH");
searcher = history_search_t(history, L"alph", HISTORY_SEARCH_TYPE_EXACT, nocase);
set_expected([](const wcstring &s) { return lower(s) == L"alph"; });
test_history_matches(searcher, expected, __LINE__);
// Test item removal case-sensitive.
searcher = history_search_t(history, L"Alpha");
test_history_matches(searcher, 1, __LINE__);
test_history_matches(searcher, {L"Alpha"}, __LINE__);
history.remove(L"Alpha");
searcher = history_search_t(history, L"Alpha");
test_history_matches(searcher, 0, __LINE__);
test_history_matches(searcher, {}, __LINE__);
// Test history escaping and unescaping, yaml, etc.
history_item_list_t before, after;

View File

@ -2,7 +2,6 @@
#include "config.h" // IWYU pragma: keep
#include <ctype.h>
#include <cstdint>
#include <errno.h>
#include <fcntl.h>
#include <pthread.h>
@ -10,6 +9,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <cstdint>
// We need the sys/file.h for the flock() declaration on Linux but not OS X.
#include <sys/file.h> // IWYU pragma: keep
#include <sys/mman.h>
@ -564,7 +564,7 @@ history_item_t::history_item_t(const wcstring &str, time_t when, history_identif
bool history_item_t::matches_search(const wcstring &term, enum history_search_type_t type,
bool case_sensitive) const {
// Note that this->term has already been lowercased when constructing the
// Note that 'term' has already been lowercased when constructing the
// search object if we're doing a case insensitive search.
const wcstring &content_to_match = case_sensitive ? contents : contents_lower;
@ -1083,74 +1083,47 @@ void history_t::load_old_if_needed() {
}
}
void history_search_t::skip_matches(const wcstring_list_t &skips) {
external_skips = skips;
std::sort(external_skips.begin(), external_skips.end());
}
bool history_search_t::should_skip_match(const wcstring &str) const {
return std::binary_search(external_skips.begin(), external_skips.end(), str);
}
bool history_search_t::go_forwards() {
// Pop the top index (if more than one) and return if we have any left.
if (prev_matches.size() > 1) {
prev_matches.pop_back();
return true;
}
return false;
}
bool history_search_t::go_backwards() {
// Backwards means increasing our index.
const size_t max_idx = (size_t)-1;
size_t idx = 0;
if (!prev_matches.empty()) idx = prev_matches.back().first;
if (idx == max_idx) return false;
const size_t max_index = (size_t)-1;
if (current_index_ == max_index) return false;
const bool main_thread = is_main_thread();
while (++idx < max_idx) {
size_t index = current_index_;
while (++index < max_index) {
if (main_thread ? reader_interrupted() : reader_thread_job_is_stale()) {
return false;
}
const history_item_t item = history->item_at_index(idx);
history_item_t item = history_->item_at_index(index);
// We're done if it's empty or we cancelled.
if (item.empty()) {
return false;
}
// Look for a term that matches and that we haven't seen before.
const wcstring &str = item.str();
if (item.matches_search(term, search_type, case_sensitive) && !match_already_made(str) &&
!should_skip_match(str)) {
prev_matches.push_back(prev_match_t(idx, item));
return true;
// Look for an item that matches and (if deduping) that we haven't seen before.
if (!item.matches_search(canon_term_, search_type_, !ignores_case())) {
continue;
}
// Skip if deduplicating.
if (dedup() && !deduper_.insert(item.str()).second) {
continue;
}
// This is our new item.
current_item_ = std::move(item);
current_index_ = index;
return true;
}
return false;
}
/// Goes to the end (forwards).
void history_search_t::go_to_end() { prev_matches.clear(); }
/// Returns if we are at the end, which is where we start.
bool history_search_t::is_at_end() const { return prev_matches.empty(); }
/// Goes to the beginning (backwards).
void history_search_t::go_to_beginning() {
// Go backwards as far as we can.
while (go_backwards()) { //!OCLINT(empty while statement)
// Do nothing.
}
}
history_item_t history_search_t::current_item() const {
assert(!prev_matches.empty()); //!OCLINT(double negative)
return prev_matches.back().second;
assert(current_item_ && "No current item");
return *current_item_;
}
wcstring history_search_t::current_string() const {
@ -1158,14 +1131,6 @@ wcstring history_search_t::current_string() const {
return item.str();
}
bool history_search_t::match_already_made(const wcstring &match) const {
for (std::vector<prev_match_t>::const_iterator iter = prev_matches.begin();
iter != prev_matches.end(); ++iter) {
if (iter->second.str() == match) return true;
}
return false;
}
static void replace_all(std::string *str, const char *needle, const char *replacement) {
size_t needle_len = strlen(needle), replacement_len = strlen(replacement);
size_t offset = 0;
@ -1633,15 +1598,13 @@ bool history_t::search_with_args(history_search_type_t search_type, wcstring_lis
size_t hist_size = this->size();
if (max_items > hist_size) max_items = hist_size;
for (wcstring_list_t::const_iterator iter = search_args.begin(); iter != search_args.end();
++iter) {
const wcstring &search_string = *iter;
for (const wcstring &search_string : search_args) {
if (search_string.empty()) {
streams.err.append_format(L"Searching for the empty string isn't allowed");
return false;
}
history_search_t searcher =
history_search_t(*this, search_string, search_type, case_sensitive);
history_search_t searcher = history_search_t(
*this, search_string, search_type, case_sensitive ? 0 : history_search_ignore_case);
while (searcher.go_backwards()) {
wcstring result;
auto cur_item = searcher.current_item();
@ -1908,8 +1871,9 @@ wcstring history_session_id() {
} else if (valid_var_name(session_id)) {
result = session_id;
} else {
debug(0, _(L"History session ID '%ls' is not a valid variable name. "
L"Falling back to `%ls`."),
debug(0,
_(L"History session ID '%ls' is not a valid variable name. "
L"Falling back to `%ls`."),
session_id.c_str(), result.c_str());
}
}

View File

@ -241,6 +241,7 @@ class history_t {
bool search(history_search_type_t search_type, wcstring_list_t search_args,
const wchar_t *show_time_format, size_t max_items, bool case_sensitive,
bool null_terminate, bool reverse, io_streams_t &streams);
bool search_with_args(history_search_type_t search_type, wcstring_list_t search_args,
const wchar_t *show_time_format, size_t max_items, bool case_sensitive,
bool null_terminate, bool reverse, io_streams_t &streams);
@ -281,52 +282,57 @@ class history_t {
size_t size();
};
/// Flags for history searching.
enum {
// If set, ignore case.
history_search_ignore_case = 1 << 0,
// If set, do not deduplicate, which can help performance.
history_search_no_dedup = 1 << 1
};
using history_search_flags_t = uint32_t;
/// Support for searching a history backwards.
/// Note this does NOT de-duplicate; it is the caller's responsibility to do so.
class history_search_t {
private:
// The history in which we are searching.
history_t *history;
history_t *history_;
// The search term.
wcstring term;
// The original search term.
wcstring orig_term_;
// The (possibly lowercased) search term.
wcstring canon_term_;
// Our search type.
enum history_search_type_t search_type;
bool case_sensitive;
enum history_search_type_t search_type_ { HISTORY_SEARCH_TYPE_CONTAINS };
// Our list of previous matches as index, value. The end is the current match.
typedef std::pair<size_t, history_item_t> prev_match_t;
std::vector<prev_match_t> prev_matches;
// Our flags.
history_search_flags_t flags_{0};
// Returns yes if a given term is in prev_matches.
bool match_already_made(const wcstring &match) const;
// The current history item.
maybe_t<history_item_t> current_item_;
// Additional strings to skip (sorted).
wcstring_list_t external_skips;
// Index of the current history item.
size_t current_index_{0};
bool should_skip_match(const wcstring &str) const;
// If deduping, the items we've seen.
std::unordered_set<wcstring> deduper_;
// return whether we are case insensitive.
bool ignores_case() const { return flags_ & history_search_ignore_case; }
// return whether we deduplicate items.
bool dedup() const { return !(flags_ & history_search_no_dedup); }
public:
// Gets the search term.
const wcstring &get_term() const { return term; }
// Sets additional string matches to skip.
void skip_matches(const wcstring_list_t &skips);
// Finds the next search term (forwards in time). Returns true if one was found.
bool go_forwards();
// Gets the original search term.
const wcstring &original_term() const { return orig_term_; }
// Finds the previous search result (backwards in time). Returns true if one was found.
bool go_backwards();
// Goes to the end (forwards).
void go_to_end();
// Returns if we are at the end. We start out at the end.
bool is_at_end() const;
// Goes to the beginning (backwards).
void go_to_beginning();
// Returns the current search result item. asserts if there is no current item.
history_item_t current_item() const;
@ -336,19 +342,15 @@ class history_search_t {
// Constructor.
history_search_t(history_t &hist, const wcstring &str,
enum history_search_type_t type = HISTORY_SEARCH_TYPE_CONTAINS,
bool case_sensitive = true)
: history(&hist), term(str), search_type(type), case_sensitive(case_sensitive) {
if (!case_sensitive) {
term = wcstring();
for (wcstring::const_iterator it = str.begin(); it != str.end(); ++it) {
term.push_back(towlower(*it));
}
history_search_flags_t flags = 0)
: history_(&hist), orig_term_(str), canon_term_(str), search_type_(type), flags_(flags) {
if (ignores_case()) {
std::transform(canon_term_.begin(), canon_term_.end(), canon_term_.begin(), towlower);
}
}
// Default constructor.
history_search_t()
: history(), term(), search_type(HISTORY_SEARCH_TYPE_CONTAINS), case_sensitive(true) {}
history_search_t() = default;
};
/// Saves the new history to disk.

View File

@ -179,8 +179,6 @@ class reader_history_search_t {
/// Attempt to append matches from the current history item.
/// \return true if something was appended.
bool append_matches_from_search() {
if (search_.is_at_end()) return false;
const size_t before = matches_.size();
wcstring text = search_.current_string();
if (mode_ == line) {
@ -260,7 +258,7 @@ class reader_history_search_t {
}
/// \return the string we are searching for.
const wcstring &search_string() const { return search_.get_term(); }
const wcstring &search_string() const { return search_.original_term(); }
/// \return whether we are at the end (most recent) of our search.
bool is_at_end() const { return match_index_ == 0; }
@ -277,7 +275,9 @@ class reader_history_search_t {
matches_ = {text};
match_index_ = 0;
mode_ = mode;
search_ = history_search_t(*hist, text);
// We can skip dedup in history_search_t because we do it ourselves in skips_.
search_ =
history_search_t(*hist, text, HISTORY_SEARCH_TYPE_CONTAINS, history_search_no_dedup);
}
/// Reset to inactive search.