diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b6afb140..6f3102a4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Improved completions for `killall` (#4052), `ln` (#4090) and `zypper` (#4079). - Implemented `string lower` and `string upper` (#4080). - `help` can now open the tutorial. +- `echo -h` now correctly echoes `-h` (#4120). --- diff --git a/src/builtin.cpp b/src/builtin.cpp index 7fd8a69f9..90e9f887f 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -474,11 +474,10 @@ void builtin_destroy() {} /// Is there a builtin command with the given name? bool builtin_exists(const wcstring &cmd) { return static_cast(builtin_lookup(cmd)); } -/// If builtin takes care of printing help itself +/// Is the command a keyword or a builtin we need to special-case the handling of `-h` and `--help`. static const wcstring_list_t help_builtins({L"for", L"while", L"function", L"if", L"end", L"switch", L"case", L"count", L"printf"}); -static bool builtin_handles_help(const wchar_t *cmd) { - CHECK(cmd, 0); +static bool cmd_needs_help(const wchar_t *cmd) { return contains(help_builtins, cmd); } @@ -488,14 +487,16 @@ int builtin_run(parser_t &parser, const wchar_t *const *argv, io_streams_t &stre UNUSED(streams); if (argv == NULL || argv[0] == NULL) return STATUS_INVALID_ARGS; - const builtin_data_t *data = builtin_lookup(argv[0]); - if (argv[1] != NULL && !builtin_handles_help(argv[0]) && argv[2] == NULL && - parse_util_argument_is_help(argv[1], 0)) { + // We can be handed a keyword by the parser as if it was a command. This happens when the user + // follows the keyword by `-h` or `--help`. Since it isn't really a builtin command we need to + // handle displaying help for it here. + if (argv[1] && !argv[2] && parse_util_argument_is_help(argv[1]) && cmd_needs_help(argv[0])) { builtin_print_help(parser, streams, argv[0], streams.out); return STATUS_CMD_OK; } - if (data != NULL) { + const builtin_data_t *data = builtin_lookup(argv[0]); + if (data) { // Warning: layering violation and naughty cast. The code originally had a much more // complicated solution to achieve exactly the same result: lie about the constness of argv. // Some of the builtins we call do mutate the array via their calls to wgetopt() which could diff --git a/src/builtin_echo.cpp b/src/builtin_echo.cpp index 3c59987c9..7b1d377a5 100644 --- a/src/builtin_echo.cpp +++ b/src/builtin_echo.cpp @@ -13,7 +13,6 @@ #include "wutil.h" // IWYU pragma: keep struct echo_cmd_opts_t { - bool print_help = false; bool print_newline = true; bool print_spaces = true; bool interpret_special_chars = false; diff --git a/src/event.cpp b/src/event.cpp index 40b281fe5..eea1e2d4b 100644 --- a/src/event.cpp +++ b/src/event.cpp @@ -247,7 +247,7 @@ static wcstring event_desc_compact(const event_t &event) { void event_add_handler(const event_t &event) { if (debug_level >= 3) { wcstring desc = event_desc_compact(event); - debug(3, "register: %ls\n", desc.c_str()); + debug(3, "register: %ls", desc.c_str()); } shared_ptr e = std::make_shared(event); @@ -262,7 +262,7 @@ void event_add_handler(const event_t &event) { void event_remove(const event_t &criterion) { if (debug_level >= 3) { wcstring desc = event_desc_compact(criterion); - debug(3, "unregister: %ls\n", desc.c_str()); + debug(3, "unregister: %ls", desc.c_str()); } event_list_t::iterator iter = s_event_handlers.begin(); diff --git a/src/exec.cpp b/src/exec.cpp index 6e02e6eb9..35bfb91a6 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1064,7 +1064,7 @@ void exec_job(parser_t &parser, job_t *j) { // safe_launch_process _never_ returns... DIE("safe_launch_process should not have returned"); } else { - debug(2, L"Fork #%d, pid %d: external command '%s' from '%ls'\n", + debug(2, L"Fork #%d, pid %d: external command '%s' from '%ls'", g_fork_count, pid, p->argv0(), file ? file : L""); if (pid < 0) { job_mark_process_as_failed(j, p); diff --git a/src/iothread.cpp b/src/iothread.cpp index 756170f30..c57464ff1 100644 --- a/src/iothread.cpp +++ b/src/iothread.cpp @@ -121,7 +121,7 @@ static void *iothread_worker(void *unused) { UNUSED(unused); struct spawn_request_t req; while (dequeue_spawn_request(&req)) { - debug(5, "pthread %p dequeued\n", this_thread()); + debug(5, "pthread %p dequeued", this_thread()); // Perform the work req.handler(); @@ -146,7 +146,7 @@ static void *iothread_worker(void *unused) { int new_thread_count = --s_spawn_requests.acquire().value.thread_count; assert(new_thread_count >= 0); - debug(5, "pthread %p exiting\n", this_thread()); + debug(5, "pthread %p exiting", this_thread()); // We're done. return NULL; } @@ -168,7 +168,7 @@ static void iothread_spawn() { // We will never join this thread. DIE_ON_FAILURE(pthread_detach(thread)); - debug(5, "pthread %p spawned\n", (void *)(intptr_t)thread); + debug(5, "pthread %p spawned", (void *)(intptr_t)thread); // Restore our sigmask. DIE_ON_FAILURE(pthread_sigmask(SIG_SETMASK, &saved_set, NULL)); } diff --git a/src/output.cpp b/src/output.cpp index 8814b5fc1..41422b52b 100644 --- a/src/output.cpp +++ b/src/output.cpp @@ -168,7 +168,7 @@ void set_color(rgb_color_t c, rgb_color_t c2) { #if 0 wcstring tmp = c.description(); wcstring tmp2 = c2.description(); - debug(3, "set_color %ls : %ls\n", tmp.c_str(), tmp2.c_str()); + debug(3, "set_color %ls : %ls", tmp.c_str(), tmp2.c_str()); #endif ASSERT_IS_MAIN_THREAD(); if (!cur_term) return; diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index b6ac88c4c..f37ded990 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -877,9 +877,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process( process_type = function_exists(L"cd") ? INTERNAL_FUNCTION : INTERNAL_BUILTIN; } else { const globspec_t glob_behavior = (cmd == L"set" || cmd == L"count") ? nullglob : failglob; - // Form the list of arguments. The command is the first argument. TODO: count hack, where we - // treat 'count --help' as different from 'count $foo' that expands to 'count --help'. fish - // 1.x never successfully did this, but it tried to! + // Form the list of arguments. The command is the first argument. parse_execution_result_t arg_result = this->determine_arguments(statement, &argument_list, glob_behavior); if (arg_result != parse_execution_success) { diff --git a/src/parse_productions.cpp b/src/parse_productions.cpp index ffd998e11..7a66f9d7d 100644 --- a/src/parse_productions.cpp +++ b/src/parse_productions.cpp @@ -436,7 +436,7 @@ const production_element_t *parse_productions::production_for_token(parse_token_ const parse_token_t &input1, const parse_token_t &input2, parse_node_tag_t *out_tag) { - debug(5, "Resolving production for %ls with input token <%ls>\n", + debug(5, "Resolving production for %ls with input token <%ls>", token_type_description(node_type), input1.describe().c_str()); // Fetch the function to resolve the list of productions. @@ -504,7 +504,7 @@ const production_element_t *parse_productions::production_for_token(parse_token_ const production_element_t *result = resolver(input1, input2, out_tag); if (result == NULL) { - debug(5, "Node type '%ls' has no production for input '%ls' (in %s)\n", + debug(5, "Node type '%ls' has no production for input '%ls' (in %s)", token_type_description(node_type), input1.describe().c_str(), __FUNCTION__); } diff --git a/src/parse_util.cpp b/src/parse_util.cpp index fc55e7c28..d522637a6 100644 --- a/src/parse_util.cpp +++ b/src/parse_util.cpp @@ -754,13 +754,8 @@ static int parser_is_pipe_forbidden(const wcstring &word) { return contains(forbidden_pipe_commands, word); } -bool parse_util_argument_is_help(const wchar_t *s, int min_match) { - CHECK(s, 0); - size_t len = wcslen(s); - - min_match = maxi(min_match, 3); //!OCLINT(parameter reassignment) - - return wcscmp(L"-h", s) == 0 || (len >= (size_t)min_match && (wcsncmp(L"--help", s, len) == 0)); +bool parse_util_argument_is_help(const wchar_t *s) { + return wcscmp(L"-h", s) == 0 || wcscmp(L"--help", s) == 0; } /// Check if the first argument under the given node is --help. @@ -773,7 +768,7 @@ static bool first_argument_is_help(const parse_node_tree_t &node_tree, const par // Check the first argument only. const parse_node_t &arg = *arg_nodes.at(0); const wcstring first_arg_src = arg.get_source(src); - is_help = parse_util_argument_is_help(first_arg_src.c_str(), 3); + is_help = parse_util_argument_is_help(first_arg_src.c_str()); } return is_help; } diff --git a/src/parse_util.h b/src/parse_util.h index 19a85a8ee..173f47913 100644 --- a/src/parse_util.h +++ b/src/parse_util.h @@ -99,12 +99,7 @@ size_t parse_util_get_offset(const wcstring &str, int line, long line_offset); wcstring parse_util_unescape_wildcards(const wcstring &in); /// Checks if the specified string is a help option. -/// -/// \param s the string to test -/// \param min_match is the minimum number of characters that must match in a long style option, -/// i.e. the longest common prefix between --help and any other option. If less than 3, 3 will be -/// assumed. -bool parse_util_argument_is_help(const wchar_t *s, int min_match); +bool parse_util_argument_is_help(const wchar_t *s); /// Calculates information on the parameter at the specified index. /// diff --git a/src/proc.cpp b/src/proc.cpp index 7a00e2b9a..0864f1383 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -718,7 +718,7 @@ static int select_try(job_t *j) { // fwprintf( stderr, L"fd %d on job %ls\n", fd, j->command ); FD_SET(fd, &fds); maxfd = maxi(maxfd, fd); - debug(3, L"select_try on %d\n", fd); + debug(3, L"select_try on %d", fd); } } @@ -731,7 +731,7 @@ static int select_try(job_t *j) { retval = select(maxfd + 1, &fds, 0, 0, &tv); if (retval == 0) { - debug(3, L"select_try hit timeout\n"); + debug(3, L"select_try hit timeout"); } return retval > 0; } @@ -755,7 +755,7 @@ static void read_try(job_t *j) { } if (buff) { - debug(3, L"proc::read_try('%ls')\n", j->command_wcstr()); + debug(3, L"proc::read_try('%ls')", j->command_wcstr()); while (1) { char b[BUFFER_SIZE]; long l;