From ee1d3106517c5fb31e290002028715535e8a9906 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Thu, 14 Sep 2017 15:44:17 -0700 Subject: [PATCH] Implement `history search --reverse` (#4375) * Implement `history search --reverse` It should be possible to have `history search` output ordered oldest to newest like nearly every other shell including bash, ksh, zsh, and csh. We can't make this the default because too many people expect the current behavior. This simply makes it possible for people to define their own abbreviations or functions that provide behavior they are likely used to if they are transitioning to fish from another shell. This also fixes a bug in the `history` function with respect to how it handles the `-n` / `--max` flag. Fixes #4354 * Fix comment for format_history_record() --- doc_src/history.txt | 12 ++-- share/functions/history.fish | 13 ++-- src/builtin_history.cpp | 17 +++-- src/history.cpp | 123 +++++++++++++++++++++++++---------- src/history.h | 10 ++- tests/history.expect | 4 +- 6 files changed, 123 insertions(+), 56 deletions(-) diff --git a/doc_src/history.txt b/doc_src/history.txt index 489dc7178..2f6fbac0a 100644 --- a/doc_src/history.txt +++ b/doc_src/history.txt @@ -2,7 +2,7 @@ \subsection history-synopsis Synopsis \fish{synopsis} -history search [ --show-time ] [ --case-sensitive ] [ --exact | --prefix | --contains ] [ --max=n ] [ --null ] [ "search string"... ] +history search [ --show-time ] [ --case-sensitive ] [ --exact | --prefix | --contains ] [ --max=n ] [ --null ] [ -R | --reverse ] [ "search string"... ] history delete [ --show-time ] [ --case-sensitive ] [ --exact | --prefix | --contains ] "search string"... history merge history save @@ -16,7 +16,7 @@ history ( -h | --help ) The following operations (sub-commands) are available: -- `search` returns history items matching the search string. If no search string is provided it returns all history items. This is the default operation if no other operation is specified. You only have to explicitly say `history search` if you wish to search for one of the subcommands. The `--contains` search option will be used if you don't specify a different search option. Entries are ordered newest to oldest. If stdout is attached to a tty the output will be piped through your pager by the history function. The history builtin simply writes the results to stdout. +- `search` returns history items matching the search string. If no search string is provided it returns all history items. This is the default operation if no other operation is specified. You only have to explicitly say `history search` if you wish to search for one of the subcommands. The `--contains` search option will be used if you don't specify a different search option. Entries are ordered newest to oldest unless you use the `--reverse` flag. If stdout is attached to a tty the output will be piped through your pager by the history function. The history builtin simply writes the results to stdout. - `delete` deletes history items. Without the `--prefix` or `--contains` options, the exact match of the specified text will be deleted. If you don't specify `--exact` a prompt will be displayed before any items are deleted asking you which entries are to be deleted. You can enter the word "all" to delete all matching entries. You can enter a single ID (the number in square brackets) to delete just that single entry. You can enter more than one ID separated by a space to delete multiple entries. Just press [enter] to not delete anything. Note that the interactive delete behavior is a feature of the history function. The history builtin only supports `--exact --case-sensitive` deletion. @@ -32,18 +32,20 @@ These flags can appear before or immediately after one of the sub-commands liste - `-C` or `--case-sensitive` does a case-sensitive search. The default is case-insensitive. Note that prior to fish 2.4.0 the default was case-sensitive. -- `-c` or `--contains` searches or deletes items in the history that contain the specified text string. This is the default for the `--search` flag. This is not currently supported by the `--delete` flag. +- `-c` or `--contains` searches or deletes items in the history that contain the specified text string. This is the default for the `--search` flag. This is not currently supported by the `delete` subcommand. -- `-e` or `--exact` searches or deletes items in the history that exactly match the specified text string. This is the default for the `--delete` flag. Note that the match is case-insensitive by default. If you really want an exact match, including letter case, you must use the `-C` or `--case-sensitive` flag. +- `-e` or `--exact` searches or deletes items in the history that exactly match the specified text string. This is the default for the `delete` subcommand. Note that the match is case-insensitive by default. If you really want an exact match, including letter case, you must use the `-C` or `--case-sensitive` flag. - `-p` or `--prefix` searches or deletes items in the history that begin with the specified text string. This is not currently supported by the `--delete` flag. -- `-t` or `--show-time` prepends each history entry with the date and time the entry was recorded . By default it uses the strftime format `# %c%n`. You can specify another format; e.g., `--show-time='%Y-%m-%d %H:%M:%S '` or `--show-time='%a%I%p'`. The short option, `-t` doesn't accept a stftime format string; it only uses the default format. Any strftime format is allowed, including `%s` to get the raw UNIX seconds since the epoch. Note that `--with-time` is also allowed but is deprecated and will be removed at a future date. +- `-t` or `--show-time` prepends each history entry with the date and time the entry was recorded . By default it uses the strftime format `# %c%n`. You can specify another format; e.g., `--show-time="%Y-%m-%d %H:%M:%S "` or `--show-time="%a%I%p"`. The short option, `-t`, doesn't accept a stftime format string; it only uses the default format. Any strftime format is allowed, including `%s` to get the raw UNIX seconds since the epoch. - `-z` or `--null` causes history entries written by the search operations to be terminated by a NUL character rather than a newline. This allows the output to be processed by `read -z` to correctly handle multiline history entries. - `-` `-n ` or `--max=` limits the matched history items to the first "n" matching entries. This is only valid for `history search`. +- `-R` or `--reverse` causes the history search results to be ordered oldest to newest. Which is the order used by most shells. The default is newest to oldest. + - `-h` or `--help` display help for this command. \subsection history-examples Example diff --git a/share/functions/history.fish b/share/functions/history.fish index 7e90caa99..ec823c3ec 100644 --- a/share/functions/history.fish +++ b/share/functions/history.fish @@ -1,7 +1,6 @@ # # Wrap the builtin history command to provide additional functionality. # - function __fish_unexpected_hist_args --no-scope-shadowing if test -n "$search_mode" or set -q show_time[1] @@ -20,13 +19,13 @@ function history --description "display or manipulate interactive command histor set -l options --exclusive 'c,e,p' --exclusive 'S,D,M,V,C' --exclusive 't,T' set -a options 'h/help' 'c/contains' 'e/exact' 'p/prefix' - set -a options 'C/case-sensitive' 'z/null' 't/show-time=?' 'n#max' + set -a options 'C/case-sensitive' 'R/reverse' 'z/null' 't/show-time=?' 'n#max' # This long option is deprecated and here solely for legacy compatibility. People should use # -t or --show-time now. set -a options 'T-with-time=?' # The following options are deprecated and will be removed in the next major release. # Note that they do not have usable short flags. - set -a options 'S-search' 'D-delete' 'M-merge' 'V-save' 'R-clear' + set -a options 'S-search' 'D-delete' 'M-merge' 'V-save' 'X-clear' argparse -n $cmd $options -- $argv or return @@ -38,7 +37,9 @@ function history --description "display or manipulate interactive command histor set -l hist_cmd set -l show_time - set -l max_count $_flag_max + set -l max_count + set -q _flag_max + set max_count -n$_flag_max set -q _flag_with_time and set -l _flag_show_time $_flag_with_time @@ -90,9 +91,9 @@ function history --description "display or manipulate interactive command histor set -l pager less set -q PAGER and set pager $PAGER - builtin history search $search_mode $show_time $max_count $_flag_case_sensitive $_flag_null -- $argv | eval $pager + builtin history search $search_mode $show_time $max_count $_flag_case_sensitive $_flag_reverse $_flag_null -- $argv | eval $pager else - builtin history search $search_mode $show_time $max_count $_flag_case_sensitive $_flag_null -- $argv + builtin history search $search_mode $show_time $max_count $_flag_case_sensitive $_flag_reverse $_flag_null -- $argv end case delete # interactively delete history diff --git a/src/builtin_history.cpp b/src/builtin_history.cpp index 5bac73e52..c7c86480c 100644 --- a/src/builtin_history.cpp +++ b/src/builtin_history.cpp @@ -2,8 +2,8 @@ #include "config.h" // IWYU pragma: keep #include -#include #include +#include #include #include @@ -31,18 +31,19 @@ struct history_cmd_opts_t { bool print_help = false; hist_cmd_t hist_cmd = HIST_UNDEF; history_search_type_t search_type = (history_search_type_t)-1; - long max_items = LONG_MAX; + size_t max_items = SIZE_MAX; bool history_search_type_defined = false; const wchar_t *show_time_format = NULL; bool case_sensitive = false; bool null_terminate = false; + bool reverse = false; }; /// Note: Do not add new flags that represent subcommands. We're encouraging people to switch to /// the non-flag subcommand form. While many of these flags are deprecated they must be /// supported at least until fish 3.0 and possibly longer to avoid breaking everyones /// config.fish and other scripts. -static const wchar_t *short_options = L":Cmn:epcht::z"; +static const wchar_t *short_options = L":CRcehmn:pt::z"; static const struct woption long_options[] = {{L"prefix", no_argument, NULL, 'p'}, {L"contains", no_argument, NULL, 'c'}, {L"help", no_argument, NULL, 'h'}, @@ -57,6 +58,7 @@ static const struct woption long_options[] = {{L"prefix", no_argument, NULL, 'p' {L"save", no_argument, NULL, 3}, {L"clear", no_argument, NULL, 4}, {L"merge", no_argument, NULL, 5}, + {L"reverse", no_argument, NULL, 'R'}, {NULL, 0, NULL, 0}}; /// Remember the history subcommand and disallow selecting more than one history subcommand. @@ -133,6 +135,10 @@ static int parse_cmd_opts(history_cmd_opts_t &opts, int *optind, //!OCLINT(high opts.case_sensitive = true; break; } + case 'R': { + opts.reverse = true; + break; + } case 'p': { opts.search_type = HISTORY_SEARCH_TYPE_PREFIX; opts.history_search_type_defined = true; @@ -153,12 +159,13 @@ static int parse_cmd_opts(history_cmd_opts_t &opts, int *optind, //!OCLINT(high break; } case 'n': { - opts.max_items = fish_wcstol(w.woptarg); + long x = fish_wcstol(w.woptarg); if (errno) { streams.err.append_format(_(L"%ls: max value '%ls' is not a valid number\n"), cmd, w.woptarg); return STATUS_INVALID_ARGS; } + opts.max_items = static_cast(x); break; } case 'z': { @@ -242,7 +249,7 @@ int builtin_history(parser_t &parser, io_streams_t &streams, wchar_t **argv) { switch (opts.hist_cmd) { case HIST_SEARCH: { if (!history->search(opts.search_type, args, opts.show_time_format, opts.max_items, - opts.case_sensitive, opts.null_terminate, streams)) { + opts.case_sensitive, opts.null_terminate, opts.reverse, streams)) { status = STATUS_CMD_ERROR; } break; diff --git a/src/history.cpp b/src/history.cpp index 33361273c..255de7eb2 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -875,6 +875,15 @@ void history_t::get_history(wcstring_list_t &result) { } } +size_t history_t::size() { + scoped_lock locker(lock); + size_t new_item_count = new_items.size(); + if (this->has_pending_item && new_item_count > 0) new_item_count -= 1; + load_old_if_needed(); + size_t old_item_count = old_item_offsets.size(); + return new_item_count + old_item_count; +} + history_item_t history_t::item_at_index(size_t idx) { scoped_lock locker(lock); @@ -1518,45 +1527,43 @@ void history_t::save(void) { this->save_internal(false); } -// Formats a single history record, including a trailing newline. Returns true -// if bytes were written to the output stream and false otherwise. -static bool format_history_record(const history_item_t &item, const wchar_t *show_time_format, - bool null_terminate, io_streams_t &streams) { +// Formats a single history record, including a trailing newline. +// +// Returns nothing. The only possible failure involves formatting the timestamp. If that happens we +// simply omit the timestamp from the output. +static void format_history_record(const history_item_t &item, const wchar_t *show_time_format, + bool null_terminate, wcstring &result) { if (show_time_format) { const time_t seconds = item.timestamp(); struct tm timestamp; - if (!localtime_r(&seconds, ×tamp)) return false; - const int max_tstamp_length = 100; - wchar_t timestamp_string[max_tstamp_length + 1]; - if (std::wcsftime(timestamp_string, max_tstamp_length, show_time_format, ×tamp) == 0) { - return false; - } - streams.out.append(timestamp_string); - } - streams.out.append(item.str()); - if (null_terminate) { - streams.out.append(L'\0'); - } else { - streams.out.append(L'\n'); - } - return true; -} - -bool history_t::search(history_search_type_t search_type, wcstring_list_t search_args, - const wchar_t *show_time_format, long max_items, bool case_sensitive, - bool null_terminate, io_streams_t &streams) { - // scoped_lock locker(lock); - if (search_args.empty()) { - // Start at one because zero is the current command. - for (int i = 1; !this->item_at_index(i).empty() && max_items; ++i, --max_items) { - if (!format_history_record(this->item_at_index(i), show_time_format, null_terminate, - streams)) { - return false; + if (localtime_r(&seconds, ×tamp)) { + const int max_tstamp_length = 100; + wchar_t timestamp_string[max_tstamp_length + 1]; + if (std::wcsftime(timestamp_string, max_tstamp_length, show_time_format, ×tamp) != + 0) { + result.append(timestamp_string); } } - return true; } + result.append(item.str()); + if (null_terminate) { + result.push_back(L'\0'); + } else { + result.push_back(L'\n'); + } +} + +/// This handles the slightly unusual case of someone searching history for +/// specific terms/patterns. +bool history_t::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) { + wcstring_list_t results; + 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; @@ -1567,11 +1574,55 @@ bool history_t::search(history_search_type_t search_type, wcstring_list_t search history_search_t searcher = history_search_t(*this, search_string, search_type, case_sensitive); while (searcher.go_backwards()) { - if (!format_history_record(searcher.current_item(), show_time_format, null_terminate, - streams)) { - return false; + wcstring result; + auto cur_item = searcher.current_item(); + format_history_record(cur_item, show_time_format, null_terminate, result); + if (reverse) { + results.push_back(result); + } else { + streams.out.append(result); } - if (--max_items == 0) return true; + if (--max_items == 0) break; + } + } + + if (reverse) { + for (auto it = results.rbegin(); it != results.rend(); it++) { + streams.out.append(*it); + } + } + + return true; +} + +bool history_t::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) { + if (!search_args.empty()) { + // User wants the results filtered. This is not the common case so we do it separate + // from the code below for unfiltered output which is much cheaper. + return search_with_args(search_type, search_args, show_time_format, max_items, + case_sensitive, null_terminate, reverse, streams); + } + + // scoped_lock locker(lock); + size_t hist_size = this->size(); + if (max_items > hist_size) max_items = hist_size; + + if (reverse) { + for (size_t i = max_items; i != 0; --i) { + auto cur_item = this->item_at_index(i); + wcstring result; + format_history_record(cur_item, show_time_format, null_terminate, result); + streams.out.append(result); + } + } else { + // Start at one because zero is the current command. + for (size_t i = 1; i < max_items + 1; ++i) { + auto cur_item = this->item_at_index(i); + wcstring result; + format_history_record(cur_item, show_time_format, null_terminate, result); + streams.out.append(result); } } diff --git a/src/history.h b/src/history.h index 4bdf752b1..6b7148fc4 100644 --- a/src/history.h +++ b/src/history.h @@ -238,8 +238,11 @@ class history_t { // Searches history. bool search(history_search_type_t search_type, wcstring_list_t search_args, - const wchar_t *show_time_format, long max_items, bool case_sensitive, - bool null_terminate, io_streams_t &streams); + 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); // Enable / disable automatic saving. Main thread only! void disable_automatic_saving(); @@ -267,6 +270,9 @@ class history_t { // Return the specified history at the specified index. 0 is the index of the current // commandline. (So the most recent item is at index 1.) history_item_t item_at_index(size_t idx); + + // Return the number of history entries. + size_t size(); }; class history_search_t { diff --git a/tests/history.expect b/tests/history.expect index 4c7c9630c..260d5bd09 100644 --- a/tests/history.expect +++ b/tests/history.expect @@ -50,8 +50,8 @@ expect_prompt -re {start2\r\necho start1; builtin history; echo end1\r\nend2\r\n # ========== # Verify explicit searching for the first two commands in the previous tests # returns the expected results. -send "history search echo start\r" -expect_prompt -re {\r\necho start1.*\r\necho start2} { +send "history search --reverse 'echo start'\r" +expect_prompt -re {\r\necho start1;.*\r\necho start2;} { puts "history function explicit search succeeded" } timeout { puts stderr "history function explicit search failed"