diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index c1a350a63..a925c9ba5 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -3128,8 +3128,10 @@ void history_tests_t::test_history_formats(void) { if (!f) { err(L"Couldn't open file tests/history_sample_bash"); } else { - // It should skip over the export command since that's a bash-ism. - const wchar_t *expected[] = {L"echo supsup", L"history --help", L"echo foo", NULL}; + // The results are in the reverse order that they appear in the bash history file. + // We don't expect whitespace to be elided. + const wchar_t *expected[] = {L"sleep 123", L" final line", L"echo supsup", + L"history --help", L"echo foo", NULL}; history_t &test_history = history_t::history_with_name(L"bash_import"); test_history.populate_from_bash(f); if (!history_equals(test_history, expected)) { diff --git a/src/history.cpp b/src/history.cpp index 780bc2841..28e89f2b6 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -1670,11 +1670,12 @@ void history_t::populate_from_config_path() { } } -/// Indicate whether we ought to import the bash history file into fish. +/// Decide whether we ought to import a bash history line into fish. This is a very crude heuristic. static bool should_import_bash_history_line(const std::string &line) { if (line.empty()) return false; - // Very naive tests! Skip export; probably should skip others. + // Very naive tests! Skip `export` and comments. + // TODO: We should probably should skip other commands. const char *const ignore_prefixes[] = {"export ", "#"}; for (size_t i = 0; i < sizeof ignore_prefixes / sizeof *ignore_prefixes; i++) { @@ -1687,39 +1688,40 @@ static bool should_import_bash_history_line(const std::string &line) { // Skip lines with backticks. if (line.find('`') != std::string::npos) return false; + // Skip lines that end with a backslash since we do not handle multiline commands from bash. + if (line.back() == '\\') return false; + return true; } +/// Import a bash command history file. Bash's history format is very simple: just lines with #s for +/// comments. Ignore a few commands that are bash-specific. It makes no attempt to handle multiline +/// commands. We can't actually parse bash syntax and the bash history file does not unambiguously +/// encode multiline commands. void history_t::populate_from_bash(FILE *stream) { - // Bash's format is very simple: just lines with #s for comments. Ignore a few commands that are - // bash-specific. This list ought to be expanded. - std::string line; - for (;;) { - line.clear(); - bool success = false; - bool has_newline = false; + bool eof = false; - // Loop until we've read a line. - do { + // Process the entire history file until EOF is observed. + while (!eof) { + auto line = std::string(); + + // Loop until we've read a line or EOF is observed. + while (true) { char buff[128]; - success = (bool)fgets(buff, sizeof buff, stream); - if (success) { - // Skip the newline. - char *a_newline = strchr(buff, '\n'); - if (a_newline) *a_newline = '\0'; - has_newline = (a_newline != NULL); - - // Append what we've got. - line.append(buff); + if (!fgets(buff, sizeof buff, stream)) { + eof = true; + break; } - } while (success && !has_newline); - // Maybe add this line. - if (should_import_bash_history_line(line)) { - this->add(str2wcstring(line)); + // Deal with the newline if present. + char *a_newline = strchr(buff, '\n'); + if (a_newline) *a_newline = '\0'; + line.append(buff); + if (a_newline) break; } - if (line.empty()) break; + // Add this line if it doesn't contain anything we know we can't handle. + if (should_import_bash_history_line(line)) this->add(str2wcstring(line)); } } diff --git a/tests/history_sample_bash b/tests/history_sample_bash index 0be128a00..bd47a18b9 100644 --- a/tests/history_sample_bash +++ b/tests/history_sample_bash @@ -5,3 +5,10 @@ export HISTTIMEFORMAT='%F %T ' #1339718298 echo supsup #abcde +echo hello \ + second line \ + the `echo third` line \ + final line +another `command + and arg` to skip +sleep 123