The process of parsing command-line options in commands that
perform an early scan over their arguments (such as 'git bisect',
'git rev-parse', and 'git fast-import') has been unified using a
new early-scan sub-API, which parses and skips 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: build early scan options from a struct option array
parse-options: add parse_options_takes_argument()
rev-parse: fix "--" detection when it is an option value
bisect: fix "--" detection when a term name is "--"
parse-options: add early_scan_options()
The `OPT_ALIAS()` option can be used to create an exact alias that maps
one option name to the same semantics as another option name. This
option type is especially useful when deprecating an old name in favor
of a new one. But curiously enough, we don't have the infrastructure in
place to properly support this use case because we don't expose the
ability to hide the alias via `PARSE_OPT_HIDDEN`.
Introduce a new `OPT_ALIAS_F()` function that allows the user to pass
flags and propagate these flags when rewriting aliases to match their
respective source options.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A command that scans its arguments early has to know which options take
a value, so that it can skip that value instead of mistaking it for an
option. When it also parses its options with the parse-options API, that
information is already available in its `struct option` array, and
duplicating it by hand in a `struct early_scan_option` array is both
tedious and easy to get out of sync when an option is added.
So let's add early_scan_options_from_options() to build the latter array
from the former, using parse_options_takes_argument() to find out which
options take a value. Its caller only has to name the options it wants
to be reported.
Note: This early scanner translation intentionally leaves out a few
complex option types to keep the scan simple and fast:
- Short options are ignored: early_scan_options_from_options()
explicitly skips options without a `long_name`, and the scanner only
looks for `--`. Properly handling short options would require parsing
bundled flags (e.g., `-abc value`), which requires replicating the
full parse_options() state machine.
- Conditional values: Options with `PARSE_OPT_LASTARG_DEFAULT` or
`PARSE_OPT_OPTARG` are treated as not taking a separate argument.
Because the scanner does not evaluate context (like whether an
argument is the final one in `argv`), it must err on the side of
caution to avoid accidentally consuming the `--` separator or a path.
- Abbreviated options remain unrecognized: Even though the scanner is
now provided with the full option array, the underlying
early_scan_options() engine still relies on exact string matches.
Safely resolving abbreviations would require duplicating the
ambiguity-checking logic from the main parser.
- Negated options are not automatically derived: The scanner strictly
matches the defined long name. It does not automatically recognize
the `--no-<name>` variants of boolean options. (This is harmless in
practice for current callers, as negated options do not take values
to skip, and boolean defaults align with the ignored state).
The above shortcomings can be addressed later, for example, when
commands that use short options or options with conditional values need
an early scan or are ported to use `struct option`.
Despite these limitations, this abstraction is a significant
improvement. It allows commands like `fast-import` to reuse their
existing `struct option` array for early scanning, ensuring the scanner
and the main parser agree on which options take arguments, and
preventing developers from having to maintain a separate, hardcoded
list that could drift out of sync.
Signed-off-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Whether an option takes a value, and therefore consumes the next
argument when that value is not stuck to it with an '=', is decided by
its type and its flags. That rule is currently open-coded in
show_gitcomp(), which needs it to decide if it should append an '=' to
the option it completes.
A following commit will need the same rule to find out which options an
early scan of the command line has to skip along with their value.
So let's factor that rule out into a new parse_options_takes_argument()
function, and let's use it in show_gitcomp().
Note that an option with PARSE_OPT_LASTARG_DEFAULT only consumes the
next argument when it isn't the last one, so it is not considered as
taking a value, which is what show_gitcomp() already did.
Signed-off-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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. Its callers describe
the options to look for, but also the ones that merely have to be
skipped along with their value, so that the scan can walk the arguments
without being fooled by option values.
Note that abbreviated options are deliberately not recognized, as a
scan cannot know about the options it hasn't been told about, and would
then resolve abbreviations differently from the actual option parsing.
So users must spell these specific options in full. This restriction
could be lifted in the future though, once the scanner is adapted to
accept a command's full option array, as this would give it the
complete context needed for safe abbreviation matching.
Signed-off-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Hidden options are not shown by `git <cmd> -h`, but are still shown by
`git <cmd> --help-all`. If there are a lot of hidden options or if they
don't belong to the same categories as other options, there is
currently no way to properly group them.
Using `OPT_GROUP("Foo")` means that "Foo" will always be shown which we
don't want if that group contains only hidden options.
To provide a way to have groups shown only when hidden options are
shown, let's implement an OPT_HIDDEN_GROUP macro.
To test this new macro, let's also improve `test-tool parse-options`
and test its output with `--help-all`.
Signed-off-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The standard philosophy for Unix software when a help option (such as
--help) is specified is that the software should exit 0, printing the
help output to standard output, since the standard output is for
user-requested output and the program performed the requested task
successfully. If the user specifies an incorrect option, then the help
output should be printed to standard error (since the user has made a
mistake) and it should exit unsuccessfully.
Most of our commands currently exit 129 on receiving the -h option to
print the short help, which does not line up with the standard
philosophy above. Let's change that to exit 0 instead.
This requires changes to a variety of tests which previously wanted the
129 exit code, so update them. Note that because git diff does its own
option parsing, it still exits with 129, so update some of the tests to
expect either exit status.
Some commands also now pass with -h but not --help-all, so handle those
cases differently for those commands.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The standard philosophy for Unix software when a help option (such as
--help) is specified is that the software should exit 0, printing the
help output to standard output, since the standard output is for
user-requested output and the program performed the requested task
successfully. If the user specifies an incorrect option, then the help
output should be printed to standard error (since the user has made a
mistake) and it should exit unsuccessfully.
git rev-parse --parseopt properly directs the output in both of these
cases, but it currently exits 129 when it receives a --help or -h option
on the command line, which causes its invoking script to do the same.
This is not in line with the usual behavior and it causes scripts using
this command to exit unsuccessfully on --help as well.
Note that Git subcommands implemented using scripts, such as git
submodule, don't have this problem because Git itself intercepts the
--help option and runs man (or a similar tool), which then exits 0.
However, this still affects the myriad scripts that use this
functionality because Git is widespread and the --parseopt functionality
is a good way to get sensible option parsing across shells in a portable
way.
Because git rev-parse --parseopt is intended to be eval'd by the shell,
when help output is to be printed to standard output, Git actually
prints a cat command with a heredoc since the standard output is being
evaluated by the shell. Thus, to do the right thing, simply add an
"exit 0" right after the end of the heredoc, which will cause the
invoking program to exit successfully.
The usual invocation recommended by the manual page is this:
eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
Thus, the fact that git rev-parse --parseopt still exits 129 in this
case is irrelevant, since the "echo exit $?" will print "exit 129", but
that will be after the "exit 0" printed by Git—and thus ignored, since
the shell will have already exited successfully.
Update the tests for this case. Note that we no longer need to delete
only the first and last lines in some tests, so add a command to delete
the end of the heredoc as well. We could do something clever with sed
to delete all but the last two lines or switch to head and tail, but
those would be more complicated and less readable, so just stick with
the simple approach.
In t1517, add three shell scripts to the failure case because they no
longer return 129 as expected. In a future commit, we'll change the
expected result to exit 0 and these will become successful again.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we parse a command line option such as -h or --help, we currently
exit 129, since that is the exit code when help output is printed. In a
future commit, we'll change this to exit 0 instead, since we're doing
what the user wanted successfully.
However, there are some cases where we print help output because the
user has provided ambiguous or invalid input, such as an ambiguous
option, and we'll want to exit unsuccessfully there. Make this easier
by defining a new return code, PARSE_OPT_HELP_ERROR, that can be used in
this case, while reserving PARSE_OPT_HELP for those cases where the user
has requested help directly.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix several spelling mistakes, subject-verb agreement issues, and
duplicated words.
Signed-off-by: Weijie Yuan <wy@wyuan.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The parse-options API learned to notice an options[] array with
duplicated long options.
* rs/parse-options-duplicated-long-options:
parseopt: check for duplicate long names and numerical options
pack-objects: remove duplicate --stdin-packs definition
We already check for duplicate short names. Check for and report
duplicate long names and numerical options as well.
Perform the slightly expensive string duplicate check only when showing
the usage to keep the cost of normal invocations low. t0012-help.sh
covers it.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A recently added configuration variable and command line option
syntax ":(optional)" for values that are of filename type
inconsistently behaved on an empty file (configuration took it
happily, while the command line option pretended as if it did not
exist), which has been corrected.
* dk/parseopt-optional-filename-fixes:
parseopt: remove unreachable code
parseopt: restore const qualifier to parsed filename
config: use boolean type for a simple flag
parseopt: use boolean type for a simple flag
doc: clarify command equivalence comment
parseopt: fix :(optional) at command line to only ignore missing files
At this point in the code after running skip_prefix() on the
variable and receiving the result in the same variable, the contents
of the variable can never be NULL. The function either (1) updates
the variable to point at a later part of the string it originally
pointed at, or (2) leaves it intact if the string does not have the
prefix. (1) will never make the variable NULL, and (2) cannot be
the source of NULL, because the variable cannot be NULL before
calling skip_prefix(), which would die immediately by dereferencing
the NULL pointer in that case.
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This was unintentionally dropped in ccfcaf399f (parseopt: values of
pathname type can be prefixed with :(optional), 2025-09-28). Notably,
continue dropping the const qualifier when free'ing value; see
4049b9cfc0 (fix const issues with some functions, 2007-10-16) or
83838d5c1b (cast variable in call to free() in builtin/diff.c and
submodule.c, 2011-11-06) for more details on why.
Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Unlike the configuration option magic, the parseopt code also ignores
empty files: compare implementations from ccfcaf399f (parseopt: values
of pathname type can be prefixed with :(optional), 2025-09-28) and
749d6d166d (config: values of pathname type can be prefixed with
:(optional), 2025-09-28).
Unify the 2 by not ignoring empty files, which is less surprising and
the intended semantics from the first patch for config.
Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Configuration variables that take a pathname as a value
(e.g. blame.ignorerevsfile) can be marked as optional by prefixing
":(optoinal)" before its value.
* jc/optional-path:
parseopt: values of pathname type can be prefixed with :(optional)
config: values of pathname type can be prefixed with :(optional)
t7500: fix GIT_EDITOR shell snippet
t7500: make each piece more independent
In the previous step, we introduced an optional filename that can be
given to a configuration variable, and nullify the fact that such a
configuration setting even existed if the named path is missing or
empty.
Let's do the same for command line options that name a pathname.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git cmd --help-all" now works outside repositories.
* dk/help-all:
builtin: also setup gently for --help-all
parse-options: refactor flags for usage_with_options_internal
Git experts often check the help summary of a command to make sure they
spell options right when suggesting advice to colleagues. Further, they
might check hidden options when responding to queries about deprecated
options like git-rebase(1)'s "preserve merges" option. But some commands
don't support "--help-all" outside of a git directory. Running (for
example)
git rebase --help-all
outside a directory fails in "setup_git_directory", erroring with the
localized form of
fatal: not a git repository (or any of the parent directories): .git
Like 99caeed05d (Let 'git <command> -h' show usage without a git dir,
2009-11-09), we want to show the "--help-all" output even without a git
dir. Make "--help-all" where we expect "-h" to mean
"setup_git_directory_gently", and interpose early in the natural place
("show_usage_with_options_if_asked").
Do the same for usage callers with show_usage_if_asked.
The exception is merge-recursive, whose help block doesn't use newer
APIs.
Best-viewed-with: --ignore-space-change
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When reading or editing calls to usage_with_options_internal, it is
difficult to tell what trailing "0, 0", "0, 1", "1, 0" arguments mean
(NB there is never a "1, 1" case).
Give the flags readable names to improve call-sites without changing any
behavior.
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The string_list_split_in_place() function was updated by 52acddf3
(string-list: multi-delimiter `string_list_split_in_place()`,
2023-04-24) to take more than one delimiter characters, hoping that
we can later use it to replace our uses of strtok(). We however did
not make a matching change to the string_list_split() function,
which is very similar.
Before giving both functions more features in future commits, allow
string_list_split() to also take more than one delimiter characters
to make them closer to each other.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar to 09705696f7 (parse-options: introduce precision handling for
`OPTION_INTEGER`, 2025-04-17) support value variables of different sizes
for OPTION_COUNTUP. Do that by requiring their "precision" to be set,
casting their "value" pointer accordingly and checking whether the value
fits.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar to 09705696f7 (parse-options: introduce precision handling for
`OPTION_INTEGER`, 2025-04-17) support value variables of different sizes
for OPTION_BITOP. Do that by requiring their "precision" to be set,
casting their "value" pointer accordingly and checking whether the value
fits.
Check if "devfal" fits into an integer variable with the given
"precision", but don't check "extra", as its value is only used to clear
bits, so cannot lead to an overflow. Not checking continues to allow
e.g., using -1 to clear all bits even if the value variable has a
narrower type than intptr_t.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar to 09705696f7 (parse-options: introduce precision handling for
`OPTION_INTEGER`, 2025-04-17) support value variables of different sizes
for OPTION_NEGBIT. Do that by requiring their "precision" to be set,
casting their "value" pointer accordingly and checking whether the value
fits.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar to 09705696f7 (parse-options: introduce precision handling for
`OPTION_INTEGER`, 2025-04-17) support value variables of different sizes
for OPTION_BIT. Do that by requiring their "precision" to be set,
casting their "value" pointer accordingly and checking whether the value
fits.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar to 09705696f7 (parse-options: introduce precision handling for
`OPTION_INTEGER`, 2025-04-17) support value variables of different sizes
for OPTION_SET_INT. Do that by requiring their "precision" to be set,
casting their "value" pointer accordingly and checking whether the value
fits.
Factor out the casting code from the part of do_get_value() that handles
OPTION_INTEGER to avoid code duplication. We're going to use it in the
next patches as well.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Build on 09705696f7 (parse-options: introduce precision handling for
`OPTION_INTEGER`, 2025-04-17) to support value variables of different
sizes for PARSE_OPT_CMDMODE options. Do that by requiring their
"precision" to be set and casting their "value" pointer accordingly.
Call the function that does the raw casting do_get_int_value() to
reserve the name get_int_value() for a more friendly wrapper we're
going to introduce in one of the next patches.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
OPTION_BITOP options don't take arguments. Make sure they are declared
that way using the flag PARSE_OPT_NOARG.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is the equivalent to the preceding commit, but instead of
introducing precision handling for `OPTION_INTEGER` we introduce it for
`OPTION_UNSIGNED`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `OPTION_INTEGER` option type accepts a signed integer. The type of
the underlying integer is a simple `int`, which restricts the range of
values accepted by such options. But there is a catch: because the
caller provides a pointer to the value via the `.value` field, which is
a simple void pointer. This has two consequences:
- There is no check whether the passed value is sufficiently long to
store the entire range of `int`. This can lead to integer wraparound
in the best case and out-of-bounds writes in the worst case.
- Even when a caller knows that they want to store a value larger than
`INT_MAX` they don't have a way to do so.
In practice this doesn't tend to be a huge issue because users typically
don't end up passing huge values to most commands. But the parsing logic
is demonstrably broken, and it is too easy to get the calling convention
wrong.
Improve the situation by introducing a new `precision` field into the
structure. This field gets assigned automatically by `OPT_INTEGER_F()`
and tracks the size of the passed value. Like this it becomes possible
for the caller to pass arbitrarily-sized integers and the underlying
logic knows to handle it correctly by doing range checks. Furthermore,
convert the code to use `strtoimax()` intstead of `strtol()` so that we
can also parse values larger than `LONG_MAX`.
Note that we do not yet assert signedness of the passed variable, which
is another source of bugs. This will be handled in a subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
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>
The help text from "git $cmd -h" appear on the standard output for
some $cmd and the standard error for others. The built-in commands
have been fixed to show them on the standard output consistently.
* jc/show-usage-help:
builtin: send usage() help text to standard output
oddballs: send usage() help text to standard output
builtins: send usage_with_options() help text to standard output
usage: add show_usage_if_asked()
parse-options: add show_usage_with_options_if_asked()
t0012: optionally check that "-h" output goes to stdout
Many commands call usage_with_options() when they are asked to give
the help message, but it sends the help text to the standard error
stream. When the user asked for it with "git cmd -h", the help
message is the primary output from the command, hence we should send
it to the standard output stream, instead.
Introduce a helper function that captures the common pattern
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(usage, options);
and replaces it with
show_usage_with_options_if_asked(argc, argv, usage, options);
to help correct code paths.
Note that this helper function still exits with status 129, and
t0012 insists on it. After converting all the mistaken callers of
usage_with_options() to call this new helper, we may want to address
it---the end user is asking us to give the help text, and we are
doing exactly as asked, so there is no reason to exit with non-zero
status.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
i18n: expose substitution hint chars in functions and macros to
translators
For example (based on builtin/commit.c and shortened): the "--author"
option takes a name. In source this can be represented as:
OPT_STRING(0, "author", &force_author, N_("author"), N_("override author")),
When the command is run with "-h" (short help) option (git commit -h),
the above definition is displayed as:
--[no-]author <author> override author
Git does not use translated option names so the first part of the
above, "--[no-]author", is given as-is (it is based on the 2nd
argument of OPT_STRING). However the string "author" in the pair of
"<>", and the explanation "override author for commit" may be
translated into user's language.
The user's language may use a convention to mark a replaceable part of
the command line (called a "placeholder string") differently from
enclosing it inside a pair of "<>", but the implementation in
parse-options.c hardcodes "<%s>".
Allow translators to specify the presentation of a placeholder string
for their languages by overriding the "<%s>".
In case the translator's writing system is sufficiently different than
Latin the "<>" characters can be substituted by an empty string thus
effectively skipping them in the output. For example languages with
uppercase versions of characters can use that to deliniate
replaceability.
Alternatively a translator can decide to use characters that are
visually close to "<>" but are not interpreted by the shell.
Signed-off-by: Alexander Shopov <ash@kambanaria.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `OPTION_FILENAME` option always assigns either an allocated string
or `NULL` to the value. In case it is passed multiple times it does not
know to free the previous value though, which causes a memory leak.
Refactor the function to always free the previous value. None of the
sites where this option is used pass a string constant, so this change
is safe.
While at it, fix the argument of `fix_filename()` to be a string
constant. The only reason why it's not is because we use it as an
in-out-parameter, where the input is a constant and the output is not.
This is weird and unnecessary, as we can just return the result instead
of using the parameter for this.
This leak is being hit in t7621, but plugging it alone does not make the
test suite pass.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the code for handling a full match of long_name first and get rid
of negations. Reduce the indent of the code for matching abbreviations
and remove unnecessary curly braces. Combine the checks for whether
negation is allowed and whether arg is "n", "no" or "no-" because they
belong together and avoid a continue statement. The result is shorter,
more readable code.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Strip "no-" from arg and long_name before comparing them. This way we
no longer have to repeat the comparison with an offset of 3 for negated
arguments.
Note that we must not modify the "flags" value, which tracks whether arg
is negated, inside the loop. When registering "--n", "--no" or "--no-"
as abbreviation for any negative option, we used to OR it with OPT_UNSET
and end the loop. We can simply hard-code OPT_UNSET and leave flags
unchanged instead.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Git currently does not detect the ambiguity of an option that starts
with "no" like --notes and its negated form if given just --n or --no.
All Git commands with such options have other negatable options, and
we detect the ambiguity with them, so that's currently only a potential
problem for scripts that use git rev-parse --parseopt.
Let's fix it nevertheless, as there's no need for that confusion. To
detect the ambiguity we have to loosen the check in register_abbrev(),
as an option is considered an alias of itself. Add non-matching
negation flags as a criterion to recognize an option being ambiguous
with its negated form.
And we need to keep going after finding a non-negated option as an
abbreviated candidate and perform the negation checks in the same
loop.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a function, register_abbrev(), for storing the necessary details for
remembering an abbreviated and thus potentially ambiguous option. Call
it instead of sharing the code using goto, to make the control flow more
explicit.
Conveniently collect these details in the new struct parsed_option to
reduce the number of necessary function arguments.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Postpone setting the opt pointer until we're about to call get_value(),
which uses it. There's no point in setting it eagerly for every
abbreviated candidate option, which may turn out to be ambiguous.
Removing this assignment from the loop doesn't noticeably improve the
performance, but allows further simplification.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Giving an argument to an option that doesn't take one causes Git to
report that error specifically:
$ git rm --dry-run=bogus
error: option `dry-run' takes no value
The same is true when the option is negated or abbreviated:
$ git rm --no-dry-run=bogus
error: option `no-dry-run' takes no value
$ git rm --dry=bogus
error: option `dry-run' takes no value
Not so when doing both, though:
$ git rm --no-dry=bogus
error: unknown option `no-dry=bogus'
usage: git rm [-f | --force] [-n] [-r] [--cached] [--ignore-unmatch]
(Rest of the usage message omitted.)
Improve consistency and usefulness of the error message by recognizing
abbreviated negated options even if they have a (most likely bogus)
argument. With this patch we get:
$ git rm --no-dry=bogus
error: option `no-dry-run' takes no value
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git diff --no-rename A B" did not disable rename detection but did
not trigger an error from the command line parser.
* rs/parse-options-with-keep-unknown-abbrev-fix:
parse-options: simplify positivation handling
parse-options: fully disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN
We accept the positive version of options whose long name starts with
"no-" and are defined without the flag PARSE_OPT_NONEG. E.g. git clone
has an explicitly defined --no-checkout option and also implicitly
accepts --checkout to override it.
parse_long_opt() handles that by restarting the option matching with the
positive version when it finds that only the current option definition
starts with "no-", but not the user-supplied argument. This code is
located almost at the end of the matching logic.
Avoid the need for a restart by moving the code up. We don't have to
check the positive arg against the negative long_name at all -- the
"no-" prefix of the latter makes a match impossible. Skip it and toggle
OPT_UNSET right away to simplify the control flow.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
baa4adc66a (parse-options: disable option abbreviation with
PARSE_OPT_KEEP_UNKNOWN, 2019-01-27) turned off support for abbreviated
options when the flag PARSE_OPT_KEEP_UNKNOWN is given, as any shortened
option could also be an abbreviation for one of the unknown options.
The code for handling abbreviated options is guarded by an if, but it
can also be reached via goto. baa4adc66a only blocked the first way.
Add the condition to the other ones as well.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove unused header "#include".
* en/header-cleanup:
treewide: remove unnecessary includes in source files
treewide: add direct includes currently only pulled in transitively
trace2/tr2_tls.h: remove unnecessary include
submodule-config.h: remove unnecessary include
pkt-line.h: remove unnecessary include
line-log.h: remove unnecessary include
http.h: remove unnecessary include
fsmonitor--daemon.h: remove unnecessary includes
blame.h: remove unnecessary includes
archive.h: remove unnecessary include
treewide: remove unnecessary includes in source files
treewide: remove unnecessary includes from header files
Each of these were checked with
gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE}
to ensure that removing the direct inclusion of the header actually
resulted in that header no longer being included at all (i.e. that
no other header pulled it in transitively).
...except for a few cases where we verified that although the header
was brought in transitively, nothing from it was directly used in
that source file. These cases were:
* builtin/credential-cache.c
* builtin/pull.c
* builtin/send-pack.c
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git $cmd --end-of-options --rev -- --path" for some $cmd failed
to interpret "--rev" as a rev, and "--path" as a path. This was
fixed for many programs like "reset" and "checkout".
* jk/end-of-options:
parse-options: decouple "--end-of-options" and "--"
Junio C Hamano