findutils-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Findutils-patches] [PATCH 4/6] all: output full usage only when request


From: Bernhard Voelker
Subject: [Findutils-patches] [PATCH 4/6] all: output full usage only when requested via the --help option
Date: Tue, 16 Feb 2016 23:47:13 +0100

When there are invalid options or arguments, all utilities printed the
full usage in addition to the original error diagnostic.  The quite
large text distracted the user's attention from the error message.

* find/defs.h (usage): Mark as _GL_ATTRIBUTE_NORETURN, and remove the
arguments FP and MSG.
* find/parser.c (parse_help): Factor out the usage text from here ...
* find/util.c (usage): ... to here.  Unless called with EXIT_SUCCESS,
just emit a short hint to try again with the --help option rather than
emitting the full usage.  Afterward, exit the program with the given
status.
(debugassoc): Sort array, as it is used in that order when outputting
the usage text.
(process_debug_options): Also output an error diagnostic when no
argument to -D is passed.
(show_valid_options): Always output to stdout, always output the header
line.  Update all callers.
* find/tree.c (build_expression): Update invocation of usage().
Also call it for syntax errors like unknown or invalid predicates
or invalid or missing arguments.
* locate/frcode.c (usage): Similar to the change in util.c (usage).
(main): Update invocation of usage().  Also output an error diagnostic
when an unexpected argument is passed.
* locate/locate.c (usage):  Similar to the change in util.c (usage).
(dolocate): Update invocation of usage().  Also output an error
diagnostic when an unexpected argument is passed.
* locate/updatedb.sh: Output "Try ... --help ..." when an invalid option
is passed.
* xargs/xargs.c (usage): Similar to the change in util.c (usage).
(parse_num, main): Update all callers, removing the now-obsolete
return or exit().
* NEWS (Improvements): Mention the change.
---
 NEWS               |  7 +++++
 find/defs.h        |  2 +-
 find/parser.c      | 34 +---------------------
 find/tree.c        | 16 ++++++-----
 find/util.c        | 83 +++++++++++++++++++++++++++++++++++++++++-------------
 locate/frcode.c    | 31 ++++++++++++--------
 locate/locate.c    | 26 +++++++++++------
 locate/updatedb.sh |  2 +-
 xargs/xargs.c      | 48 +++++++++++++++----------------
 9 files changed, 140 insertions(+), 109 deletions(-)

diff --git a/NEWS b/NEWS
index 6e4f43c..16477dc 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,13 @@ database is now different to previous versions.  However, 
you should
 not rely on locate's output appearing in any particular order in any
 case.
 
+** Improvements
+
+All utilities now only show the full usage text when requested via
+the --help option.  Previously, when the user passed invalid options
+or arguments, the user's attention to the corresponding error
+diagnostic was distracted by that lengthy text.
+
 ** Translations
 
 Updated translations: Hungarian, Slovak, Dutch, German, Danish.
diff --git a/find/defs.h b/find/defs.h
index 71184a5..9311565 100644
--- a/find/defs.h
+++ b/find/defs.h
@@ -482,7 +482,7 @@ bool fd_leak_check_is_enabled (void);
 struct predicate *insert_primary (const struct parser_table *entry, const char 
*arg);
 struct predicate *insert_primary_noarg (const struct parser_table *entry);
 struct predicate *insert_primary_withpred (const struct parser_table *entry, 
PRED_FUNC fptr, const char *arg);
-void usage (FILE *fp, int status, char *msg);
+void usage (int status) _GL_ATTRIBUTE_NORETURN;
 extern bool check_nofollow(void);
 void complete_pending_execs(struct predicate *p);
 void complete_pending_execdirs (void);
diff --git a/find/parser.c b/find/parser.c
index 54fee03..57fb296 100644
--- a/find/parser.c
+++ b/find/parser.c
@@ -1233,39 +1233,7 @@ parse_help (const struct parser_table* entry, char 
**argv, int *arg_ptr)
   (void) argv;
   (void) arg_ptr;
 
