From 88d2d54276c8c7cb9423b6e9f8868789e8ef85b3 Mon Sep 17 00:00:00 2001
From: Fabian Homborg <FHomborg@gmail.com>
Date: Tue, 26 Mar 2019 19:13:01 +0100
Subject: [PATCH] Stop printing help summary on error

This now displays

- the error message

- a (significantly shorter) backtrace

- A call to open `help $cmd` if necessary

See #5434.
Fixes #3404.
---
 src/builtin.cpp             | 85 +++++--------------------------------
 src/builtin.h               |  2 +
 src/builtin_argparse.cpp    |  8 +++-
 src/builtin_commandline.cpp | 16 +++----
 src/builtin_complete.cpp    |  2 +-
 src/builtin_disown.cpp      |  2 +-
 src/builtin_exit.cpp        |  4 +-
 src/builtin_fg.cpp          |  4 +-
 src/builtin_function.cpp    |  2 +-
 src/builtin_functions.cpp   | 14 +++---
 src/builtin_read.cpp        | 12 +++---
 src/builtin_return.cpp      |  6 +--
 src/builtin_set.cpp         | 34 +++++++--------
 src/builtin_ulimit.cpp      |  6 +--
 tests/string.err            |  6 ---
 15 files changed, 71 insertions(+), 132 deletions(-)

