From ecfa6b273408d11ab0499ed9ee05605e9c210c05 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 25 Sep 2024 16:17:07 +0100 Subject: [PATCH] Don't print long usage messages on a command-line error. In the course of debugging the command-line argument refactoring in previous commits, I found I wasn't quite sure whether PSCP thought I'd given it too many arguments, or too few, because it didn't print an error message saying which: it just printed its giant usage message. Over the last few years I've come to the belief that this is Just Wrong anyway. Printing the whole of a giant help message should only be done when the user asked for it: otherwise, print a short and to-the-point error, and maybe _suggest_ how to get help, but scrolling everything else off the user's screen is not a good response to a typo. I wrote this thought up more fully last year: https://www.chiark.greenend.org.uk/~sgtatham/quasiblog/stop-helping/ So, time to practise what I preach! The PuTTY tools now follow the 'Stop helping!' principle. You can get full help by saying --help. Also, when we do print the help, we now exit(0) rather than exit(1), because there's no reason to report failure: we successfully did what the user asked us for. --- cmdgen.c | 5 +++-- pscp.c | 6 +++--- psftp.c | 4 ++-- unix/pageant.c | 1 - unix/plink.c | 6 ++++-- windows/plink.c | 7 +++++-- 6 files changed, 17 insertions(+), 12 deletions(-) diff --git a/cmdgen.c b/cmdgen.c index b12758a1..295cdd77 100644 --- a/cmdgen.c +++ b/cmdgen.c @@ -718,10 +718,11 @@ int main(int argc, char **argv) /* * If run with at least one argument _but_ not the required - * ones, print the usage message and return failure. + * ones, fail with an error. */ if (!infile && keytype == NOKEYGEN) { - usage(true); + fprintf(stderr, "puttygen: expected an input key file name, " + "or -t for a type of key to generate\n"); RETURN(1); } diff --git a/pscp.c b/pscp.c index 93e31204..192bc138 100644 --- a/pscp.c +++ b/pscp.c @@ -2236,7 +2236,6 @@ static void usage(void) printf(" -logoverwrite\n"); printf(" -logappend\n"); printf(" control what happens when a log file already exists\n"); - cleanup_exit(1); } void version(void) @@ -2311,6 +2310,7 @@ int psftp_main(CmdlineArgList *arglist) strcmp(argstr, "-?") == 0 || strcmp(argstr, "--help") == 0) { usage(); + cleanup_exit(0); } else if (strcmp(argstr, "-V") == 0 || strcmp(argstr, "--version") == 0) { version(); @@ -2351,12 +2351,12 @@ int psftp_main(CmdlineArgList *arglist) if (list) { if (nscpargs != 1) - usage(); + cmdline_error("expected a single argument with -ls"); get_dir_list(scpargs, nscpargs); } else { if (nscpargs < 2) - usage(); + cmdline_error("expected at least two arguments"); if (nscpargs > 2) targetshouldbedirectory = true; diff --git a/psftp.c b/psftp.c index 7ac0bcfd..78d6d139 100644 --- a/psftp.c +++ b/psftp.c @@ -2565,7 +2565,6 @@ static void usage(void) printf(" -logoverwrite\n"); printf(" -logappend\n"); printf(" control what happens when a log file already exists\n"); - cleanup_exit(1); } static void version(void) @@ -2819,7 +2818,7 @@ int psftp_main(CmdlineArgList *arglist) if (argstr[0] != '-') { if (userhost) - usage(); + cmdline_error("unexpected extra argument \"%s\"", argstr); else userhost = dupstr(argstr); continue; @@ -2837,6 +2836,7 @@ int psftp_main(CmdlineArgList *arglist) strcmp(argstr, "-?") == 0 || strcmp(argstr, "--help") == 0) { usage(); + cleanup_exit(0); } else if (strcmp(argstr, "-pgpfp") == 0) { pgp_fingerprints(); return 1; diff --git a/unix/pageant.c b/unix/pageant.c index 4d7a4fa5..36ae2d22 100644 --- a/unix/pageant.c +++ b/unix/pageant.c @@ -218,7 +218,6 @@ static void usage(void) printf(" --tty-prompt force tty-based passphrase prompt\n"); printf(" --gui-prompt force GUI-based passphrase prompt\n"); printf(" --askpass behave like a standalone askpass program\n"); - exit(1); } static void version(void) diff --git a/unix/plink.c b/unix/plink.c index 0f8056e0..f2b6b072 100644 --- a/unix/plink.c +++ b/unix/plink.c @@ -573,7 +573,6 @@ static void usage(void) printf(" control what happens when a log file already exists\n"); printf(" -shareexists\n"); printf(" test whether a connection-sharing upstream exists\n"); - exit(1); } static void version(void) @@ -807,7 +806,10 @@ int main(int argc, char **argv) return 1; if (!cmdline_host_ok(conf)) { - usage(); + fprintf(stderr, "plink: no valid host name provided\n" + "try \"plink --help\" for help\n"); + cmdline_arg_list_free(arglist); + return 1; } prepare_session(conf); diff --git a/windows/plink.c b/windows/plink.c index 8184d619..95025281 100644 --- a/windows/plink.c +++ b/windows/plink.c @@ -189,7 +189,6 @@ static void usage(void) printf(" control what happens when a log file already exists\n"); printf(" -shareexists\n"); printf(" test whether a connection-sharing upstream exists\n"); - exit(1); } static void version(void) @@ -350,6 +349,7 @@ int main(int argc, char **argv) version(); } else if (!strcmp(p, "--help")) { usage(); + exit(0); } else if (!strcmp(p, "-pgpfp")) { pgp_fingerprints(); exit(1); @@ -395,7 +395,10 @@ int main(int argc, char **argv) return 1; if (!cmdline_host_ok(conf)) { - usage(); + fprintf(stderr, "plink: no valid host name provided\n" + "try \"plink --help\" for help\n"); + cmdline_arg_list_free(arglist); + return 1; } prepare_session(conf);