-  usage (stdout, 0, NULL);
-  puts (_("\n\
-default path is the current directory; default expression is -print\n\
-expression may consist of: operators, options, tests, and actions:\n"));
-  puts (_("\
-operators (decreasing precedence; -and is implicit where no others are 
given):\n\
-      ( EXPR )   ! EXPR   -not EXPR   EXPR1 -a EXPR2   EXPR1 -and EXPR2\n\
-      EXPR1 -o EXPR2   EXPR1 -or EXPR2   EXPR1 , EXPR2\n"));
-  puts (_("\
-positional options (always true): -daystart -follow -regextype\n\n\
-normal options (always true, specified before other expressions):\n\
-      -depth --help -maxdepth LEVELS -mindepth LEVELS -mount -noleaf\n\
-      --version -xdev -ignore_readdir_race -noignore_readdir_race\n"));
-  puts (_("\
-tests (N can be +N or -N or N): -amin N -anewer FILE -atime N -cmin N\n\
-      -cnewer FILE -ctime N -empty -false -fstype TYPE -gid N -group NAME\n\
-      -ilname PATTERN -iname PATTERN -inum N -iwholename PATTERN -iregex 
PATTERN\n\
-      -links N -lname PATTERN -mmin N -mtime N -name PATTERN -newer FILE"));
-  puts (_("\
-      -nouser -nogroup -path PATTERN -perm [-/]MODE -regex PATTERN\n\
-      -readable -writable -executable\n\
-      -wholename PATTERN -size N[bcwkMG] -true -type [bcdpflsD] -uid N\n\
-      -used N -user NAME -xtype [bcdpfls]"));
-  puts (_("\
-      -context CONTEXT\n"));
-  puts (_("\n\
-actions: -delete -print0 -printf FORMAT -fprintf FILE FORMAT -print \n\
-      -fprint0 FILE -fprint FILE -ls -fls FILE -prune -quit\n\
-      -exec COMMAND ; -exec COMMAND {} + -ok COMMAND ;\n\
-      -execdir COMMAND ; -execdir COMMAND {} + -okdir COMMAND ;\n\
-"));
-  explain_how_to_report_bugs (stdout, program_name);
-  exit (EXIT_SUCCESS);
+  usage (EXIT_SUCCESS);
 }
 
 static float
diff --git a/find/tree.c b/find/tree.c
index e471279..8a413f8 100644
--- a/find/tree.c
+++ b/find/tree.c
@@ -1286,7 +1286,7 @@ build_expression_tree (int argc, char *argv[], int 
end_of_leading_options)
       if (!looks_like_expression (argv[i], false))
        {
          error (0, 0, _("paths must precede expression: %s"), argv[i]);
-         usage (stderr, 1, NULL);
+         usage (EXIT_FAILURE);
        }
 
       predicate_name = argv[i];
@@ -1294,7 +1294,8 @@ build_expression_tree (int argc, char *argv[], int 
end_of_leading_options)
       if (parse_entry == NULL)
        {
          /* Command line option not recognized */
-         error (EXIT_FAILURE, 0, _("unknown predicate `%s'"), predicate_name);
+         error (0, 0, _("unknown predicate `%s'"), predicate_name);
+         usage (EXIT_FAILURE);
        }
 
       /* We have recognised a test of the form -foo.  Eat that,
@@ -1314,20 +1315,21 @@ build_expression_tree (int argc, char *argv[], int 
end_of_leading_options)
                  /* The special parse function spat out the
                   * predicate.  It must be invalid, or not tasty.
                   */
-                 error (EXIT_FAILURE, 0, _("invalid predicate `%s'"),
-                        predicate_name);
+                 error (0, 0, _("invalid predicate `%s'"), predicate_name);
+                 usage (EXIT_FAILURE);
                }
              else
                {
-                 error (EXIT_FAILURE, 0, _("invalid argument `%s' to `%s'"),
+                 error (0, 0, _("invalid argument `%s' to `%s'"),
                         argv[i], predicate_name);
+                 usage (EXIT_FAILURE);
                }
            }
          else
            {
              /* Command line option requires an argument */
-             error (EXIT_FAILURE, 0,
-                    _("missing argument to `%s'"), predicate_name);
+             error (0, 0, _("missing argument to `%s'"), predicate_name);
+             usage (EXIT_FAILURE);
            }
        }
       else