diff --git a/src/builtin.cpp b/src/builtin.cpp
index 6efffd6b3..f30162799 100644
--- a/src/builtin.cpp
+++ b/src/builtin.cpp
@@ -187,92 +187,29 @@ wcstring builtin_help_get(parser_t &parser, io_streams_t &streams, const wchar_t
 ///
 void builtin_print_help(parser_t &parser, io_streams_t &streams, const wchar_t *cmd,
                         output_stream_t &b) {
-    bool is_stderr = &b == &streams.err;
-    if (is_stderr) {
-        b.append(parser.current_line());
-    }
-
-    const wcstring h = builtin_help_get(parser, streams, cmd);
-
-    if (!h.size()) return;
-
-    wchar_t *str = wcsdup(h.c_str());
-    if (str) {
-        bool is_short = false;
-        if (is_stderr) {
-            // Interactive mode help to screen - only print synopsis if the rest won't fit.
-            int screen_height, my_lines;
-
-            screen_height = common_get_height();
-            my_lines = count_char(str, L'\n');
-            if (!shell_is_interactive() || (my_lines > 2 * screen_height / 3)) {
-                wchar_t *pos;
-                int cut = 0;
-                int i;
-
-                is_short = true;
-
-                // First move down 4 lines.
-                pos = str;
-                for (i = 0; (i < 4) && pos && *pos; i++) {
-                    pos = std::wcschr(pos + 1, L'\n');
-                }
-
-                if (pos && *pos) {
-                    // Then find the next empty line.
-                    for (; *pos; pos++) {
-                        if (*pos != L'\n') {
-                            continue;
-                        }
-
-                        int is_empty = 1;
-                        wchar_t *pos2;
-                        for (pos2 = pos + 1; *pos2; pos2++) {
-                            if (*pos2 == L'\n') break;
-
-                            if (*pos2 != L'\t' && *pos2 != L' ') {
-                                is_empty = 0;
-                                break;
-                            }
-                        }
-                        if (is_empty) {
-                            // And cut it.
-                            *(pos2 + 1) = L'\0';
-                            cut = 1;
-                            break;
-                        }
-                    }
-                }
-
-                // We did not find a good place to cut message to shorten it - so we make sure we
-                // don't print anything.
-                if (!cut) {
-                    *str = 0;
-                }
-            }
-        }
-
-        b.append(str);
-        if (is_short) {
-            b.append_format(_(L"%ls: Type 'help %ls' for related documentation\n\n"), cmd, cmd);
-        }
-
-        free(str);
-    }
+    b.append(builtin_help_get(parser, streams, cmd));
 }
 
 /// Perform error reporting for encounter with unknown option.
 void builtin_unknown_option(parser_t &parser, io_streams_t &streams, const wchar_t *cmd,
                             const wchar_t *opt) {
     streams.err.append_format(BUILTIN_ERR_UNKNOWN, cmd, opt);
-    builtin_print_help(parser, streams, cmd, streams.err);
+    builtin_print_error_trailer(parser, streams.err, cmd);
 }
 
 /// Perform error reporting for encounter with missing argument.
 void builtin_missing_argument(parser_t &parser, io_streams_t &streams, const wchar_t *cmd,
                               const wchar_t *opt) {
     streams.err.append_format(BUILTIN_ERR_MISSING, cmd, opt);
-    builtin_print_help(parser, streams, cmd, streams.err);
+    builtin_print_error_trailer(parser, streams.err, cmd);
+}
+
+/// Print the backtrace and call for help that we use at the end of error messages.
+void builtin_print_error_trailer(parser_t &parser, output_stream_t &b, const wchar_t *cmd) {
+    b.append(L"\n");
+    b.append(parser.current_line());
+    b.append(L"\n");
+    b.append_format(_(L"(Type 'help %ls' for related documentation)\n"), cmd);
 }
 
 /// A generic bultin that only supports showing a help message. This is only a placeholder that
diff --git a/src/builtin.h b/src/builtin.h
index 8e5fd5e18..6185267bf 100644
--- a/src/builtin.h
+++ b/src/builtin.h
@@ -115,6 +115,8 @@ void builtin_unknown_option(parser_t &parser, io_streams_t &streams, const wchar
 void builtin_missing_argument(parser_t &parser, io_streams_t &streams, const wchar_t *cmd,
                               const wchar_t *opt);
 
+void builtin_print_error_trailer(parser_t &parser, output_stream_t &b, const wchar_t *cmd);
+
 void builtin_wperror(const wchar_t *s, io_streams_t &streams);
 
 struct help_only_cmd_opts_t {
diff --git a/src/builtin_argparse.cpp b/src/builtin_argparse.cpp
index 9c6d930a3..4c70fd657 100644
--- a/src/builtin_argparse.cpp
+++ b/src/builtin_argparse.cpp
@@ -548,7 +548,13 @@ static int argparse_parse_flags(parser_t &parser, const argparse_cmd_opts_t &opt
                 retval = validate_and_store_implicit_int(parser, opts, arg_contents, w, long_idx,
                                                          streams);
             } else {
-                builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1]);
+                streams.err.append_format(BUILTIN_ERR_UNKNOWN, cmd, argv[w.woptind - 1]);
+                // We don't use builtin_print_error_trailer as that
+                // says to use the cmd help,
+                // which doesn't work if it's a command that does not belong to fish.
+                //
+                // Plus this particular error is not an error in argparse usage.
+                streams.err.append(parser.current_line());
                 retval = STATUS_INVALID_ARGS;
             }
             if (retval != STATUS_CMD_OK) return retval;
diff --git a/src/builtin_commandline.cpp b/src/builtin_commandline.cpp
index a4fb80f46..d37e87384 100644
--- a/src/builtin_commandline.cpp
+++ b/src/builtin_commandline.cpp
@@ -196,7 +196,7 @@ int builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_t **argv)
 
         streams.err.append(argv[0]);
         streams.err.append(L": Can not set commandline in non-interactive mode\n");
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_CMD_ERROR;
     }
 
@@ -315,7 +315,7 @@ int builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_t **argv)
         if (buffer_part || cut_at_cursor || append_mode || tokenize || cursor_mode || line_mode ||
             search_mode || paging_mode) {
             streams.err.append_format(BUILTIN_ERR_COMBO, argv[0]);
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_INVALID_ARGS;
         }
 
