diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 0a2153a18..379a475bc 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -4406,7 +4406,7 @@ static bool history_equals(history_t &hist, const wchar_t *const *strings) { history_item_t item = hist.item_at_index(history_idx); if (expected == NULL) { if (!item.empty()) { - err(L"Expected empty item at history index %lu", history_idx); + err(L"Expected empty item at history index %lu, instead found: %ls", history_idx, item.str().c_str()); } break; } else { diff --git a/src/history.cpp b/src/history.cpp index 0f742907f..1963e8cfc 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -172,22 +172,28 @@ class history_lru_cache_t : public lru_cache_tcontents == item.contents) { - this->creation_timestamp = std::max(this->creation_timestamp, item.creation_timestamp); - if (this->required_paths.size() < item.required_paths.size()) { - this->required_paths = item.required_paths; - } - if (this->identifier < item.identifier) { - this->identifier = item.identifier; - } - result = true; + // We can only merge items if they agree on their text and persistence mode. + if (this->contents != item.contents || this->persist_mode != item.persist_mode) { + return false; } - return result; + + // Ok, merge this item. + this->creation_timestamp = std::max(this->creation_timestamp, item.creation_timestamp); + if (this->required_paths.size() < item.required_paths.size()) { + this->required_paths = item.required_paths; + } + if (this->identifier < item.identifier) { + this->identifier = item.identifier; + } + return true; } -history_item_t::history_item_t(wcstring str, time_t when, history_identifier_t ident) - : contents(trim(std::move(str))), creation_timestamp(when), identifier(ident) {} +history_item_t::history_item_t(wcstring str, time_t when, history_identifier_t ident, + history_persistence_mode_t persist_mode) + : contents(std::move(str)), + creation_timestamp(when), + identifier(ident), + persist_mode(persist_mode) {} bool history_item_t::matches_search(const wcstring &term, enum history_search_type_t type, bool case_sensitive) const { @@ -228,9 +234,11 @@ bool history_item_t::matches_search(const wcstring &term, enum history_search_ty } struct history_impl_t { - // Privately add an item. If pending, the item will not be returned by history searches until a - // call to resolve_pending. - void add(const history_item_t &item, bool pending = false, bool do_save = true); + // Add a new history item to the end. If pending is set, the item will not be returned by + // item_at_index until a call to resolve_pending(). Pending items are tracked with an offset + // into the array of new items, so adding a non-pending item has the effect of resolving all + // pending items. + void add(history_item_t item, bool pending = false, bool do_save = true); // Internal function. void clear_file_state(); @@ -266,7 +274,10 @@ struct history_impl_t { // the boundary are considered "old". Items whose timestemps are > the boundary are new, and are // ignored by this instance (unless they came from this instance). The timestamp may be adjusted // by incorporate_external_changes(). - time_t boundary_timestamp{time(nullptr)}; + time_t boundary_timestamp{}; + + /// The most recent "unique" identifier for a history item. + history_identifier_t last_identifier{0}; // How many items we add until the next vacuum. Initially a random value. int countdown_to_vacuum{-1}; @@ -277,6 +288,12 @@ struct history_impl_t { // List of old items, as offsets into out mmap data. std::deque old_item_offsets{}; + /// \return a timestamp for new items - see the implementation for a subtlety. + time_t timestamp_now() const; + + /// \return a new item identifier, incrementing our counter. + history_identifier_t next_identifier() { return ++last_identifier; } + // Figure out the offsets of our file contents. void populate_from_file_contents(); @@ -286,6 +303,11 @@ struct history_impl_t { // Deletes duplicates in new_items. void compact_new_items(); + // Removes trailing ephemeral items. + // Ephemeral items have leading spaces, and can only be retrieved immediately; adding any item + // removes them. + void remove_ephemeral_items(); + // Attempts to rewrite the existing file to a target temporary file // Returns false on error, true on success bool rewrite_to_temporary_file(int existing_fd, int dst_fd) const; @@ -302,7 +324,9 @@ struct history_impl_t { // Saves history unless doing so is disabled. void save_unless_disabled(); - explicit history_impl_t(wcstring name) : name(std::move(name)) {} + explicit history_impl_t(wcstring name) + : name(std::move(name)), boundary_timestamp(time(nullptr)) {} + history_impl_t(history_impl_t &&) = default; ~history_impl_t() = default; @@ -313,20 +337,9 @@ struct history_impl_t { // require populating the history. bool is_empty(); - // Add a new history item to the end. If pending is set, the item will not be returned by - // item_at_index until a call to resolve_pending(). Pending items are tracked with an offset - // into the array of new items, so adding a non-pending item has the effect of resolving all - // pending items. - void add(const wcstring &str, history_identifier_t ident = 0, bool pending = false, - bool save = true); - // Remove a history item. void remove(const wcstring &str); - // Add a new pending history item to the end, and then begin file detection on the items to - // determine which arguments are paths - void add_pending_with_file_detection(const wcstring &str, const wcstring &working_dir_slash); - // Resolves any pending history items, so that they may be returned in history searches. void resolve_pending(); @@ -366,12 +379,14 @@ struct history_impl_t { size_t size(); }; -void history_impl_t::add(const history_item_t &item, bool pending, bool do_save) { +void history_impl_t::add(history_item_t item, bool pending, bool do_save) { + assert(item.timestamp() != 0 && "Should not add an item with a 0 timestamp"); // We use empty items as sentinels to indicate the end of history. // Do not allow them to be added (#6032). if (item.contents.empty()) { return; } + // Try merging with the last item. if (!new_items.empty() && new_items.back().merge(item)) { // We merged, so we don't have to add anything. Maybe this item was pending, but it just got @@ -422,20 +437,6 @@ void history_impl_t::save_unless_disabled() { countdown_to_vacuum--; } -void history_impl_t::add(const wcstring &str, history_identifier_t ident, bool pending, - bool do_save) { - time_t when = time(nullptr); - // Big hack: do not allow timestamps equal to our boundary date. This is because we include - // items whose timestamps are equal to our boundary when reading old history, so we can catch - // "just closed" items. But this means that we may interpret our own items, that we just wrote, - // as old items, if we wrote them in the same second as our birthdate. - if (when == this->boundary_timestamp) { - when++; - } - - this->add(history_item_t(str, when, ident), pending, do_save); -} - // Remove matching history entries from our list of new items. This only supports literal, // case-sensitive, matches. void history_impl_t::remove(const wcstring &str_to_remove) { @@ -557,6 +558,18 @@ std::unordered_map history_impl_t::items_at_indexes(const std::v return result; } +time_t history_impl_t::timestamp_now() const { + time_t when = time(nullptr); + // Big hack: do not allow timestamps equal to our boundary date. This is because we include + // items whose timestamps are equal to our boundary when reading old history, so we can catch + // "just closed" items. But this means that we may interpret our own items, that we just wrote, + // as old items, if we wrote them in the same second as our birthdate. + if (when == this->boundary_timestamp) { + when++; + } + return when; +} + void history_impl_t::populate_from_file_contents() { old_item_offsets.clear(); if (file_contents) { @@ -648,12 +661,15 @@ void history_impl_t::clear_file_state() { } void history_impl_t::compact_new_items() { - // Keep only the most recent items with the given contents. This algorithm could be made more - // efficient, but likely would consume more memory too. + // Keep only the most recent items with the given contents. std::unordered_set seen; size_t idx = new_items.size(); while (idx--) { const history_item_t &item = new_items[idx]; + + // Only compact persisted items. + if (!item.should_write_to_disk()) continue; + if (!seen.insert(item.contents).second) { // This item was not inserted because it was already in the set, so delete the item at // this index. @@ -668,6 +684,14 @@ void history_impl_t::compact_new_items() { } } +void history_impl_t::remove_ephemeral_items() { + while (!new_items.empty() && + new_items.back().persist_mode == history_persistence_mode_t::ephemeral) { + new_items.pop_back(); + } + first_unwritten_new_item_index = std::min(first_unwritten_new_item_index, new_items.size()); +} + // Given the fd of an existing history file, or -1 if none, write // a new history file to temp_fd. Returns true on success, false // on error @@ -698,7 +722,9 @@ bool history_impl_t::rewrite_to_temporary_file(int existing_fd, int dst_fd) cons // Insert any unwritten new items for (auto iter = new_items.cbegin() + this->first_unwritten_new_item_index; iter != new_items.cend(); ++iter) { - lru.add_item(*iter); + if (iter->should_write_to_disk()) { + lru.add_item(*iter); + } } // Stable-sort our items by timestamp @@ -920,10 +946,12 @@ bool history_impl_t::save_internal_via_appending() { std::string buffer; while (first_unwritten_new_item_index < new_items.size()) { const history_item_t &item = new_items.at(first_unwritten_new_item_index); - append_history_item_to_buffer(item, &buffer); - err = flush_to_fd(&buffer, history_fd.fd(), HISTORY_OUTPUT_BUFFER_SIZE); - if (err) break; - // We wrote this item, hooray. + if (item.should_write_to_disk()) { + append_history_item_to_buffer(item, &buffer); + err = flush_to_fd(&buffer, history_fd.fd(), HISTORY_OUTPUT_BUFFER_SIZE); + if (err) break; + } + // We wrote or skipped this item, hooray. first_unwritten_new_item_index++; } @@ -1057,6 +1085,12 @@ bool history_impl_t::is_empty() { return empty; } +void history_t::add(wcstring str) { + auto imp = this->impl(); + time_t when = imp->timestamp_now(); + imp->add(history_item_t(std::move(str), when)); +} + /// Populates from older location (in config path, rather than data path) This is accomplished by /// clearing ourselves, and copying the contents of the old history file to the new history file. /// The new contents will automatically be re-mapped later. @@ -1132,6 +1166,8 @@ static bool should_import_bash_history_line(const wcstring &line) { /// encode multiline commands. void history_impl_t::populate_from_bash(FILE *stream) { // Process the entire history file until EOF is observed. + // Pretend all items were created at this time. + const auto when = this->timestamp_now(); bool eof = false; while (!eof) { auto line = std::string(); @@ -1151,10 +1187,11 @@ void history_impl_t::populate_from_bash(FILE *stream) { if (a_newline) break; } - wcstring wide_line = str2wcstring(line); + wcstring wide_line = trim(str2wcstring(line)); // Add this line if it doesn't contain anything we know we can't handle. if (should_import_bash_history_line(wide_line)) { - this->add(wide_line, 0, false /* pending */, false /* do_save */); + this->add(history_item_t(std::move(wide_line), when), false /* pending */, + false /* do_save */); } } this->save_unless_disabled(); @@ -1163,7 +1200,7 @@ void history_impl_t::populate_from_bash(FILE *stream) { void history_impl_t::incorporate_external_changes() { // To incorporate new items, we simply update our timestamp to now, so that items from previous // instances get added. We then clear the file state so that we remap the file. Note that this - // is somehwhat expensive because we will be going back over old items. An optimization would be + // is somewhat expensive because we will be going back over old items. An optimization would be // to preserve old_item_offsets so that they don't have to be recomputed. (However, then items // *deleted* in other instances would not show up here). time_t new_timestamp = time(nullptr); @@ -1174,9 +1211,11 @@ void history_impl_t::incorporate_external_changes() { this->boundary_timestamp = new_timestamp; this->clear_file_state(); - // We also need to erase new_items, since we go through those first, and that means we + // We also need to erase new items, since we go through those first, and that means we // will not properly interleave them with items from other instances. // We'll pick them up from the file (#2312) + // TODO: this will drop items that had no_persist set, how can we avoid that while still + // properly interleaving? this->save(false); this->new_items.clear(); this->first_unwritten_new_item_index = 0; @@ -1258,16 +1297,15 @@ bool history_t::is_default() const { return impl()->is_default(); } bool history_t::is_empty() { return impl()->is_empty(); } -void history_t::add(const history_item_t &item, bool pending) { impl()->add(item, pending); } - -void history_t::add(const wcstring &str, history_identifier_t ident, bool pending) { - impl()->add(str, ident, pending); -} +void history_t::add(history_item_t item, bool pending) { impl()->add(std::move(item), pending); } void history_t::remove(const wcstring &str) { impl()->remove(str); } +void history_t::remove_ephemeral_items() { impl()->remove_ephemeral_items(); } + void history_t::add_pending_with_file_detection(const wcstring &str, - const wcstring &working_dir_slash) { + const wcstring &working_dir_slash, + history_persistence_mode_t persist_mode) { // We use empty items as sentinels to indicate the end of history. // Do not allow them to be added (#6032). if (str.empty()) { @@ -1311,17 +1349,18 @@ void history_t::add_pending_with_file_detection(const wcstring &str, bool wants_file_detection = !potential_paths.empty() && !needs_sync_write; auto imp = this->impl(); - history_identifier_t identifier = 0; + // Make our history item. + time_t when = imp->timestamp_now(); + history_identifier_t identifier = imp->next_identifier(); + history_item_t item{str, when, identifier, persist_mode}; + if (wants_file_detection) { - // Grab the next identifier. - static relaxed_atomic_t s_last_identifier{0}; - identifier = ++s_last_identifier; imp->disable_automatic_saving(); // Add the item. Then check for which paths are valid on a background thread, // and unblock the item. // Don't hold the lock while we perform this file detection. - imp->add(str, identifier, true /* pending */); + imp->add(std::move(item), true /* pending */); iothread_perform([=]() { auto validated_paths = valid_paths(potential_paths, working_dir_slash); auto imp = this->impl(); @@ -1332,7 +1371,7 @@ void history_t::add_pending_with_file_detection(const wcstring &str, // Add the item. // If we think we're about to exit, save immediately, regardless of any disabling. This may // cause us to lose file hinting for some commands, but it beats losing history items. - imp->add(str, identifier, true /* pending */); + imp->add(std::move(item), true /* pending */); if (needs_sync_write) { imp->save(); } diff --git a/src/history.h b/src/history.h index 0f06f4aa1..053a4b0d9 100644 --- a/src/history.h +++ b/src/history.h @@ -62,11 +62,20 @@ enum class history_search_type_t { typedef uint64_t history_identifier_t; +/// Ways that a history item may be written to disk (or omitted). +enum class history_persistence_mode_t : uint8_t { + disk, // the history item is written to disk normally + memory, // the history item is stored in-memory only, not written to disk + ephemeral, // the history item is stored in-memory and deleted when a new item is added +}; + class history_item_t { public: - /// Construct from a text, optional timestamp, and optional identifier. - explicit history_item_t(wcstring str = wcstring(), time_t when = 0, - history_identifier_t ident = 0); + /// Construct from a text, timestamp, and optional identifier. + /// If \p no_persist is set, then do not write this item to disk. + explicit history_item_t( + wcstring str = {}, time_t when = 0, history_identifier_t ident = 0, + history_persistence_mode_t persist_mode = history_persistence_mode_t::disk); /// \return the text as a string. const wcstring &str() const { return contents; } @@ -81,6 +90,9 @@ class history_item_t { /// \return the timestamp for creating this history item. time_t timestamp() const { return creation_timestamp; } + /// \return whether this item should be persisted (written to disk). + bool should_write_to_disk() const { return persist_mode == history_persistence_mode_t::disk; } + /// Get and set the list of arguments which referred to files. /// This is used for autosuggestion hinting. const path_list_t &get_required_paths() const { return required_paths; } @@ -96,11 +108,14 @@ class history_item_t { // Original creation time for the entry. time_t creation_timestamp; + // Paths that we require to be valid for this item to be autosuggested. + path_list_t required_paths; + // Sometimes unique identifier used for hinting. history_identifier_t identifier; - // Paths that we require to be valid for this item to be autosuggested. - path_list_t required_paths; + // If set, do not write this item to disk. + history_persistence_mode_t persist_mode; friend class history_t; friend struct history_impl_t; @@ -129,8 +144,11 @@ class history_t { acquired_lock impl() const; // Privately add an item. If pending, the item will not be returned by history searches until a - // call to resolve_pending. - void add(const history_item_t &item, bool pending = false); + // call to resolve_pending. Any trailing ephemeral items are dropped. + void add(history_item_t item, bool pending = false); + + // Add a new history item with text \p str to the end of history. + void add(wcstring str); public: explicit history_t(wcstring name); @@ -153,18 +171,17 @@ class history_t { // require populating the history. bool is_empty(); - // Add a new history item to the end. If pending is set, the item will not be returned by - // item_at_index until a call to resolve_pending(). Pending items are tracked with an offset - // into the array of new items, so adding a non-pending item has the effect of resolving all - // pending items. - void add(const wcstring &str, history_identifier_t ident = 0, bool pending = false); - // Remove a history item. void remove(const wcstring &str); + /// Remove any trailing ephemeral items. + void remove_ephemeral_items(); + // Add a new pending history item to the end, and then begin file detection on the items to - // determine which arguments are paths - void add_pending_with_file_detection(const wcstring &str, const wcstring &working_dir_slash); + // determine which arguments are paths. The item has the given \p persist_mode. + void add_pending_with_file_detection( + const wcstring &str, const wcstring &working_dir_slash, + history_persistence_mode_t persist_mode = history_persistence_mode_t::disk); // Resolves any pending history items, so that they may be returned in history searches. void resolve_pending(); diff --git a/src/history_file.cpp b/src/history_file.cpp index a3a9e8aed..7598191c1 100644 --- a/src/history_file.cpp +++ b/src/history_file.cpp @@ -431,6 +431,7 @@ static size_t offset_of_next_item_fish_2_0(const history_file_contents_t &conten } void append_history_item_to_buffer(const history_item_t &item, std::string *buffer) { + assert(item.should_write_to_disk() && "Item should not be persisted"); auto append = [=](const char *a, const char *b = nullptr, const char *c = nullptr) { if (a) buffer->append(a); if (b) buffer->append(b); diff --git a/src/reader.cpp b/src/reader.cpp index cf81c6710..a9f7d855f 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -1576,10 +1576,6 @@ void reader_data_t::completion_insert(const wcstring &val, size_t token_end, set_buffer_maintaining_pager(new_command_line, cursor); } -static bool may_add_to_history(const wcstring &commandline_prefix) { - return !commandline_prefix.empty() && commandline_prefix.at(0) != L' '; -} - // Returns a function that can be invoked (potentially // on a background thread) to determine the autosuggestion static std::function get_autosuggestion_performer( @@ -1603,21 +1599,20 @@ static std::function get_autosuggestion_performer( return nothing; } - if (may_add_to_history(search_string)) { - history_search_t searcher(*history, search_string, history_search_type_t::prefix, - history_search_flags_t{}); - while (!ctx.check_cancel() && searcher.go_backwards()) { - const history_item_t &item = searcher.current_item(); + // Search history for a matching item. + history_search_t searcher(*history, search_string, history_search_type_t::prefix, + history_search_flags_t{}); + while (!ctx.check_cancel() && searcher.go_backwards()) { + const history_item_t &item = searcher.current_item(); - // Skip items with newlines because they make terrible autosuggestions. - if (item.str().find(L'\n') != wcstring::npos) continue; + // Skip items with newlines because they make terrible autosuggestions. + if (item.str().find(L'\n') != wcstring::npos) continue; - if (autosuggest_validate_from_history(item, working_directory, ctx)) { - // The command autosuggestion was handled specially, so we're done. - // History items are case-sensitive, see #3978. - return autosuggestion_t{searcher.current_string(), search_string, - false /* icase */}; - } + if (autosuggest_validate_from_history(item, working_directory, ctx)) { + // The command autosuggestion was handled specially, so we're done. + // History items are case-sensitive, see #3978. + return autosuggestion_t{searcher.current_string(), search_string, + false /* icase */}; } } @@ -3158,11 +3153,28 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat } if (command_test_result == 0) { - // Finished command, execute it. Don't add items that start with a leading - // space, or if in silent mode (#7230). - const editable_line_t *el = &command_line; - if (history != nullptr && !conf.in_silent_mode && may_add_to_history(el->text())) { - history->add_pending_with_file_detection(el->text(), vars.get_pwd_slash()); + // Finished command, execute it. Don't add items in silent mode (#7230). + wcstring text = command_line.text(); + if (text.empty()) { + // Here the user just hit return. Make a new prompt, don't remove ephemeral + // items. + rls.finished = true; + break; + } + + // Historical behavior is to trim trailing spaces. + while (!text.empty() && text.back() == L' ') { + text.pop_back(); + } + if (history && !conf.in_silent_mode) { + // Remove ephemeral items. + // Note we fall into this case if the user just types a space and hits return. + history->remove_ephemeral_items(); + + // Mark this item as ephemeral if there is a leading space (#615). + auto mode = text.front() == L' ' ? history_persistence_mode_t::ephemeral + : history_persistence_mode_t::disk; + history->add_pending_with_file_detection(text, vars.get_pwd_slash(), mode); } rls.finished = true; update_buff_pos(&command_line, command_line.size());