From: Abhra303 <chakrabortyabhradeep79@xxxxxxxxx>
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@xxxxxxxxx>
---
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@xxxxxxxxx>
## 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@xxxxxxxxx>
- ## 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