@@ -331,7 +331,7 @@ int builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_t **argv)
                 input_queue_ch(*mc);
             } else {
                 streams.err.append_format(_(L"%ls: Unknown input function '%ls'"), cmd, argv[i]);
-                builtin_print_help(parser, streams, cmd, streams.err);
+                builtin_print_error_trailer(parser, streams.err, cmd);
                 return STATUS_INVALID_ARGS;
             }
         }
@@ -351,14 +351,14 @@ int builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_t **argv)
     // Check for invalid switch combinations.
     if ((search_mode || line_mode || cursor_mode || paging_mode) && (argc - w.woptind > 1)) {
         streams.err.append_format(L"%ls: Too many arguments", argv[0]);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
     if ((buffer_part || tokenize || cut_at_cursor) &&
         (cursor_mode || line_mode || search_mode || paging_mode)) {
         streams.err.append_format(BUILTIN_ERR_COMBO, argv[0]);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -366,7 +366,7 @@ int builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_t **argv)
         streams.err.append_format(
             BUILTIN_ERR_COMBO2, cmd,
             L"--cut-at-cursor and --tokenize can not be used when setting the commandline");
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -374,7 +374,7 @@ int builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_t **argv)
         streams.err.append_format(
             BUILTIN_ERR_COMBO2, cmd,
             L"insertion mode switches can not be used when not in insertion mode");
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -392,7 +392,7 @@ int builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_t **argv)
             long new_pos = fish_wcstol(argv[w.woptind]);
             if (errno) {
                 streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, argv[w.woptind]);
-                builtin_print_help(parser, streams, cmd, streams.err);
+                builtin_print_error_trailer(parser, streams.err, cmd);
             }
 
             current_buffer = reader_get_buffer();
