Skip to content

Commit

Permalink
Merge branch 'rs/no-no-no-parseopt'
Browse files Browse the repository at this point in the history
* rs/no-no-no-parseopt:
  parse-options: remove PARSE_OPT_NEGHELP
  parse-options: allow positivation of options starting, with no-
  test-parse-options: convert to OPT_BOOL()

Conflicts:
	builtin/grep.c
  • Loading branch information
Junio C Hamano committed Mar 2, 2012
2 parents 260eb7e + cbb08c2 commit 66b8800
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 32 deletions.
19 changes: 18 additions & 1 deletion Documentation/RelNotes/1.7.10.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ Updates since v1.7.9

UI, Workflows & Features

* Teams for localizing the messages from the Porcelain layer of
commands are starting to form, thanks to Jiang Xin who volunteered
to be the localization coordinator. An initial set of translated
messages for simplified chinese is available.

* Improved handling of views, labels and branches in git-p4 (in contrib).

* "git-p4" (in contrib) suffered from unnecessary merge conflicts when
Expand Down Expand Up @@ -98,6 +103,18 @@ Unless otherwise noted, all the fixes since v1.7.9 in the maintenance
releases are contained in this release (see release notes to them for
details).

* "git branch --with $that" assumed incorrectly that the user will never
ask the question with nonsense value in $that.
(merge 6c41e97 cn/maint-branch-with-bad later to maint).

* An invalid regular expression pattern given by an end user made
"gitweb" to return garbled response.
(merge 36612e4 jn/maint-gitweb-invalid-regexp later to maint).

* "git rev-list --verify-objects -q" omitted the extra verification
it needs to do over "git rev-list --objects -q" by mistake.
(merge 9899372 nd/maint-verify-objects later to maint).

* The bulk check-in codepath streamed contents that needs
smudge/clean filters without running them, instead of punting and
delegating to the codepath to run filters after slurping everything
Expand Down Expand Up @@ -126,7 +143,7 @@ details).

---
exec >/var/tmp/1
O=v1.7.9.2-322-g472fdee
O=v1.7.9.2-347-gbfabdfe
echo O=$(git describe)
git log --first-parent --oneline ^maint $O..
echo
Expand Down
3 changes: 2 additions & 1 deletion Documentation/technical/api-parse-options.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ The parse-options API allows:
* Short options may be bundled, e.g. `-a -b` can be specified as `-ab`.

* Boolean long options can be 'negated' (or 'unset') by prepending
`no-`, e.g. `\--no-abbrev` instead of `\--abbrev`.
`no-`, e.g. `\--no-abbrev` instead of `\--abbrev`. Conversely,
options that begin with `no-` can be 'negated' by removing it.

* Options and non-option arguments can clearly be separated using the `\--`
option, e.g. `-a -b \--option \-- \--this-is-a-file` indicates that
Expand Down
4 changes: 1 addition & 3 deletions builtin/fast-export.c
Original file line number Diff line number Diff line change
Expand Up @@ -647,9 +647,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
"Output full tree for each commit"),
OPT_BOOLEAN(0, "use-done-feature", &use_done_feature,
"Use the done feature to terminate the stream"),
{ OPTION_NEGBIT, 0, "data", &no_data, NULL,
"Skip output of blob data",
PARSE_OPT_NOARG | PARSE_OPT_NEGHELP, NULL, 1 },
OPT_BOOL(0, "no-data", &no_data, "Skip output of blob data"),
OPT_END()
};

