[PATCH v2] add usage-strings check and amend remaining usage strings

["Abhradeep Chakraborty via GitGitGadget" <gitgitgadget@gmail.com>]

From: Abhra303 <chakrabortyabhradeep79@gmail.com>

Usage strings for git (sub)command flags has a style guide that
suggests - first letter should not capitalized (unless requied)
and it should skip full-stop at the end of line. But there are
some files where usage-strings do not follow the above mentioned
guide. Moreover, there are no checks to verify if all usage strings
are following the guide/convention or not.

Amend the usage strings that don't follow the convention/guide and
add a check in the `parse_options_check()` function in `parse-options.c`
to check the usage strings against the style guide.

Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
---
    add usage-strings ci check and amend remaining usage strings
    
    This patch series completely fixes #636.
    
    The issue is about amending the usage-strings (for command flags such as
    -h, -v etc.) which do not follow the style convention/guide. There was a
    PR [https://github.com/gitgitgadget/git/pull/920] addressing this issue
    but as Johannes [https://github.com/dscho] said in his comment
    [https://github.com/gitgitgadget/git/issues/636#issuecomment-1018660439],
    there are some files that still have those kind of usage strings.
    Johannes also suggested to add a CI check under ci/test-documentation.sh
    to check the usage strings.
    
    So, in this patch, all remaining usage strings are corrected. I also
    added checks to parse_options_check() in parse-options.c (as suggested
    by Ævar).
    
    Changes since v1:
    
     1. remove check-usage-strings.sh
     2. remove CI check
     3. add checks to parse-options.c
     4. modify t/t1502-rev-parse-parseopt.sh to pass the test

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1147%2FAbhra303%2Fusage_command_amend-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1147/Abhra303/usage_command_amend-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1147

Range-diff vs v1:

 1:  ea0ba45d77a ! 1:  902937e768d add usage-strings ci check and amend remaining usage strings
     @@ Metadata
      Author: Abhra303 <chakrabortyabhradeep79@gmail.com>
      
       ## Commit message ##
     -    add usage-strings ci check and amend remaining usage strings
     +    add usage-strings check  and amend remaining usage strings
      
          Usage strings for git (sub)command flags has a style guide that
          suggests - first letter should not capitalized (unless requied)
     @@ Commit message
          are following the guide/convention or not.
      
          Amend the usage strings that don't follow the convention/guide and
     -    add a `CI` check for checking the usage strings (whether the first
     -    letter is capital or it ends with full-stop). If the `check` find
     -    such strings then print those strings and return a non-zero status.
     -
     -    Also provide a script that takes an optional argument (a valid <tree>
     -    string), to check the usage strings in the given <tree> (`HEAD` is
     -    the default argument).
     +    add a check in the `parse_options_check()` function in `parse-options.c`
     +    to check the usage strings against the style guide.
      
          Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
      
     - ## Makefile ##
     -@@ Makefile: check-docs::
     - check-builtins::
     - 	./check-builtins.sh
     - 
     -+### Make sure all the usage strings follow usage string style guide
     -+#
     -+check-usage-strings::
     -+	./check-usage-strings.sh
     -+
     - ### Test suite coverage testing
     - #
     - .PHONY: coverage coverage-clean coverage-compile coverage-test coverage-report
     -
       ## builtin/bisect--helper.c ##
      @@ builtin/bisect--helper.c: int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
       		OPT_CMDMODE(0, "bisect-visualize", &cmdmode,
     @@ builtin/submodule--helper.c: static int module_clone(int argc, const char **argv
       			   N_("force cloning progress")),
       		OPT_BOOL(0, "require-init", &require_init,
      
     - ## check-usage-strings.sh (new) ##
     -@@
     -+{
     -+  if test -d ".git"
     -+  then
     -+    rev=${1:-"HEAD"}
     -+    for entry in $(git grep -l 'struct option .* = {$' "$rev" -- \*.c);
     -+    do
     -+      git show "$entry" |
     -+      sed -n '/struct option .* = {/,/OPT_END/{=;p;}' |
     -+      sed "N;s/^\\([0-9]*\\)\\n/$(echo "$entry" | sed 's/\//\\&/g'):\\1/";
     -+    done
     -+  else
     -+    for entry in $(grep -rl --include="*.c" 'struct option .* = {$' . );
     -+    do
     -+      cat "$entry" |
     -+      sed -n '/struct option .* = {/,/OPT_END/{=;p;}' |
     -+      sed "N;s/^\\([0-9]*\\)\\n/$(echo "$entry" | sed -e 's/\//\\&/g' -e 's/^\.\\\///'):\\1/";
     -+    done
     -+  fi
     -+} |
     -+grep -Pe '((?<!OPT_GROUP\(N_\(|OPT_GROUP\()"(?!GPG|DEPRECATED|SHA1|HEAD)[A-Z]|(?<!"|\.\.)\.")' |
     -+{
     -+  status=0
     -+  while read content;
     -+  do
     -+    if test -n "$content"
     -+    then
     -+      echo "$content";
     -+      status=1;
     -+    fi
     -+  done
     -+
     -+  exit $status
     -+}
     -
     - ## ci/test-documentation.sh ##
     -@@ ci/test-documentation.sh: filter_log () {
     - }
     - 
     - make check-builtins
     -+make check-usage-strings
     - make check-docs
     - 
     - # Build docs with AsciiDoc
     -
       ## diff.c ##
      @@ diff.c: static void prep_parse_options(struct diff_options *options)
       			       N_("select files by diff type"),
     @@ diff.c: static void prep_parse_options(struct diff_options *options)
       
       		OPT_END()
      
     + ## parse-options.c ##
     +@@ parse-options.c: static void parse_options_check(const struct option *opts)
     + 		default:
     + 			; /* ok. (usually accepts an argument) */
     + 		}
     ++		if (opts->type != OPTION_GROUP && opts->help &&
     ++			!(starts_with(opts->help, "HEAD") ||
     ++			  starts_with(opts->help, "GPG") ||
     ++			  starts_with(opts->help, "DEPRECATED") ||
     ++			  starts_with(opts->help, "SHA1")) &&
     ++			  (opts->help[0] >= 65 && opts->help[0] <= 90))
     ++			err |= optbug(opts, xstrfmt("help should not start with capital letter unless needed: %s", opts->help));
     ++		if (opts->help && !ends_with(opts->help, "...") && ends_with(opts->help, "."))
     ++			err |= optbug(opts, xstrfmt("help should not end with a dot: %s", opts->help));
     + 		if (opts->argh &&
     + 		    strcspn(opts->argh, " _") != strlen(opts->argh))
     + 			err |= optbug(opts, "multi-word argh should use dash to separate words");
     +
       ## t/helper/test-run-command.c ##
      @@ t/helper/test-run-command.c: static int quote_stress_test(int argc, const char **argv)
       	struct strbuf out = STRBUF_INIT;
     @@ t/helper/test-run-command.c: static int quote_stress_test(int argc, const char *
       		OPT_END()
       	};
       	const char * const usage[] = {
     +
     + ## t/t1502-rev-parse-parseopt.sh ##
     +@@ t/t1502-rev-parse-parseopt.sh: test_expect_success 'setup optionspec-only-hidden-switches' '
     + |
     + |some-command does foo and bar!
     + |--
     +-|hidden1* A hidden switch
     ++|hidden1* a hidden switch
     + EOF
     + '
     + 
     +@@ t/t1502-rev-parse-parseopt.sh: test_expect_success 'test --parseopt help-all output hidden switches' '
     + |
     + |    some-command does foo and bar!
     + |
     +-|    --hidden1             A hidden switch
     ++|    --hidden1             a hidden switch
     + |
     + |EOF
     + END_EXPECT


 builtin/bisect--helper.c      | 2 +-
 builtin/reflog.c              | 6 +++---
 builtin/submodule--helper.c   | 2 +-
 diff.c                        | 2 +-
 parse-options.c               | 9 +++++++++
 t/helper/test-run-command.c   | 6 +++---
 t/t1502-rev-parse-parseopt.sh | 4 ++--
 7 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 28a2e6a5750..614d95b022c 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -1209,7 +1209,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE(0, "bisect-visualize", &cmdmode,
 			 N_("visualize the bisection"), BISECT_VISUALIZE),
 		OPT_CMDMODE(0, "bisect-run", &cmdmode,
-			 N_("use <cmd>... to automatically bisect."), BISECT_RUN),
+			 N_("use <cmd>... to automatically bisect"), BISECT_RUN),
 		OPT_BOOL(0, "no-log", &nolog,
 			 N_("no log for BISECT_WRITE")),
 		OPT_END()
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 85b838720c3..28372c5e2b5 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -600,7 +600,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 		OPT_BIT(0, "updateref", &flags,
 			N_("update the reference to the value of the top reflog entry"),
 			EXPIRE_REFLOGS_UPDATE_REF),
-		OPT_BOOL(0, "verbose", &verbose, N_("print extra information on screen.")),
+		OPT_BOOL(0, "verbose", &verbose, N_("print extra information on screen")),
 		OPT_CALLBACK_F(0, "expire", &cmd, N_("timestamp"),
 			       N_("prune entries older than the specified time"),
 			       PARSE_OPT_NONEG,
@@ -613,7 +613,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 			 N_("prune any reflog entries that point to broken commits")),
 		OPT_BOOL(0, "all", &do_all, N_("process the reflogs of all references")),
 		OPT_BOOL(1, "single-worktree", &all_worktrees,
-			 N_("limits processing to reflogs from the current worktree only.")),
+			 N_("limits processing to reflogs from the current worktree only")),
 		OPT_END()
 	};
 
@@ -736,7 +736,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
 		OPT_BIT(0, "updateref", &flags,
 			N_("update the reference to the value of the top reflog entry"),
 			EXPIRE_REFLOGS_UPDATE_REF),
-		OPT_BOOL(0, "verbose", &verbose, N_("print extra information on screen.")),
+		OPT_BOOL(0, "verbose", &verbose, N_("print extra information on screen")),
 		OPT_END()
 	};
 
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 33c82c3ab91..6332d305983 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1875,7 +1875,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "depth", &clone_data.depth,
 			   N_("string"),
 			   N_("depth for shallow clones")),
-		OPT__QUIET(&quiet, "Suppress output for cloning a submodule"),
+		OPT__QUIET(&quiet, "suppress output for cloning a submodule"),
 		OPT_BOOL(0, "progress", &progress,
 			   N_("force cloning progress")),
 		OPT_BOOL(0, "require-init", &require_init,
diff --git a/diff.c b/diff.c
index 7d5cfd325ea..387435a4a45 100644
--- a/diff.c
+++ b/diff.c
@@ -5630,7 +5630,7 @@ static void prep_parse_options(struct diff_options *options)
 			       N_("select files by diff type"),
 			       PARSE_OPT_NONEG, diff_opt_diff_filter),
 		{ OPTION_CALLBACK, 0, "output", options, N_("<file>"),
-		  N_("Output to a specific file"),
+		  N_("output to a specific file"),
 		  PARSE_OPT_NONEG, NULL, 0, diff_opt_output },
 
 		OPT_END()
diff --git a/parse-options.c b/parse-options.c
index 2437ad3bcdd..91cbfb0d7f7 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -492,6 +492,15 @@ static void parse_options_check(const struct option *opts)
 		default:
 			; /* ok. (usually accepts an argument) */
 		}
+		if (opts->type != OPTION_GROUP && opts->help &&
+			!(starts_with(opts->help, "HEAD") ||
+			  starts_with(opts->help, "GPG") ||
+			  starts_with(opts->help, "DEPRECATED") ||
+			  starts_with(opts->help, "SHA1")) &&
+			  (opts->help[0] >= 65 && opts->help[0] <= 90))
+			err |= optbug(opts, xstrfmt("help should not start with capital letter unless needed: %s", opts->help));
+		if (opts->help && !ends_with(opts->help, "...") && ends_with(opts->help, "."))
+			err |= optbug(opts, xstrfmt("help should not end with a dot: %s", opts->help));
 		if (opts->argh &&
 		    strcspn(opts->argh, " _") != strlen(opts->argh))
 			err |= optbug(opts, "multi-word argh should use dash to separate words");
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 913775a14b7..8f370cd89f1 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -221,9 +221,9 @@ static int quote_stress_test(int argc, const char **argv)
 	struct strbuf out = STRBUF_INIT;
 	struct strvec args = STRVEC_INIT;
 	struct option options[] = {
-		OPT_INTEGER('n', "trials", &trials, "Number of trials"),
-		OPT_INTEGER('s', "skip", &skip, "Skip <n> trials"),
-		OPT_BOOL('m', "msys2", &msys2, "Test quoting for MSYS2's sh"),
+		OPT_INTEGER('n', "trials", &trials, "number of trials"),
+		OPT_INTEGER('s', "skip", &skip, "skip <n> trials"),
+		OPT_BOOL('m', "msys2", &msys2, "test quoting for MSYS2's sh"),
 		OPT_END()
 	};
 	const char * const usage[] = {
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 284fe18e726..2a07e130b96 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -53,7 +53,7 @@ test_expect_success 'setup optionspec-only-hidden-switches' '
 |
 |some-command does foo and bar!
 |--
-|hidden1* A hidden switch
+|hidden1* a hidden switch
 EOF
 '
 
@@ -131,7 +131,7 @@ test_expect_success 'test --parseopt help-all output hidden switches' '
 |
 |    some-command does foo and bar!
 |
-|    --hidden1             A hidden switch
+|    --hidden1             a hidden switch
 |
 |EOF
 END_EXPECT

base-commit: e6ebfd0e8cbbd10878070c8a356b5ad1b3ca464e
-- 
gitgitgadget