diff --git a/src/builtin_complete.cpp b/src/builtin_complete.cpp
index c96571210..c3b25cc75 100644
--- a/src/builtin_complete.cpp
+++ b/src/builtin_complete.cpp
@@ -270,7 +270,7 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
 
     if (w.woptind != argc) {
         streams.err.append_format(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
diff --git a/src/builtin_disown.cpp b/src/builtin_disown.cpp
index 382c95099..52e2c995b 100644
--- a/src/builtin_disown.cpp
+++ b/src/builtin_disown.cpp
@@ -19,7 +19,7 @@
 static int disown_job(const wchar_t *cmd, parser_t &parser, io_streams_t &streams, job_t *j) {
     if (j == 0) {
         streams.err.append_format(_(L"%ls: Unknown job '%ls'\n"), L"bg");
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
diff --git a/src/builtin_exit.cpp b/src/builtin_exit.cpp
index d089598ae..9337f7e23 100644
--- a/src/builtin_exit.cpp
+++ b/src/builtin_exit.cpp
@@ -73,7 +73,7 @@ int builtin_exit(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
 
     if (optind + 1 < argc) {
         streams.err.append_format(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -84,7 +84,7 @@ int builtin_exit(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         if (errno) {
             streams.err.append_format(_(L"%ls: Argument '%ls' must be an integer\n"), cmd,
                                       argv[optind]);
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_INVALID_ARGS;
         }
     }
diff --git a/src/builtin_fg.cpp b/src/builtin_fg.cpp
index a89f45866..8a915ecc1 100644
--- a/src/builtin_fg.cpp
+++ b/src/builtin_fg.cpp
@@ -68,14 +68,14 @@ int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
             streams.err.append_format(_(L"%ls: '%ls' is not a job\n"), cmd, argv[optind]);
         }
 
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
 
         j = 0;
     } else {
         int pid = abs(fish_wcstoi(argv[optind]));
         if (errno) {
             streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, argv[optind]);
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
         } else {
             j = job_t::from_pid(pid);
             if (!j || !j->is_constructed() || j->is_completed()) {
diff --git a/src/builtin_function.cpp b/src/builtin_function.cpp
index 104a728d8..db9777f32 100644
--- a/src/builtin_function.cpp
+++ b/src/builtin_function.cpp
@@ -230,7 +230,7 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis
     if (retval != STATUS_CMD_OK) return retval;
 
     if (opts.print_help) {
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_CMD_OK;
     }
 
diff --git a/src/builtin_functions.cpp b/src/builtin_functions.cpp
index c9d02753b..91b334e6e 100644
--- a/src/builtin_functions.cpp
+++ b/src/builtin_functions.cpp
@@ -282,7 +282,7 @@ int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     bool describe = opts.description ? true : false;
     if (describe + opts.erase + opts.list + opts.query + opts.copy > 1) {
         streams.err.append_format(_(L"%ls: Invalid combination of options\n"), cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -296,14 +296,14 @@ int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
 
         if (argc - optind != 1) {
             streams.err.append_format(_(L"%ls: Expected exactly one function name\n"), cmd);
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_INVALID_ARGS;
         }
 
         func = argv[optind];
         if (!function_exists(func)) {
             streams.err.append_format(_(L"%ls: Function '%ls' does not exist\n"), cmd, func);
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_CMD_ERROR;
         }
 
@@ -370,7 +370,7 @@ int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
             streams.err.append_format(_(L"%ls: Expected exactly two names (current function name, "
                                         L"and new function name)\n"),
                                       cmd);
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_INVALID_ARGS;
         }
         current_func = argv[optind];
@@ -379,14 +379,14 @@ int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         if (!function_exists(current_func)) {
             streams.err.append_format(_(L"%ls: Function '%ls' does not exist\n"), cmd,
                                       current_func.c_str());
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_CMD_ERROR;
         }
 
         if (!valid_func_name(new_func) || parser_keywords_is_reserved(new_func)) {
             streams.err.append_format(_(L"%ls: Illegal function name '%ls'\n"), cmd,
                                       new_func.c_str());
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_INVALID_ARGS;
         }
 
@@ -395,7 +395,7 @@ int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
             streams.err.append_format(
                 _(L"%ls: Function '%ls' already exists. Cannot create copy '%ls'\n"), cmd,
                 new_func.c_str(), current_func.c_str());
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_CMD_ERROR;
         }
 
diff --git a/src/builtin_read.cpp b/src/builtin_read.cpp
index 077bf164d..ec8dc974a 100644
--- a/src/builtin_read.cpp
+++ b/src/builtin_read.cpp
@@ -130,13 +130,13 @@ static int parse_cmd_opts(read_cmd_opts_t &opts, int *optind,  //!OCLINT(high nc
                     if (errno == ERANGE) {
                         streams.err.append_format(_(L"%ls: Argument '%ls' is out of range\n"), cmd,
                                                   w.woptarg);
-                        builtin_print_help(parser, streams, cmd, streams.err);
+                        builtin_print_error_trailer(parser, streams.err, cmd);
                         return STATUS_INVALID_ARGS;
                     }
 
                     streams.err.append_format(_(L"%ls: Argument '%ls' must be an integer\n"), cmd,
                                               w.woptarg);
-                    builtin_print_help(parser, streams, cmd, streams.err);
+                    builtin_print_error_trailer(parser, streams.err, cmd);
                     return STATUS_INVALID_ARGS;
                 }
                 break;
@@ -351,7 +351,7 @@ static int validate_read_args(const wchar_t *cmd, read_cmd_opts_t &opts, int arg
                               const wchar_t *const *argv, parser_t &parser, io_streams_t &streams) {
     if (opts.prompt && opts.prompt_str) {
         streams.err.append_format(_(L"%ls: Options %ls and %ls cannot be used together\n"), cmd, L"-p", L"-P");
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -373,7 +373,7 @@ static int validate_read_args(const wchar_t *cmd, read_cmd_opts_t &opts, int arg
 
     if ((opts.place & ENV_UNEXPORT) && (opts.place & ENV_EXPORT)) {
         streams.err.append_format(BUILTIN_ERR_EXPUNEXP, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -381,7 +381,7 @@ static int validate_read_args(const wchar_t *cmd, read_cmd_opts_t &opts, int arg
             (opts.place & ENV_UNIVERSAL ? 1 : 0) >
         1) {
         streams.err.append_format(BUILTIN_ERR_GLOCAL, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -404,7 +404,7 @@ static int validate_read_args(const wchar_t *cmd, read_cmd_opts_t &opts, int arg
     for (int i = 0; i < argc; i++) {
         if (!valid_var_name(argv[i])) {
             streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, argv[i]);
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_INVALID_ARGS;
         }
     }
diff --git a/src/builtin_return.cpp b/src/builtin_return.cpp
index 27ce2beda..32a201af8 100644
--- a/src/builtin_return.cpp
+++ b/src/builtin_return.cpp
@@ -73,7 +73,7 @@ int builtin_return(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
 
     if (optind + 1 < argc) {
         streams.err.append_format(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -83,7 +83,7 @@ int builtin_return(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         retval = fish_wcstoi(argv[1]);
         if (errno) {
             streams.err.append_format(_(L"%ls: Argument '%ls' must be an integer\n"), cmd, argv[1]);
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_INVALID_ARGS;
         }
         retval &= 0xFF;
@@ -98,7 +98,7 @@ int builtin_return(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
 
     if (function_block_idx >= parser.block_count()) {
         streams.err.append_format(_(L"%ls: Not inside of function\n"), cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_CMD_ERROR;
     }
 
diff --git a/src/builtin_set.cpp b/src/builtin_set.cpp
index 3a18e5a09..f435bbff7 100644
--- a/src/builtin_set.cpp
+++ b/src/builtin_set.cpp
@@ -175,42 +175,42 @@ static int validate_cmd_opts(const wchar_t *cmd, set_cmd_opts_t &opts,  //!OCLIN
     // Can't query and erase or list.
     if (opts.query && (opts.erase || opts.list)) {
         streams.err.append_format(BUILTIN_ERR_COMBO, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
     // We can't both list and erase variables.
     if (opts.erase && opts.list) {
         streams.err.append_format(BUILTIN_ERR_COMBO, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
     // Variables can only have one scope.
     if (opts.local + opts.global + opts.universal > 1) {
         streams.err.append_format(BUILTIN_ERR_GLOCAL, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
     // Variables can only have one export status.
     if (opts.exportv && opts.unexport) {
         streams.err.append_format(BUILTIN_ERR_EXPUNEXP, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
     // Variables can only have one path status.
     if (opts.pathvar && opts.unpathvar) {
         streams.err.append_format(BUILTIN_ERR_EXPUNEXP, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
     // Trying to erase and (un)export at the same time doesn't make sense.
     if (opts.erase && (opts.exportv || opts.unexport)) {
         streams.err.append_format(BUILTIN_ERR_COMBO, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -218,13 +218,13 @@ static int validate_cmd_opts(const wchar_t *cmd, set_cmd_opts_t &opts,  //!OCLIN
     if (opts.show &&
         (opts.local || opts.global || opts.erase || opts.list || opts.exportv || opts.universal)) {
         streams.err.append_format(BUILTIN_ERR_COMBO, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
     if (argc == 0 && opts.erase) {
         streams.err.append_format(BUILTIN_SET_ERASE_NO_VAR, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -519,7 +519,7 @@ static int builtin_set_query(const wchar_t *cmd, set_cmd_opts_t &opts, int argc,
         int idx_count = parse_index(indexes, dest, scope, streams, parser.vars());
         if (idx_count == -1) {
             free(dest);
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_CMD_ERROR;
         }
 
@@ -612,7 +612,7 @@ static int builtin_set_show(const wchar_t *cmd, set_cmd_opts_t &opts, int argc,
             if (std::wcschr(arg, L'[')) {
                 streams.err.append_format(
                     _(L"%ls: `set --show` does not allow slices with the var names\n"), cmd);
-                builtin_print_help(parser, streams, cmd, streams.err);
+                builtin_print_error_trailer(parser, streams.err, cmd);
                 return STATUS_CMD_ERROR;
             }
 
@@ -631,7 +631,7 @@ static int builtin_set_erase(const wchar_t *cmd, set_cmd_opts_t &opts, int argc,
                              parser_t &parser, io_streams_t &streams) {
     if (argc != 1) {
         streams.err.append_format(BUILTIN_ERR_ARG_COUNT2, cmd, L"--erase", 1, argc);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_CMD_ERROR;
     }
 
@@ -641,14 +641,14 @@ static int builtin_set_erase(const wchar_t *cmd, set_cmd_opts_t &opts, int argc,
     std::vector<long> indexes;
     int idx_count = parse_index(indexes, dest, scope, streams, parser.vars());
     if (idx_count == -1) {
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_CMD_ERROR;
     }
 
     int retval;
     if (!valid_var_name(dest)) {
         streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, dest);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -709,7 +709,7 @@ static int set_var_slices(const wchar_t *cmd, set_cmd_opts_t &opts, const wchar_
     if (opts.append || opts.prepend) {
         streams.err.append_format(
             L"%ls: Cannot use --append or --prepend when assigning to a slice", cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -740,7 +740,7 @@ static int builtin_set_set(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, w
                            parser_t &parser, io_streams_t &streams) {
     if (argc == 0) {
         streams.err.append_format(BUILTIN_ERR_MIN_ARG_COUNT1, cmd, 1);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -752,13 +752,13 @@ static int builtin_set_set(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, w
     std::vector<long> indexes;
     int idx_count = parse_index(indexes, varname, scope, streams, parser.vars());
     if (idx_count == -1) {
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
     if (!valid_var_name(varname)) {
         streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, varname);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
diff --git a/src/builtin_ulimit.cpp b/src/builtin_ulimit.cpp
index 553c35514..40fe6bfef 100644
--- a/src/builtin_ulimit.cpp
+++ b/src/builtin_ulimit.cpp
@@ -269,7 +269,7 @@ int builtin_ulimit(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         return STATUS_CMD_OK;
     } else if (arg_count != 1) {
         streams.err.append_format(BUILTIN_ERR_TOO_MANY_ARGUMENTS, cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     }
 
@@ -282,7 +282,7 @@ int builtin_ulimit(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     rlim_t new_limit;
     if (*argv[w.woptind] == L'\0') {
         streams.err.append_format(_(L"%ls: New limit cannot be an empty string\n"), cmd);
-        builtin_print_help(parser, streams, cmd, streams.err);
+        builtin_print_error_trailer(parser, streams.err, cmd);
         return STATUS_INVALID_ARGS;
     } else if (wcscasecmp(argv[w.woptind], L"unlimited") == 0) {
         new_limit = RLIM_INFINITY;
@@ -294,7 +294,7 @@ int builtin_ulimit(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         new_limit = fish_wcstol(argv[w.woptind]);
         if (errno) {
             streams.err.append_format(_(L"%ls: Invalid limit '%ls'\n"), cmd, argv[w.woptind]);
-            builtin_print_help(parser, streams, cmd, streams.err);
+            builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_INVALID_ARGS;
         }
         new_limit *= get_multiplier(what);
diff --git a/tests/string.err b/tests/string.err
index 0b2106c73..d291768ce 100644
--- a/tests/string.err
+++ b/tests/string.err
@@ -185,9 +185,6 @@ string match: ^
 ####################
 # string invalidarg
 string: Subcommand 'invalidarg' is not valid
-Standard input (line 211): 
-string invalidarg; and echo "unexpected exit 0"
-^
 
 ####################
 # string length
@@ -270,9 +267,6 @@ string repeat: Expected argument
 ####################
 # string repeat -l fakearg 2>&1
 string repeat: Unknown option '-l'
-Standard input (line 287): 
-string repeat -l fakearg
-^
 
 ####################
 # string repeat ""