fix history --delete regression

The recent change to reconcile the history builtin command and function
broke an undocumented behavior of `history --delete`. This change
reinstates that behavior. It also adds an explicit `--exact` search mode
for the `--search` and `--delete` subcommands.

Fixes #3270
This commit is contained in:
Kurtis Rader 2016-07-29 21:24:26 -07:00
parent ef5d3232e4
commit 710addde16
8 changed files with 156 additions and 28 deletions

View file

@ -2,8 +2,8 @@
\subsection history-synopsis Synopsis
\fish{synopsis}
history ( -s | --search ) [ -t | --with-time ] [ -p | --prefix | -c | --contains ] [ "search string"... ]
history ( -d | --delete ) [ -t | --with-time ] [ -p | --prefix | -c | --contains ] "search string"...
history ( -s | --search ) [ -t | --with-time ] [ -e | --exact | -p | --prefix | -c | --contains ] [ "search string"... ]
history ( -d | --delete ) [ -t | --with-time ] [ -e | --exact | -p | --prefix | -c | --contains ] "search string"...
history ( -m | --merge )
history ( -s | --save )
history ( -l | --clear )
@ -24,13 +24,15 @@ The following commands are available:
- `-v` or `--save` saves all changes in the history file. The shell automatically saves the history file; this option is provided for internal use.
- `-l` or `--clear` clears the history file. A prompt is displayed before the history is erased asking you to confirm you really want to clear all history.
- `-l` or `--clear` clears the history file. A prompt is displayed before the history is erased asking you to confirm you really want to clear all history unless `builtin history` is used.
The following options are available:
- `-p` or `--prefix` searches or deletes items in the history that begin with the specified text string.
- `-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.
- `-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.
- `-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 `--with-time` prefixes the output of each displayed history entry with the time it was recorded in the format "%Y-%m-%d %H:%M:%S" in your local timezone.

View file

@ -3,7 +3,7 @@
#
function history --shadow-builtin --description "display or manipulate interactive command history"
set -l cmd
set -l search_mode --contains
set -l search_mode
set -l with_time
# The "set cmd $cmd xyz" lines are to make it easy to detect if the user specifies more than one
@ -28,6 +28,8 @@ function history --shadow-builtin --description "display or manipulate interacti
set search_mode --prefix
case -c --contains
set search_mode --contains
case -e --exact
set search_mode --exact
case --
set -e argv[1]
break
@ -46,6 +48,9 @@ function history --shadow-builtin --description "display or manipulate interacti
switch $cmd
case search
test -z "$search_mode"
and set search_mode "--contains"
if isatty stdout
set -l pager less
set -q PAGER
@ -58,10 +63,18 @@ function history --shadow-builtin --description "display or manipulate interacti
case delete # Interactively delete history
# TODO: Fix this to deal with history entries that have multiple lines.
if not set -q argv[1]
printf "You have to specify at least one search term to find entries to delete" >&2
printf (_ "You must specify at least one search term when deleting entries") >&2
return 1
end
test -z "$search_mode"
and set search_mode "--exact"
if test $search_mode = "--exact"
builtin history --delete $search_mode $argv
return
end
# TODO: Fix this so that requesting history entries with a timestamp works:
# set -l found_items (builtin history --search $search_mode $with_time -- $argv)
set -l found_items (builtin history --search $search_mode -- $argv)

View file

@ -2866,15 +2866,21 @@ static int builtin_history(parser_t &parser, io_streams_t &streams, wchar_t **ar
;
int argc = builtin_count_args(argv);
hist_cmd_t hist_cmd = HIST_NOOP;
history_search_type_t search_type = HISTORY_SEARCH_TYPE_CONTAINS;
history_search_type_t search_type = (history_search_type_t)-1;
bool history_search_type_defined = false;
bool with_time = false;
static const struct woption long_options[] = {
{L"delete", no_argument, 0, 'd'}, {L"search", no_argument, 0, 's'},
{L"prefix", no_argument, 0, 'p'}, {L"contains", no_argument, 0, 'c'},
{L"save", no_argument, 0, 'v'}, {L"clear", no_argument, 0, 'l'},
{L"merge", no_argument, 0, 'm'}, {L"help", no_argument, 0, 'h'},
{L"with-time", no_argument, 0, 't'}, {0, 0, 0, 0}};
static const struct woption long_options[] = {{L"delete", no_argument, 0, 'd'},
{L"search", no_argument, 0, 's'},
{L"prefix", no_argument, 0, 'p'},
{L"contains", no_argument, 0, 'c'},
{L"save", no_argument, 0, 'v'},
{L"clear", no_argument, 0, 'l'},
{L"merge", no_argument, 0, 'm'},
{L"help", no_argument, 0, 'h'},
{L"with-time", no_argument, 0, 't'},
{L"exact", no_argument, 0, 'e'},
{0, 0, 0, 0}};
history_t *history = reader_get_history();
// Use the default history if we have none (which happens if invoked non-interactively, e.g.
@ -2884,7 +2890,7 @@ static int builtin_history(parser_t &parser, io_streams_t &streams, wchar_t **ar
int opt = 0;
int opt_index = 0;
wgetopter_t w;
while ((opt = w.wgetopt_long(argc, argv, L"+dspcvlmht", long_options, &opt_index)) != EOF) {
while ((opt = w.wgetopt_long(argc, argv, L"+despcvlmht", long_options, &opt_index)) != EOF) {
switch (opt) {
case 's': {
if (!set_hist_cmd(cmd, &hist_cmd, HIST_SEARCH, streams)) {
@ -2918,10 +2924,17 @@ static int builtin_history(parser_t &parser, io_streams_t &streams, wchar_t **ar
}
case 'p': {
search_type = HISTORY_SEARCH_TYPE_PREFIX;
history_search_type_defined = true;
break;
}
case 'c': {
search_type = HISTORY_SEARCH_TYPE_CONTAINS;
history_search_type_defined = true;
break;
}
case 'e': {
search_type = HISTORY_SEARCH_TYPE_EXACT;
history_search_type_defined = true;
break;
}
case 't': {
@ -2946,7 +2959,12 @@ static int builtin_history(parser_t &parser, io_streams_t &streams, wchar_t **ar
// Everything after the flags is an argument for a subcommand (e.g., a search term).
const wcstring_list_t args(argv + w.woptind, argv + argc);
// Establish appropriate defaults for unspecified options.
if (hist_cmd == HIST_NOOP) hist_cmd = HIST_SEARCH;
if (!history_search_type_defined) {
if (hist_cmd == HIST_SEARCH) search_type = HISTORY_SEARCH_TYPE_CONTAINS;
if (hist_cmd == HIST_DELETE) search_type = HISTORY_SEARCH_TYPE_EXACT;
}
int status = STATUS_BUILTIN_OK;
switch (hist_cmd) {
@ -2957,11 +2975,19 @@ static int builtin_history(parser_t &parser, io_streams_t &streams, wchar_t **ar
break;
}
case HIST_DELETE: {
// TODO: Move this code to the history module and support the other search types. At
// this time we expect the non-exact deletions to be handled only by the history
// function's interactive delete feature.
if (search_type != HISTORY_SEARCH_TYPE_EXACT) {
streams.err.append_format(_(L"builtin history --delete only supports --exact\n"));
status = STATUS_BUILTIN_ERROR;
break;
}
for (wcstring_list_t::const_iterator iter = args.begin(); iter != args.end(); ++iter) {
wcstring delete_string = *iter;
if (delete_string[0] == '"' && delete_string[delete_string.length() - 1] == '"')
if (delete_string[0] == '"' && delete_string[delete_string.length() - 1] == '"') {
delete_string = delete_string.substr(1, delete_string.length() - 2);
}
history->remove(delete_string);
}
break;

View file

@ -30,7 +30,6 @@
#include "parse_tree.h"
#include "path.h"
#include "reader.h"
#include "sanity.h"
#include "signal.h"
#include "wutil.h" // IWYU pragma: keep
@ -438,19 +437,17 @@ 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) const {
switch (type) {
case HISTORY_SEARCH_TYPE_CONTAINS: {
// We consider equal strings to NOT match a contains search (so that you don't have to
// see history equal to what you typed). The length check ensures that.
return contents.size() > term.size() && contents.find(term) != wcstring::npos;
return contents.find(term) != wcstring::npos;
}
case HISTORY_SEARCH_TYPE_PREFIX: {
// We consider equal strings to match a prefix search, so that autosuggest will allow
// suggesting what you've typed.
return string_prefixes_string(term, contents);
}
default: {
sanity_lose();
abort();
case HISTORY_SEARCH_TYPE_EXACT: {
return term == contents;
}
default: { DIE("unexpected history_search_type_t value"); }
}
}

View file

@ -40,9 +40,11 @@ struct io_streams_t;
typedef std::vector<wcstring> path_list_t;
enum history_search_type_t {
// The history searches for strings containing the given string.
// Search for commands exactly matching the given string.
HISTORY_SEARCH_TYPE_EXACT = 1,
// Search for commands containing the given string.
HISTORY_SEARCH_TYPE_CONTAINS,
// The history searches for strings starting with the given string.
// Search for commands starting with the given string.
HISTORY_SEARCH_TYPE_PREFIX
};

View file

@ -2873,11 +2873,15 @@ const wchar_t *reader_readline(int nchars) {
data->history_search = history_search_t(*data->history, data->search_buff,
HISTORY_SEARCH_TYPE_CONTAINS);
// Skip the autosuggestion as history unless it was truncated.
// Always skip history entries that exactly match what has been typed so far.
wcstring_list_t skip_list;
skip_list.push_back(data->command_line.text);
const wcstring &suggest = data->autosuggestion;
if (!suggest.empty() && !data->screen.autosuggestion_is_truncated) {
data->history_search.skip_matches(wcstring_list_t(&suggest, 1 + &suggest));
// Also skip the autosuggestion in the history unless it was truncated.
skip_list.push_back(suggest);
}
data->history_search.skip_matches(skip_list);
}
switch (data->search_mode) {

View file

@ -83,3 +83,80 @@ expect_prompt -re {\r\n\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d echo start1; builtin h
} unmatched {
puts stderr "history function implicit search with timestamps failed"
}
# ==========
# Verify explicit searching for an exact command returns just that command.
# returns the expected results.
send "echo hello\r"
expect_prompt
send "echo goodbye\r"
expect_prompt
send "echo hello again\r"
expect_prompt
send "echo hello AGAIN\r"
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 {
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 {
puts stderr "history function explicit exact search 'echo hello' failed"
}
# This is slightly subtle in that it shouldn't actually match anything between
# the command we sent and the next prompt.
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 {
puts stderr "history function explicit exact search 'echo hell' failed"
}
# ==========
# Delete a single command we recently ran.
send "history --delete 'echo hello'\r"
expect_prompt -re {history --delete 'echo hello'\r\n} {
puts "history function explicit exact delete 'echo hello' succeeded"
} unmatched {
puts stderr "history function explicit exact delete 'echo hello' failed"
}
# ==========
# Interactively delete one of multiple matched commands. This verifies that we
# delete the first entry matched by the prefix search (the most recent command
# sent above that matches).
send "history --delete -p 'echo hello'\r"
expect -re {history --delete -p 'echo hello'\r\n}
expect -re {\[1\] echo hello AGAIN\r\n}
expect -re {\[2\] echo hello again\r\n\r\n}
expect -re {Enter nothing to cancel.*\r\nEnter "all" to delete all the matching entries\.\r\n}
expect -re {Delete which entries\? >}
send "1\r"
expect_prompt -re {Deleting history entry 1: "echo hello AGAIN"\r\n} {
puts "history function explicit prefix delete 'echo hello' succeeded"
} unmatched {
puts stderr "history function explicit prefix delete 'echo hello' failed"
}
# Verify that the deleted history entry is gone and the other one that matched
# the prefix search above is still there.
send "history --search --exact 'echo hello again'\r"
expect_prompt -re {\r\necho hello again\r\n} {
puts "history function explicit exact search 'echo hello again' succeeded"
} unmatched {
puts stderr "history function explicit exact search 'echo hello again' failed"
}
send "history --search --exact 'echo hello AGAIN'\r"
expect_prompt -re {\r\necho hello AGAIN\r\n} {
puts stderr "history function explicit exact search 'echo hello AGAIN' found the entry"
} unmatched {
puts "history function explicit exact search 'echo hello AGAIN' failed to find the entry"
}

View file

@ -4,3 +4,10 @@ invalid attempt at multiple history commands detected
history function explicit search succeeded
history function implicit search succeeded
history function implicit search with timestamps succeeded
history function explicit exact search 'echo goodbye' succeeded
history function explicit exact search 'echo hello' succeeded
history function explicit exact search 'echo hell' succeeded
history function explicit exact delete 'echo hello' succeeded
history function explicit prefix delete 'echo hello' succeeded
history function explicit exact search 'echo hello again' succeeded
history function explicit exact search 'echo hello AGAIN' failed to find the entry