diff --git a/find/util.c b/find/util.c
index 51bc597..f35924c 100644
--- a/find/util.c
+++ b/find/util.c
@@ -61,14 +61,14 @@ struct debug_option_assoc
 };
 static struct debug_option_assoc debugassoc[] =
   {
+    { "exec", DebugExec, "Show diagnostic information relating to -exec, 
-execdir, -ok and -okdir" },
     { "help", DebugHelp, "Explain the various -D options" },
-    { "tree", DebugExpressionTree, "Display the expression tree" },
+    { "opt",  DebugExpressionTree|DebugTreeOpt, "Show diagnostic information 
relating to optimisation" },
+    { "rates", DebugSuccessRates, "Indicate how often each predicate 
succeeded" },
     { "search",DebugSearch, "Navigate the directory tree verbosely" },
     { "stat", DebugStat, "Trace calls to stat(2) and lstat(2)" },
-    { "rates", DebugSuccessRates, "Indicate how often each predicate 
succeeded" },
-    { "opt",  DebugExpressionTree|DebugTreeOpt, "Show diagnostic information 
relating to optimisation" },
-    { "exec", DebugExec,  "Show diagnostic information relating to -exec, 
-execdir, -ok and -okdir" },
-    { "time", DebugTime,  "Show diagnostic information relating to time-of-day 
and timestamp comparisons" }
+    { "time", DebugTime, "Show diagnostic information relating to time-of-day 
and timestamp comparisons" },
+    { "tree", DebugExpressionTree, "Display the expression tree" }
   };
 #define N_DEBUGASSOC (sizeof(debugassoc)/sizeof(debugassoc[0]))
 
@@ -138,15 +138,15 @@ insert_primary_noarg (const struct parser_table *entry)
 
 
 static void
-show_valid_debug_options (FILE *fp, int full)
+show_valid_debug_options (int full)
 {
   size_t i;
+  fputs (_("Valid arguments for -D:\n"), stdout);
   if (full)
     {
-      fprintf (fp, "Valid arguments for -D:\n");
       for (i=0; i<N_DEBUGASSOC; ++i)
        {
-         fprintf (fp, "%-10s %s\n",
+         fprintf (stdout, "%-10s %s\n",
                   debugassoc[i].name,
                   debugassoc[i].docstring);
        }
@@ -155,22 +155,64 @@ show_valid_debug_options (FILE *fp, int full)
     {
       for (i=0; i<N_DEBUGASSOC; ++i)
        {
-         fprintf (fp, "%s%s", (i>0 ? "|" : ""), debugassoc[i].name);
+         fprintf (stdout, "%s%s", (i>0 ? ", " : ""), debugassoc[i].name);
        }
     }
 }
 
 void
-usage (FILE *fp, int status, char *msg)
+usage (int status)
 {
-  if (msg)
-    fprintf (fp, "%s: %s\n", program_name, msg);
-
-  fprintf (fp, _("Usage: %s [-H] [-L] [-P] [-Olevel] [-D "), program_name);
-  show_valid_debug_options (fp, 0);
-  fprintf (fp, _("] [path...] [expression]\n"));
-  if (0 != status)
-    exit (status);
+  if (status != EXIT_SUCCESS)
+    {
+      fprintf (stderr, _("Try '%s --help' for more information.\n"), 
program_name);
+      exit (status);
+    }
+
+#define HTL(t) fputs (t, stdout);
+
+  fprintf (stdout, _("\
+Usage: %s [-H] [-L] [-P] [-Olevel] [-D debugopts] [path...] [expression]\n"),
+           program_name);
+
+  HTL (_("\n\
+default path is the current directory; default expression is -print\n\
+expression may consist of: operators, options, tests, and actions:\n"));
+  HTL (_("\
+operators (decreasing precedence; -and is implicit where no others are 
given):\n\
+      ( EXPR )   ! EXPR   -not EXPR   EXPR1 -a EXPR2   EXPR1 -and EXPR2\n\
+      EXPR1 -o EXPR2   EXPR1 -or EXPR2   EXPR1 , EXPR2\n"));
+  HTL (_("\
+positional options (always true): -daystart -follow -regextype\n\n\
+normal options (always true, specified before other expressions):\n\
+      -depth --help -maxdepth LEVELS -mindepth LEVELS -mount -noleaf\n\
+      --version -xdev -ignore_readdir_race -noignore_readdir_race\n"));
+  HTL (_("\
+tests (N can be +N or -N or N): -amin N -anewer FILE -atime N -cmin N\n\
+      -cnewer FILE -ctime N -empty -false -fstype TYPE -gid N -group NAME\n\
+      -ilname PATTERN -iname PATTERN -inum N -iwholename PATTERN -iregex 
PATTERN\n\
+      -links N -lname PATTERN -mmin N -mtime N -name PATTERN -newer FILE"));
+  HTL (_("\n\
+      -nouser -nogroup -path PATTERN -perm [-/]MODE -regex PATTERN\n\
+      -readable -writable -executable\n\
+      -wholename PATTERN -size N[bcwkMG] -true -type [bcdpflsD] -uid N\n\
+      -used N -user NAME -xtype [bcdpfls]"));
+  HTL (_("\
+      -context CONTEXT\n"));
+  HTL (_("\n\
+actions: -delete -print0 -printf FORMAT -fprintf FILE FORMAT -print \n\
+      -fprint0 FILE -fprint FILE -ls -fls FILE -prune -quit\n\
+      -exec COMMAND ; -exec COMMAND {} + -ok COMMAND ;\n\
+      -execdir COMMAND ; -execdir COMMAND {} + -okdir COMMAND ;\n\
+\n"));
+
+  show_valid_debug_options (0);
+  HTL (_("\n\
+Use '-D help' for a description of the options, or see find(1)\n\
+\n"));
+
+  explain_how_to_report_bugs (stdout, program_name);
+  exit (status);
 }
 
 void
@@ -827,11 +869,12 @@ process_debug_options (char *arg)
     }
   if (empty)
     {
-      error(EXIT_FAILURE, 0, _("Empty argument to the -D option."));
+      error (0, 0, _("Empty argument to the -D option."));
+      usage (EXIT_FAILURE);
     }
   else if (options.debug_options & DebugHelp)
     {
-      show_valid_debug_options (stdout, 1);
+      show_valid_debug_options (1);
       exit (EXIT_SUCCESS);
     }
 }
diff --git a/locate/frcode.c b/locate/frcode.c
index d0fb03f..c1fea64 100644
--- a/locate/frcode.c
+++ b/locate/frcode.c
@@ -88,6 +88,7 @@
 #include "findutils-version.h"
 #include "bugreports.h"
 #include "locatedb.h"
+#include "gcc-function-attributes.h"
 
 #if ENABLE_NLS
 # include <libintl.h>
@@ -142,13 +143,21 @@ static struct option const longopts[] =
 
 extern char *version_string;
 
-static void
-usage (FILE *stream)
+static void _GL_ATTRIBUTE_NORETURN
+usage (int status)
 {
-  fprintf (stream,
+  if (status != EXIT_SUCCESS)
+    {
+      fprintf (stderr, _("Try '%s --help' for more information.\n"), 
program_name);
+      exit (status);
+    }
+
+  fprintf (stdout,
           _("Usage: %s [-0 | --null] [--version] [--help]\n"),
           program_name);
-  explain_how_to_report_bugs (stream, program_name);
+
+  explain_how_to_report_bugs (stdout, program_name);
+  exit (status);
 }
 
 static long
@@ -157,8 +166,8 @@ get_seclevel (char *s)
   long result;
   char *p;
 
-  /* Reset errno in oreder to be able to distinguish LONG_MAX/LONG_MIN
-   * from values whichare actually out of range
+  /* Reset errno in order to be able to distinguish LONG_MAX/LONG_MIN
+   * from values which are actually out of range.
    */
   errno = 0;
 
@@ -249,23 +258,21 @@ main (int argc, char **argv)
        break;
 
       case 'h':
-       usage (stdout);
-       return 0;
+       usage (EXIT_SUCCESS);
 
       case 'v':
        display_findutils_version ("frcode");
        return 0;
 
       default:
-       usage (stderr);
-       return 1;
+       usage (EXIT_FAILURE);
       }
 
   /* We expect to have no arguments. */
   if (optind != argc)
     {
-      usage (stderr);
-      return 1;
+      error (0, 0, _("no argument expected."));
+      usage (EXIT_FAILURE);
     }
 
 
diff --git a/locate/locate.c b/locate/locate.c
index a50c2b1..c4a9714 100644
--- a/locate/locate.c
+++ b/locate/locate.c
@@ -100,6 +100,7 @@
 #include "printquoted.h"
 #include "bugreports.h"
 #include "splitstring.h"
+#include "gcc-function-attributes.h"
 
 
 #if ENABLE_NLS
@@ -1382,10 +1383,16 @@ search_one_database (int argc,
 
 extern char *version_string;
 
-static void
-usage (FILE *stream)
+static void _GL_ATTRIBUTE_NORETURN
+usage (int status)
 {
-  fprintf (stream, _("\
+  if (status != EXIT_SUCCESS)
+    {
+      fprintf (stderr, _("Try '%s --help' for more information.\n"), 
program_name);
+      exit (status);
+    }
+
+  fprintf (stdout, _("\
 Usage: %s [-d path | --database=path] [-e | -E | --[non-]existing]\n\
       [-i | --ignore-case] [-w | --wholename] [-b | --basename] \n\
       [--limit=N | -l N] [-S | --statistics] [-0 | --null] [-c | --count]\n\
@@ -1394,8 +1401,11 @@ Usage: %s [-d path | --database=path] [-e | -E | 
--[non-]existing]\n\
       [--max-database-age D] [--version] [--help]\n\
       pattern...\n"),
            program_name);
+
   explain_how_to_report_bugs (stdout, program_name);
+  exit (status);
 }
+
 enum
   {
     REGEXTYPE_OPTION = CHAR_MAX + 1,
@@ -1642,8 +1652,7 @@ dolocate (int argc, char **argv, int secure_db_fd)
           break;
 
         case 'h':
-          usage (stdout);
-          return 0;
+          usage (EXIT_SUCCESS);
 
         case MAX_DB_AGE:
           /* XXX: nothing in the test suite for this option. */
@@ -1707,8 +1716,7 @@ dolocate (int argc, char **argv, int secure_db_fd)
           break;
 
         default:
-          usage (stderr);
-          return 1;
+          usage (EXIT_FAILURE);
         }
     }
 
@@ -1737,8 +1745,8 @@ dolocate (int argc, char **argv, int secure_db_fd)
     {
       if (!just_count && optind == argc)
         {
-          usage (stderr);
-          return 1;
+          error (0, 0, _("pattern argument expected"));
+          usage (EXIT_FAILURE);
         }
     }
 
diff --git a/locate/updatedb.sh b/locate/updatedb.sh
index 13870ca..5d0e1eb 100644
--- a/locate/updatedb.sh
+++ b/locate/updatedb.sh
@@ -84,7 +84,7 @@ do
     --version) fail=0; echo "$version" || fail=1; exit $fail ;;
     --help)    fail=0; echo "$usage"   || fail=1; exit $fail ;;
     *) echo "updatedb: invalid option $opt
-$usage" >&2
+Try '$0 --help' for more information." >&2
        exit 1 ;;
   esac
 done
diff --git a/xargs/xargs.c b/xargs/xargs.c
index 8a84ade..5cf8c13 100644
--- a/xargs/xargs.c
+++ b/xargs/xargs.c
@@ -67,6 +67,7 @@
 #include "fdleak.h"
 #include "bugreports.h"
 #include "findutils-version.h"
+#include "gcc-function-attributes.h"
 
 #if ENABLE_NLS
 # include <libintl.h>
@@ -224,7 +225,7 @@ static void wait_for_proc_all (void);
 static void increment_proc_max (int);
 static void decrement_proc_max (int);
 static long parse_num (char *str, int option, long min, long max, int fatal);
-static void usage (FILE * stream);
+static void usage (int status) _GL_ATTRIBUTE_NORETURN;
 
 
 static char
@@ -532,8 +533,7 @@ main (int argc, char **argv)
          break;
 
        case 'h':
-         usage (stdout);
-         return 0;
+         usage (EXIT_SUCCESS);
 
        case 'I':               /* POSIX */
        case 'i':               /* deprecated */
@@ -652,8 +652,7 @@ main (int argc, char **argv)
          break;
 
        default:
-         usage (stderr);
-         return 1;
+         usage (EXIT_FAILURE);
        }
     }
 
@@ -1613,7 +1612,7 @@ parse_num (char *str, int option, long int min, long int 
max, int fatal)
     {
       fprintf (stderr, _("%s: invalid number \"%s\" for -%c option\n"),
               program_name, str, option);
-      usage (stderr);
+      usage (EXIT_FAILURE);
       exit (EXIT_FAILURE);
     }
   else if (val < min)
@@ -1621,40 +1620,36 @@ parse_num (char *str, int option, long int min, long 
int max, int fatal)
       fprintf (stderr, _("%s: value %s for -%c option should be >= %ld\n"),
               program_name, str, option, min);
       if (fatal)
-       {
-         usage (stderr);
-         exit (EXIT_FAILURE);
-       }
-      else
-       {
-         val = min;
-       }
+       usage (EXIT_FAILURE);
+
+      val = min;
     }
   else if (max >= 0 && val > max)
     {
       fprintf (stderr, _("%s: value %s for -%c option should be <= %ld\n"),
               program_name, str, option, max);
       if (fatal)
-       {
-         usage (stderr);
-         exit (EXIT_FAILURE);
-       }
-      else
-       {
-         val = max;
-       }
+       usage (EXIT_FAILURE);
+
+      val = max;
     }
   return val;
 }
 
 static void
-usage (FILE *stream)
+usage (int status)
 {
-  fprintf (stream,
+  if (status != EXIT_SUCCESS)
+    {
+      fprintf (stderr, _("Try '%s --help' for more information.\n"), 
program_name);
+      exit (status);
+    }
+
+  fprintf (stdout,
            _("Usage: %s [OPTION]... COMMAND [INITIAL-ARGS]...\n"),
            program_name);
 
-#define HTL(t) fputs (t, stream);
+#define HTL(t) fputs (t, stdout);
 
   HTL (_("Run COMMAND with arguments INITIAL-ARGS and more arguments read from 
input.\n"
          "\n"));
@@ -1694,5 +1689,6 @@ usage (FILE *stream)
 
   HTL (_("      --help                   display this help and exit\n"));
   HTL (_("      --version                output version information and 
exit\n\n"));
-  explain_how_to_report_bugs (stream, program_name);
+  explain_how_to_report_bugs (stdout, program_name);
+  exit (status);
 }
-- 
2.1.4




reply via email to

[Prev in Thread] Current Thread [Next in Thread]