Expand Down
5 changes: 2 additions & 3 deletions builtin/grep.c
Original file line number Diff line number Diff line change
Expand Up @@ -684,9 +684,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
struct option options[] = {
OPT_BOOLEAN(0, "cached", &cached,
"search in index instead of in the work tree"),
{ OPTION_BOOLEAN, 0, "index", &use_index, NULL,
"finds in contents not managed by git",
PARSE_OPT_NOARG | PARSE_OPT_NEGHELP },
OPT_NEGBIT(0, "no-index", &use_index,
"finds in contents not managed by git", 1),
OPT_BOOLEAN(0, "untracked", &untracked,
"search in both tracked and untracked files"),
OPT_SET_INT(0, "exclude-standard", &opt_exclude,
Expand Down
33 changes: 19 additions & 14 deletions parse-options.c
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,14 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
arg_end = arg + strlen(arg);

for (; options->type != OPTION_END; options++) {
const char *rest;
int flags = 0;
const char *rest, *long_name = options->long_name;
int flags = 0, opt_flags = 0;

if (!options->long_name)
if (!long_name)
continue;

rest = skip_prefix(arg, options->long_name);
again:
rest = skip_prefix(arg, long_name);
if (options->type == OPTION_ARGUMENT) {
if (!rest)
continue;
Expand All @@ -212,7 +213,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
}
if (!rest) {
/* abbreviated? */
if (!strncmp(options->long_name, arg, arg_end - arg)) {
if (!strncmp(long_name, arg, arg_end - arg)) {
is_abbreviated:
if (abbrev_option) {
/*
Expand All @@ -227,7 +228,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
if (!(flags & OPT_UNSET) && *arg_end)
p->opt = arg_end + 1;
abbrev_option = options;
abbrev_flags = flags;
abbrev_flags = flags ^ opt_flags;
continue;
}
/* negation allowed? */
Expand All @@ -239,12 +240,18 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
goto is_abbreviated;
}
/* negated? */
if (strncmp(arg, "no-", 3))
if (prefixcmp(arg, "no-")) {
if (!prefixcmp(long_name, "no-")) {
long_name += 3;
opt_flags |= OPT_UNSET;
goto again;
}
continue;
}
flags |= OPT_UNSET;
rest = skip_prefix(arg + 3, options->long_name);
rest = skip_prefix(arg + 3, long_name);
/* abbreviated and negated? */
if (!rest && !prefixcmp(options->long_name, arg + 3))
if (!rest && !prefixcmp(long_name, arg + 3))
goto is_abbreviated;
if (!rest)
continue;
Expand All @@ -254,7 +261,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
continue;
p->opt = rest + 1;
}
return get_value(p, options, flags);
return get_value(p, options, flags ^ opt_flags);
}

if (ambiguous_option)
Expand Down Expand Up @@ -526,7 +533,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
continue;

pos = fprintf(outfile, " ");
if (opts->short_name && !(opts->flags & PARSE_OPT_NEGHELP)) {
if (opts->short_name) {
if (opts->flags & PARSE_OPT_NODASH)
pos += fprintf(outfile, "%c", opts->short_name);
else
Expand All @@ -535,9 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
if (opts->long_name && opts->short_name)
pos += fprintf(outfile, ", ");
if (opts->long_name)
pos += fprintf(outfile, "--%s%s",
(opts->flags & PARSE_OPT_NEGHELP) ? "no-" : "",
opts->long_name);
pos += fprintf(outfile, "--%s", opts->long_name);
if (opts->type == OPTION_NUMBER)
pos += fprintf(outfile, "-NUM");

Expand Down
4 changes: 0 additions & 4 deletions parse-options.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ enum parse_opt_option_flags {
PARSE_OPT_LASTARG_DEFAULT = 16,
PARSE_OPT_NODASH = 32,
PARSE_OPT_LITERAL_ARGHELP = 64,
PARSE_OPT_NEGHELP = 128,
PARSE_OPT_SHELL_EVAL = 256
};

Expand Down Expand Up @@ -90,9 +89,6 @@ typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
* PARSE_OPT_LITERAL_ARGHELP: says that argh shouldn't be enclosed in brackets
* (i.e. '<argh>') in the help message.
* Useful for options with multiple parameters.
* PARSE_OPT_NEGHELP: says that the long option should always be shown with
* the --no prefix in the usage message. Sometimes
* useful for users of OPTION_NEGBIT.
*
* `callback`::
* pointer to the callback to use for OPTION_CALLBACK or
Expand Down
60 changes: 58 additions & 2 deletions t/t0040-parse-options.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ test_description='our own option parser'
cat > expect << EOF
usage: test-parse-options <options>
-b, --boolean get a boolean
--yes get a boolean
-D, --no-doubt begins with 'no-'
-B, --no-fear be brave
-b, --boolean increment by one
-4, --or4 bitwise-or boolean with ...0100
--neg-or4 same as --no-or4
Expand Down Expand Up @@ -53,6 +56,59 @@ test_expect_success 'test help' '

mv expect expect.err

cat >expect.template <<EOF
boolean: 0
integer: 0
timestamp: 0
string: (not set)
abbrev: 7
verbose: 0
quiet: no
dry run: no
file: (not set)
EOF

check() {
what="$1" &&
shift &&
expect="$1" &&
shift &&
sed "s/^$what .*/$what $expect/" <expect.template >expect &&
test-parse-options $* >output 2>output.err &&
test ! -s output.err &&
test_cmp expect output
}

check_unknown() {
case "$1" in
--*)
echo error: unknown option \`${1#--}\' >expect ;;
-*)
echo error: unknown switch \`${1#-}\' >expect ;;
esac &&
cat expect.err >>expect &&
test_must_fail test-parse-options $* >output 2>output.err &&
test ! -s output &&
test_cmp expect output.err
}

test_expect_success 'OPT_BOOL() #1' 'check boolean: 1 --yes'
test_expect_success 'OPT_BOOL() #2' 'check boolean: 1 --no-doubt'
test_expect_success 'OPT_BOOL() #3' 'check boolean: 1 -D'
test_expect_success 'OPT_BOOL() #4' 'check boolean: 1 --no-fear'
test_expect_success 'OPT_BOOL() #5' 'check boolean: 1 -B'

test_expect_success 'OPT_BOOL() is idempotent #1' 'check boolean: 1 --yes --yes'
test_expect_success 'OPT_BOOL() is idempotent #2' 'check boolean: 1 -DB'

test_expect_success 'OPT_BOOL() negation #1' 'check boolean: 0 -D --no-yes'
test_expect_success 'OPT_BOOL() negation #2' 'check boolean: 0 -D --no-no-doubt'

test_expect_success 'OPT_BOOL() no negation #1' 'check_unknown --fear'
test_expect_success 'OPT_BOOL() no negation #2' 'check_unknown --no-no-fear'

test_expect_success 'OPT_BOOL() positivation' 'check boolean: 0 -D --doubt'

cat > expect << EOF
boolean: 2
integer: 1729
Expand Down Expand Up @@ -296,7 +352,7 @@ test_expect_success 'OPT_NEGBIT() works' '
test_cmp expect output
'

test_expect_success 'OPT_BOOLEAN() with PARSE_OPT_NODASH works' '
test_expect_success 'OPT_COUNTUP() with PARSE_OPT_NODASH works' '
test-parse-options + + + + + + > output 2> output.err &&
test ! -s output.err &&
test_cmp expect output
Expand Down
12 changes: 8 additions & 4 deletions test-parse-options.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ int main(int argc, const char **argv)
NULL
};
struct option options[] = {
OPT_BOOLEAN('b', "boolean", &boolean, "get a boolean"),
OPT_BOOL(0, "yes", &boolean, "get a boolean"),
OPT_BOOL('D', "no-doubt", &boolean, "begins with 'no-'"),
{ OPTION_SET_INT, 'B', "no-fear", &boolean, NULL,
"be brave", PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
OPT_COUNTUP('b', "boolean", &boolean, "increment by one"),
OPT_BIT('4', "or4", &boolean,
"bitwise-or boolean with ...0100", 4),
OPT_NEGBIT(0, "neg-or4", &boolean, "same as --no-or4", 4),
Expand All @@ -62,11 +66,11 @@ int main(int argc, const char **argv)
OPT_ARGUMENT("quux", "means --quux"),
OPT_NUMBER_CALLBACK(&integer, "set integer to NUM",
number_callback),
{ OPTION_BOOLEAN, '+', NULL, &boolean, NULL, "same as -b",
{ OPTION_COUNTUP, '+', NULL, &boolean, NULL, "same as -b",
PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH },
{ OPTION_BOOLEAN, 0, "ambiguous", &ambiguous, NULL,
{ OPTION_COUNTUP, 0, "ambiguous", &ambiguous, NULL,
"positive ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG },
{ OPTION_BOOLEAN, 0, "no-ambiguous", &ambiguous, NULL,
{ OPTION_COUNTUP, 0, "no-ambiguous", &ambiguous, NULL,
"negative ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG },
OPT_GROUP("Standard options"),
OPT__ABBREV(&abbrev),
Expand Down

0 comments on commit 66b8800

Please sign in to comment.