The process of parsing command-line options in commands that perform
an early scan over their arguments has been unified using a new
early-scan sub-API. The 'git fast-import' command has been converted
to use this API, safely parsing and skipping known options taking
separate values to prevent logic bugs.
* cc/early-scan-options:
fast-import: use early_scan_options() for --allow-unsafe-features
parse-options: add early_scan_options()
parse-options: extract lookup_long_opt() from parse_long_opt()
parse-options: add parse_options_takes_argument()
Some commands need to look at a few of their options before they can
parse their command line for real, for example because the result
decides whether a repository is needed at all, or how the beginning of
their input should be interpreted.
Such an early scan has to know which options take their value as a
separate argument, or it mistakes such a value for an option. Several
commands get this wrong, as they just walk their arguments comparing
them to the few option names they care about.
Let's add early_scan_options() to help with this. It walks the
arguments using the very same `struct option` array that the command
already passes to parse_options(). It looks up long options with the
lookup_long_opt() helper extracted from parse_long_opt() in a previous
commit, so that negated forms, unambiguous abbreviations and aliases
are recognized the same way, and it uses the
parse_options_takes_argument() helper added in another previous
commit, so that the scan and the actual parsing agree on which options
take a value.
The options the caller wants to be told about are marked with a new
PARSE_OPT_EARLY flag, so that nothing has to be spelled out a second
time, and so that the mark cannot drift away from the option it refers
to.
Using a per-option flag for this is not new as that flag space already
holds flags that the parsing loop itself ignores, like
PARSE_OPT_NOCOMPLETE and PARSE_OPT_COMP_ARG, which only the completion
helper looks at, or PARSE_OPT_HIDDEN and PARSE_OPT_LITERAL_ARGHELP,
which only the usage output looks at.
The scan is deliberately kept much simpler than parse_options(),
instead of teaching the latter to perform a side effect free "dry
run". Such a dry run would have to avoid writing through `opt->value`,
calling option callbacks, dying on an invalid value, handling `--help`
and tracking command mode conflicts, so it would be a much larger
refactoring. If parse_options() learns to do it in the future though,
the commands converted now would keep both their option array and their
PARSE_OPT_EARLY marks, so their conversion would not have to be redone.
One consequence of staying simple is that the scan doesn't try to
handle what it cannot reliably skip, like short options, unknown
options, ambiguous abbreviations or options with a low-level callback,
which may consume any number of arguments. It stops at them instead,
so that it never takes an option's value for an option. This matters,
as for example with:
git fast-import --quiet --export-pack --allow-unsafe-features
parse_options() takes `--allow-unsafe-features` as the value of
`--export-pack-edges`, which `--export-pack` abbreviates. A scan that
just skipped `--export-pack` as an unknown option would wrongly see
`--allow-unsafe-features` there.
So the scan can fail to see an option that parse_options() would
accept, but it doesn't report an option that parse_options() would
not, and its callers can safely err on the side of not having seen an
option. The differences with parse_options() are documented in
"parse-options.h".
In practice, despite these limitations, early scans using
early_scan_options() should still be safer and cleaner than the
ad-hoc hand-rolled scans they are meant to replace, which don't know
about option values at all and therefore disagree with
parse_options() in ways that create plain bugs.
Helped-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `OPT_GROUP()` macro can be used to create a new group. These groups
can only be used to group options though, they do not have any effect
when used in combination with subcommands. As our use of subcommands
grows though it can be quite useful to group these, as well.
Extend `OPT_SUBCOMMAND_F()` to take an optional help string. If given,
such subcommands will be considered as part of `OPT_GROUP()` and printed
with that help string.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In "Documentation/technical/api-parse-options.adoc", the list of option
macros does not mention the `OPT_*_F()` macro variants that take a
trailing `flags` argument, nor the `OPT_HIDDEN_GROUP()` and
`OPT_HIDDEN_BOOL()` convenience macros.
Now that a previous commit documents the per-option flags, let's
document these macros too:
- Add a paragraph explaining the `OPT_*_F` convention and how it
relates to the per-option flags.
- Document `OPT_HIDDEN_GROUP()`, introduced in a previous commit,
right after `OPT_GROUP()`.
- Document `OPT_HIDDEN_BOOL()` right after `OPT_BOOL()`.
Signed-off-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "Flags" section in "Documentation/technical/api-parse-options.adoc"
documents the flags that can be passed to parse_options() itself. It
does not, however, document the flags that can be set on individual
options through the `flags` member of `struct option` (and through the
`OPT_*_F()` macro variants).
These per-option flags are used throughout the codebase (for example
`PARSE_OPT_HIDDEN` is used to hide an option from `-h` while still
showing it with `--help-all`), but a reader currently has to dig into
"parse-options.h" to find them.
To remediate that, let's add an "Option flags" subsection to the
"Data Structure" section, just before the list of option macros.
Let's also make it explicit that these are distinct from the
parse_options() flags described earlier, and let's describe the `-h`
versus `--help-all` behavior for `PARSE_OPT_HIDDEN`.
Signed-off-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
With the preceding commit, `OPT_INTEGER()` has learned to support unit
factors. Consequently, the major differencen between `OPT_INTEGER()` and
`OPT_MAGNITUDE()` isn't the support of unit factors anymore, as both of
them do support them now. Instead, the difference is that one handles
signed and the other handles unsigned integers.
Adapt the name of `OPT_MAGNITUDE()` accordingly by renaming it to
`OPT_UNSIGNED()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are two main differences between `OPT_INTEGER()` and
`OPT_MAGNITUDE()`:
- The former parses signed integers whereas the latter parses unsigned
integers.
- The latter parses unit factors like 'k', 'm' or 'g'.
While the first difference makes obvious sense, there isn't really a
good reason why signed integers shouldn't support unit factors, too.
This inconsistency will also become a bit of a problem with subsequent
commits, where we will fix a couple of callsites that pass an unsigned
integer to `OPT_INTEGER()`. There are three options:
- We could adapt those users to instead pass a signed integer, but
this would needlessly extend the range of accepted integer values.
- We could convert them to use `OPT_MAGNITUDE()`, as it only accepts
unsigned integers. But now we have the inconsistency that we also
start to accept unit factors.
- We could introduce `OPT_UNSIGNED()` as equivalent to `OPT_INTEGER()`
so that it knows to only accept unsigned integers without unit
suffix.
Introducing a whole new option type feels a bit excessive. There also
isn't really a good reason why `OPT_INTEGER()` cannot be extended to
also accept unit factors: all valid values passed to such options cannot
have a unit factors right now, so there wouldn't be any ambiguity.
Refactor `OPT_INTEGER()` to use `git_parse_int()`, which knows to
interpret unit factors. This removes the inconsistency between the
signed and unsigned options so that we can easily fix up callsites that
pass the wrong integer type right now.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We presently use the ".txt" extension for our AsciiDoc files. While not
wrong, most editors do not associate this extension with AsciiDoc,
meaning that contributors don't get automatic editor functionality that
could be useful, such as syntax highlighting and prose linting.
It is much more common to use the ".adoc" extension for AsciiDoc files,
since this helps editors automatically detect files and also allows
various forges to provide rich (HTML-like) rendering. Let's do that
here, renaming all of the files and updating the includes where
relevant. Adjust the various build scripts and makefiles to use the new
extension as well.
Note that this should not result in any user-visible changes to the
documentation.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano