summaryrefslogtreecommitdiffstats
path: root/parse-options.c (follow)
Commit message (Collapse)AuthorAgeFilesLines
* parse-options: rearrange long_name matching codeRené Scharfe2024-03-031-22/+15
| | | | | | | | | | | | 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>
* parse-options: normalize arg and long_name before comparisonRené Scharfe2024-03-031-22/+22
| | | | | | | | | | | | | | | 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>
* parse-options: detect ambiguous self-negationRené Scharfe2024-03-031-2/+1
| | | | | | | | | | | | | | | | | | | | | 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>
* parse-options: factor out register_abbrev() and struct parsed_optionRené Scharfe2024-03-031-34/+49
| | | | | | | | | | | | | 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>
* parse-options: set arg of abbreviated option lazilyRené Scharfe2024-03-031-3/+4
| | | | | | | | | | | 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>
* parse-options: recognize abbreviated negated option with argRené Scharfe2024-03-031-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* Merge branch 'rs/parse-options-with-keep-unknown-abbrev-fix'Junio C Hamano2024-01-301-10/+11
|\ | | | | | | | | | | | | | | | | "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
| * parse-options: simplify positivation handlingRené Scharfe2024-01-221-7/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * parse-options: fully disable option abbreviation with PARSE_OPT_KEEP_UNKNOWNRené Scharfe2024-01-201-3/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* | Merge branch 'en/header-cleanup'Junio C Hamano2024-01-081-2/+0
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| * | treewide: remove unnecessary includes in source filesElijah Newren2023-12-261-2/+0
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* | Merge branch 'jk/end-of-options'Junio C Hamano2023-12-201-2/+7
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | "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 "--"
| * | parse-options: decouple "--end-of-options" and "--"Jeff King2023-12-091-2/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we added generic end-of-options support in 51b4594b40 (parse-options: allow --end-of-options as a synonym for "--", 2019-08-06), we made them true synonyms. They both stop option parsing, and they are both returned in the resulting argv if the KEEP_DASHDASH flag is used. The hope was that this would work for all callers: - most generic callers would not pass KEEP_DASHDASH, and so would just do the right thing (stop parsing there) without needing to know anything more. - callers with KEEP_DASHDASH were generally going to rely on setup_revisions(), which knew to handle --end-of-options specially But that turned out miss quite a few cases that pass KEEP_DASHDASH but do their own manual parsing. For example, "git reset", "git checkout", and so on want pass KEEP_DASHDASH so they can support: git reset $revs -- $paths but of course aren't going to actually do a traversal, so they don't call setup_revisions(). And those cases currently get confused by --end-of-options being left in place, like: $ git reset --end-of-options HEAD fatal: option '--end-of-options' must come before non-option arguments We could teach each of these callers to handle the leftover option explicitly. But let's try to be a bit more clever and see if we can solve it centrally in parse-options.c. The bogus assumption here is that KEEP_DASHDASH tells us the caller wants to see --end-of-options in the result. But really, the callers which need to know that --end-of-options was reached are those that may potentially parse more options from argv. In other words, those that pass the KEEP_UNKNOWN_OPT flag. If such a caller is aware of --end-of-options (e.g., because they call setup_revisions() with the result), then this will continue to do the right thing, treating anything after --end-of-options as a non-option. And if the caller is not aware of --end-of-options, they are better off keeping it intact, because either: 1. They are just passing the options along to somebody else anyway, in which case that somebody would need to know about the --end-of-options marker. 2. They are going to parse the remainder themselves, at which point choking on --end-of-options is much better than having it silently removed. The point is to avoid option injection from untrusted command line arguments, and bailing is better than quietly treating the untrusted argument as an option. This fixes bugs with --end-of-options across several commands, but I've focused on two in particular here: - t7102 confirms that "git reset --end-of-options --foo" now works. This checks two things. One, that we no longer barf on "--end-of-options" itself (which previously we did, even if the rev was something vanilla like "HEAD" instead of "--foo"). And two, that we correctly treat "--foo" as a revision rather than an option. This fix applies to any other cases which pass KEEP_DASHDASH but not KEEP_UNKNOWN_OPT, like "git checkout", "git check-attr", "git grep", etc, which would previously choke on "--end-of-options". - t9350 shows the opposite case: fast-export passed KEEP_UNKNOWN_OPT but not KEEP_DASHDASH, but then passed the result on to setup_revisions(). So it never saw --end-of-options, and would erroneously parse "fast-export --end-of-options --foo" as having a "--foo" option. This is now fixed. Note that this does shut the door for callers which want to know if we hit end-of-options, but don't otherwise need to keep unknown opts. The obvious thing here is feeding it to the DWIM verify_filename() machinery. And indeed, this is a problem even for commands which do understand --end-of-options already. For example, without this patch, you get: $ git log --end-of-options --foo fatal: option '--foo' must come before non-option arguments because we refuse to accept "--foo" as a filename (because it starts with a dash) even though we could know that we saw end-of-options. The verify_filename() function simply doesn't accept this extra information. So that is the status quo, and this patch doubles down further on that. Commands like "git reset" have the same problem, but they won't even know that parse-options saw --end-of-options! So even if we fixed verify_filename(), they wouldn't have anything to pass to it. But in practice I don't think this is a big deal. If you are being careful enough to use --end-of-options, then you should also be using "--" to disambiguate and avoid the DWIM behavior in the first place. In other words, doing: git log --end-of-options --this-is-a-rev -- --this-is-a-path works correctly, and will continue to do so. And likewise, with this patch now: git reset --end-of-options --this-is-a-rev -- --this-is-a-path will work, as well. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | i18n: factorize even more 'incompatible options' messagesRené Scharfe2023-11-271-1/+2
| |/ |/| | | | | | | | | | | | | | | | | | | | | Continue the work of 12909b6b8a (i18n: turn "options are incompatible" into "cannot be used together", 2022-01-05) and a699367bb8 (i18n: factorize more 'incompatible options' messages, 2022-01-31) to use the same parameterized error message for reporting incompatible command line options. This reduces the number of strings to translate and makes the UI slightly more consistent. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | parse-options: make CMDMODE errors more preciseRené Scharfe2023-10-291-52/+92
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Only a single PARSE_OPT_CMDMODE option can be specified for the same variable at the same time. This is enforced by get_value(), but the error messages are imprecise in three ways: 1. If a non-PARSE_OPT_CMDMODE option changes the value variable of a PARSE_OPT_CMDMODE option then an ominously vague message is shown: $ t/helper/test-tool parse-options --set23 --mode1 error: option `mode1' : incompatible with something else Worse: If the order of options is reversed then no error is reported at all: $ t/helper/test-tool parse-options --mode1 --set23 boolean: 0 integer: 23 magnitude: 0 timestamp: 0 string: (not set) abbrev: 7 verbose: -1 quiet: 0 dry run: no file: (not set) Fortunately this can currently only happen in the test helper; actual Git commands don't share the same variable for the value of options with and without the flag PARSE_OPT_CMDMODE. 2. If there are multiple options with the same value (synonyms), then the one that is defined first is shown rather than the one actually given on the command line, which is confusing: $ git am --resolved --quit error: option `quit' is incompatible with --continue 3. Arguments of PARSE_OPT_CMDMODE options are not handled by the parse-option machinery. This is left to the callback function. We currently only have a single affected option, --show-current-patch of git am. Errors for it can show an argument that was not actually given on the command line: $ git am --show-current-patch --show-current-patch=diff error: options '--show-current-patch=diff' and '--show-current-patch=raw' cannot be used together The options --show-current-patch and --show-current-patch=raw are synonyms, but the error accuses the user of input they did not actually made. Or it can awkwardly print a NULL pointer: $ git am --show-current-patch=diff --show-current-patch error: options '--show-current-patch=(null)' and '--show-current-patch=diff' cannot be used together The reasons for these shortcomings is that the current code checks incompatibility only when encountering a PARSE_OPT_CMDMODE option at the command line, and that it searches the previous incompatible option by value. Fix the first two points by checking all PARSE_OPT_CMDMODE variables after parsing each option and by storing all relevant details if their value changed. Do that whether or not the changing options has the flag PARSE_OPT_CMDMODE set. Report an incompatibility only if two options change the variable to different values and at least one of them is a PARSE_OPT_CMDMODE option. This changes the output of the first three examples above to: $ t/helper/test-tool parse-options --set23 --mode1 error: --mode1 is incompatible with --set23 $ t/helper/test-tool parse-options --mode1 --set23 error: --set23 is incompatible with --mode1 $ git am --resolved --quit error: --quit is incompatible with --resolved Store the argument of PARSE_OPT_CMDMODE options of type OPTION_CALLBACK as well to allow taking over the responsibility for compatibility checking from the callback function. The next patch will use this capability to fix the messages for git am --show-current-patch. Use a linked list for storing the PARSE_OPT_CMDMODE variables. This somewhat outdated data structure is simple and suffices, as the number of elements per command is currently only zero or one. We do support multiple different command modes variables per command, but I don't expect that we'd ever use a significant number of them. Once we do we can switch to a hashmap. Since we no longer need to search the conflicting option, the all_opts parameter of get_value() is no longer used. Remove it. Extend the tests to check for both conflicting option names, but don't insist on a particular order. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | parse: separate out parsing functions from config.hCalvin Wan2023-09-301-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The files config.{h,c} contain functions that have to do with parsing, but not config. In order to further reduce all-in-one headers, separate out functions in config.c that do not operate on config into its own file, parse.h, and update the include directives in the .c files that need only such functions accordingly. Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | parse-options: allow omitting option help textRené Scharfe2023-08-281-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 1b68387e02 (builtin/receive-pack.c: use parse_options API, 2016-03-02) added the options --stateless-rpc, --advertise-refs and --reject-thin-pack-for-testing with a NULL `help` string; 03831ef7b5 (difftool: implement the functionality in the builtin, 2017-01-19) similarly added the "helpless" option --prompt. Presumably this was done because all four options are hidden and self-explanatory. They cause a NULL pointer dereference when using the option --help-all with their respective tool, though. Handle such options gracefully instead by turning the NULL pointer into an empty string at the top of the loop, always printing a newline at the end and passing through the separating newlines from the help text. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'rs/parse-options-negation-help'Junio C Hamano2023-08-251-19/+50
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git cmd -h" learned to signal which options can be negated by listing such options like "--[no-]opt". * rs/parse-options-negation-help: parse-options: simplify usage_padding() parse-options: no --[no-]no-... parse-options: factor out usage_indent() and usage_padding() parse-options: show negatability of options in short help t1502: test option negation t1502: move optionspec help output to a file t1502, docs: disallow --no-help subtree: disallow --no-{help,quiet,debug,branch,message}
| * | parse-options: simplify usage_padding()René Scharfe2023-08-071-12/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | c512643e67 (short help: allow a gap smaller than USAGE_GAP, 2023-07-18) effectively did away with the two-space gap between options and their description; one space is enough now. Incorporate USAGE_GAP into USAGE_OPTS_WIDTH, merge the two cases with enough space on the line and incorporate the newline into the format for the remaining case. The output remains the same. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | parse-options: no --[no-]no-...René Scharfe2023-08-071-1/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Avoid showing an optional "no-" for options that already start with a "no-" in the short help, as that double negation is confusing. Document the opposite variant on its own line with a generated help text instead, unless it's defined and documented explicitly already. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | parse-options: factor out usage_indent() and usage_padding()René Scharfe2023-08-071-15/+24
| | | | | | | | | | | | | | | | | | | | | | | | Extract functions for printing spaces before and after options. We'll need them in the next commit. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | parse-options: show negatability of options in short helpRené Scharfe2023-08-071-2/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a "[no-]" prefix to options without the flag PARSE_OPT_NONEG to document the fact that you can negate them. This looks a bit strange for options that already start with "no-", e.g. for the option --no-name of git show-branch: --[no-]no-name suppress naming strings You can actually use --no-no-name as an alias of --name, so the short help is not wrong. If we strip off any of the "no-"s, we lose either the ability to see if the remaining one belongs to the documented variant or to see if it can be negated. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | parse-options: disallow negating OPTION_SET_INT 0René Scharfe2023-08-091-0/+3
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | An option of type OPTION_SET_INT can be defined to set its variable to zero. It's negated variant will do the same, though, which is confusing. Several such options were fixed by disabling negation, changing the value to set or using a different option type: 991c552916 (ls-tree: fix --no-full-name, 2023-07-18) e12cb98e1e (branch: reject "--no-all" and "--no-remotes" early, 2023-07-18) 68cbb20e73 (show-branch: reject --[no-](topo|date)-order, 2023-07-19) 3821eb6c3d (reset: reject --no-(mixed|soft|hard|merge|keep) option, 2023-07-19) 36f76d2a25 (pack-objects: fix --no-quiet, 2023-07-21) 3a5f308741 (pack-objects: fix --no-keep-true-parents, 2023-07-21) c95ae3ff9c (describe: fix --no-exact-match, 2023-07-21) d089a06421 (bundle: use OPT_PASSTHRU_ARGV, 2023-07-29) Check for such options that allow negation in parse_options_check() and report them to find future cases quicker. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | short help: allow a gap smaller than USAGE_GAPJunio C Hamano2023-07-201-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The parse-options API responds to "git cmd -h" by listing the option flag (padded to the USAGE_OPTS_WIDTH column), followed by USAGE_GAP (set to 2) whitespaces, followed by the help text. If the flags part does not fit within the USAGE_OPTS_WIDTH, the help text is given on its own line. Imagine that "@" below depicts the USAGE_OPTS_WIDTH'th column, and "#" are for the usage help text, the output may look like this: @@@@@@@@@@@@@ ######################################## -f description of the flag '-f' comes here --short=<num> description of the flag '--short' --very-long-option=<number> description of the flag '--very-long-option' This is all good and nice in principle, but it becomes awkward when the flags part is just one column over the limit and forces a line break. See the description of the "--almost" option below: @@@@@@@@@@@@@ ######################################## -f description of the flag '-f' comes here --short=<num> description of the flag '--short' --almost=<num> description of the flag '--almost' --very-long-option=<number> description of the flag '--very-long-option' If we allow shrinking the gap to a single whitespace only in such a case, we would instead get: @@@@@@@@@@@@@ ######################################## -f description of the flag '-f' comes here --short=<num> description of the flag '--short' --almost=<num> description of the flag '--almost' --very-long-option=<number> description of the flag '--very-long-option' and the boundary between the flags and their descriptions does not become any harder to see, while saving precious vertical screen real estate. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | short help: allow multi-line opthelpJunio C Hamano2023-07-201-1/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When "-h" triggers the short-help in a command that implements its option parsing using the parse-options API, the option help text is shown with a single fprintf() as a long line. When the text is multi-line, the second and subsequent lines are not left padded, that breaks the alignment across options. Borrowing the idea from the advice API where its hint strings are shown with (localized) "hint:" prefix, let's internally split the (localized) help text into lines, and showing the first line, pad the remaining lines to align. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | commit.h: reduce unnecessary includesElijah Newren2023-04-241-0/+1
| | | | | | | | | | Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | treewide: remove unnecessary includes of cache.hElijah Newren2023-03-211-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The last several commits were geared at replacing the include of cache.h in strbuf.c with an include of git-compat-util.h. Unfortunately, I had to drop a patch moving some functions from cache.h to object-name.h, due to excessive conflicts with other in-flight topics. However, even without that patch, the series of patches so far allows us to modify a number of C files to replace an include of cache.h with git-compat-util.h. Do that to reduce our dependencies. (If we could have kept our object-name.h patch in this series, it would have also let us reduce the includes in checkout.c and fmt-merge-msg.c in addition to strbuf.c). Just to ensure that nothing else was bringing in cache.h, all of the affected files have been checked to ensure that gcc -E -I. $SOURCE_FILE | grep '"cache.h"' found no hits and that make DEVELOPER=1 ${OBJECT_FILE_FOR_SOURCE_FILE} successfully compiles without warnings. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | abspath.h: move absolute path functions from cache.hElijah Newren2023-03-211-0/+1
| | | | | | | | | | | | | | | | | | This is another step towards letting us remove the include of cache.h in strbuf.c. It does mean that we also need to add includes of abspath.h in a number of C files. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | treewide: be explicit about dependence on gettext.hElijah Newren2023-03-211-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Dozens of files made use of gettext functions, without explicitly including gettext.h. This made it more difficult to find which files could remove a dependence on cache.h. Make C files explicitly include gettext.h if they are using it. However, while compat/fsmonitor/fsm-ipc-darwin.c should also gain an include of gettext.h, it was left out to avoid conflicting with an in-flight topic. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jk/bundle-use-dash-for-stdfiles'Junio C Hamano2023-03-191-6/+6
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | | | | | "git bundle" learned that "-" is a common way to say that the input comes from the standard input and/or the output goes to the standard output. It used to work only for output and only from the root level of the working tree. * jk/bundle-use-dash-for-stdfiles: parse-options: use prefix_filename_except_for_dash() helper parse-options: consistently allocate memory in fix_filename() bundle: don't blindly apply prefix_filename() to "-" bundle: document handling of "-" as stdin bundle: let "-" mean stdin for reading operations
| * parse-options: use prefix_filename_except_for_dash() helperJeff King2023-03-061-3/+1
| | | | | | | | | | | | | | | | Since our fix_filename()'s only remaining special case is handling "-", we can use the newly-minted helper function that handles this already. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * parse-options: consistently allocate memory in fix_filename()Jeff King2023-03-061-6/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When handling OPT_FILENAME(), we have to stick the "prefix" (if any) in front of the filename to make up for the fact that Git has chdir()'d to the top of the repository. We can do this with prefix_filename(), but there are a few special cases we handle ourselves. Unfortunately the memory allocation is inconsistent here; if we do make it to prefix_filename(), we'll allocate a string which the caller must free to avoid a leak. But if we hit our special cases, we'll return the string as-is, and a caller which tries to free it will crash. So there's no way to win. Let's consistently allocate, so that callers can do the right thing. There are now three cases to care about in the function (and hence a three-armed if/else): 1. we got a NULL input (and should leave it as NULL, though arguably this is the sign of a bug; let's keep the status quo for now and we can pick at that scab later) 2. we hit a special case that means we leave the name intact; we should duplicate the string. This includes our special "-" matching. Prior to this patch, it also included empty prefixes and absolute filenames. But we can observe that prefix_filename() already handles these, so we don't need to detect them. 3. everything else goes to prefix_filename() I've dropped the "const" from the "char **file" parameter to indicate that we're allocating, though in practice it's not really important. This is all being shuffled through a void pointer via opt->value before it hits code which ever looks at the string. And it's even a bit weird, because we are really taking _in_ a const string and using the same out-parameter for a non-const string. A better function signature would be: static char *fix_filename(const char *prefix, const char *file); but that would mean the caller dereferences the double-pointer (and the NULL check is currently handled inside this function). So I took the path of least-change here. Note that we have to fix several callers in this commit, too, or we'll break the leak-checking tests. These are "new" leaks in the sense that they are now triggered by the test suite, but these spots have always been leaky when Git is run in a subdirectory of the repository. I fixed all of the cases that trigger with GIT_TEST_PASSING_SANITIZE_LEAK. There may be others in scripts that have other leaks, but we can fix them later along with those other leaks (and again, you _couldn't_ fix them before this patch, so this is the necessary first step). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | use DUP_ARRAYRené Scharfe2023-01-091-2/+1
|/ | | | | | | | Add a semantic patch for replace ALLOC_ARRAY+COPY_ARRAY with DUP_ARRAY to reduce code duplication and apply its results. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* parse-options: add support for parsing subcommandsSZEDER Gábor2022-08-191-6/+107
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Several Git commands have subcommands to implement mutually exclusive "operation modes", and they usually parse their subcommand argument with a bunch of if-else if statements. Teach parse-options to handle subcommands as well, which will result in shorter and simpler code with consistent error handling and error messages on unknown or missing subcommand, and it will also make possible for our Bash completion script to handle subcommands programmatically. The approach is guided by the following observations: - Most subcommands [1] are implemented in dedicated functions, and most of those functions [2] either have a signature matching the 'int cmd_foo(int argc, const char **argc, const char *prefix)' signature of builtin commands or can be trivially converted to that signature, because they miss only that last prefix parameter or have no parameters at all. - Subcommand arguments only have long form, and they have no double dash prefix, no negated form, and no description, and they don't take any arguments, and can't be abbreviated. - There must be exactly one subcommand among the arguments, or zero if the command has a default operation mode. - All arguments following the subcommand are considered to be arguments of the subcommand, and, conversely, arguments meant for the subcommand may not preceed the subcommand. So in the end subcommand declaration and parsing would look something like this: parse_opt_subcommand_fn *fn = NULL; struct option builtin_commit_graph_options[] = { OPT_STRING(0, "object-dir", &opts.obj_dir, N_("dir"), N_("the object directory to store the graph")), OPT_SUBCOMMAND("verify", &fn, graph_verify), OPT_SUBCOMMAND("write", &fn, graph_write), OPT_END(), }; argc = parse_options(argc, argv, prefix, options, builtin_commit_graph_usage, 0); return fn(argc, argv, prefix); Here each OPT_SUBCOMMAND specifies the name of the subcommand and the function implementing it, and the address of the same 'fn' subcommand function pointer. parse_options() then processes the arguments until it finds the first argument matching one of the subcommands, sets 'fn' to the function associated with that subcommand, and returns, leaving the rest of the arguments unprocessed. If none of the listed subcommands is found among the arguments, parse_options() will show usage and abort. If a command has a default operation mode, 'fn' should be initialized to the function implementing that mode, and parse_options() should be invoked with the PARSE_OPT_SUBCOMMAND_OPTIONAL flag. In this case parse_options() won't error out when not finding any subcommands, but will return leaving 'fn' unchanged. Note that if that default operation mode has any --options, then the PARSE_OPT_KEEP_UNKNOWN_OPT flag is necessary as well (otherwise parse_options() would error out upon seeing the unknown option meant to the default operation mode). Some thoughts about the implementation: - The same pointer to 'fn' must be specified as 'value' for each OPT_SUBCOMMAND, because there can be only one set of mutually exclusive subcommands; parse_options() will BUG() otherwise. There are other ways to tell parse_options() where to put the function associated with the subcommand given on the command line, but I didn't like them: - Change parse_options()'s signature by adding a pointer to subcommand function to be set to the function associated with the given subcommand, affecting all callsites, even those that don't have subcommands. - Introduce a specific parse_options_and_subcommand() variant with that extra funcion parameter. - I decided against automatically calling the subcommand function from within parse_options(), because: - There are commands that have to perform additional actions after option parsing but before calling the function implementing the specified subcommand. - The return code of the subcommand is usually the return code of the git command, but preserving the return code of the automatically called subcommand function would have made the API awkward. - Also add a OPT_SUBCOMMAND_F() variant to allow specifying an option flag: we have two subcommands that are purposefully excluded from completion ('git remote rm' and 'git stash save'), so they'll have to be specified with the PARSE_OPT_NOCOMPLETE flag. - Some of the 'parse_opt_flags' don't make sense with subcommands, and using them is probably just an oversight or misunderstanding. Therefore parse_options() will BUG() when invoked with any of the following flags while the options array contains at least one OPT_SUBCOMMAND: - PARSE_OPT_KEEP_DASHDASH: parse_options() stops parsing arguments when encountering a "--" argument, so it doesn't make sense to expect and keep one before a subcommand, because it would prevent the parsing of the subcommand. However, this flag is allowed in combination with the PARSE_OPT_SUBCOMMAND_OPTIONAL flag, because the double dash might be meaningful for the command's default operation mode, e.g. to disambiguate refs and pathspecs. - PARSE_OPT_STOP_AT_NON_OPTION: As its name suggests, this flag tells parse_options() to stop as soon as it encouners a non-option argument, but subcommands are by definition not options... so how could they be parsed, then?! - PARSE_OPT_KEEP_UNKNOWN: This flag can be used to collect any unknown --options and then pass them to a different command or subsystem. Surely if a command has subcommands, then this functionality should rather be delegated to one of those subcommands, and not performed by the command itself. However, this flag is allowed in combination with the PARSE_OPT_SUBCOMMAND_OPTIONAL flag, making possible to pass --options to the default operation mode. - If the command with subcommands has a default operation mode, then all arguments to the command must preceed the arguments of the subcommand. AFAICT we don't have any commands where this makes a difference, because in those commands either only the command accepts any arguments ('notes' and 'remote'), or only the default subcommand ('reflog' and 'stash'), but never both. - The 'argv' array passed to subcommand functions currently starts with the name of the subcommand. Keep this behavior. AFAICT no subcommand functions depend on the actual content of 'argv[0]', but the parse_options() call handling their options expects that the options start at argv[1]. - To support handling subcommands programmatically in our Bash completion script, 'git cmd --git-completion-helper' will now list both subcommands and regular --options, if any. This means that the completion script will have to separate subcommands (i.e. words without a double dash prefix) from --options on its own, but that's rather easy to do, and it's not much work either, because the number of subcommands a command might have is rather low, and those commands accept only a single --option or none at all. An alternative would be to introduce a separate option that lists only subcommands, but then the completion script would need not one but two git invocations and command substitutions for commands with subcommands. Note that this change doesn't affect the behavior of our Bash completion script, because when completing the --option of a command with subcommands, e.g. for 'git notes --<TAB>', then all subcommands will be filtered out anyway, as none of them will match the word to be completed starting with that double dash prefix. [1] Except 'git rerere', because many of its subcommands are implemented in the bodies of the if-else if statements parsing the command's subcommand argument. [2] Except 'credential', 'credential-store' and 'fsmonitor--daemon', because some of the functions implementing their subcommands take special parameters. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* parse-options: drop leading space from '--git-completion-helper' outputSZEDER Gábor2022-08-191-1/+2
| | | | | | | | | | | | | | | | | | | | | The output of 'git <cmd> --git-completion-helper' always starts with a space, e.g.: $ git config --git-completion-helper --global --system --local [...] This doesn't matter for the completion script, because field splitting discards that space anyway. However, later patches in this series will teach parse-options to handle subcommands, and subcommands will be included in the completion helper output as well. This will make the loop printing options (and subcommands) a tad more complex, so I wanted to test the result. The test would have to account for the presence of that leading space, which bugged my OCD, so let's get rid of it. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* parse-options: PARSE_OPT_KEEP_UNKNOWN only applies to --optionsSZEDER Gábor2022-08-191-3/+3
| | | | | | | | | | | | | | The description of 'PARSE_OPT_KEEP_UNKNOWN' starts with "Keep unknown arguments instead of erroring out". This is a bit misleading, as this flag only applies to unknown --options, while non-option arguments are kept even without this flag. Update the description to clarify this, and rename the flag to PARSE_OPTIONS_KEEP_UNKNOWN_OPT to make this obvious just by looking at the flag name. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* parse-options.c: use optbug() instead of BUG() "opts" checkÆvar Arnfjörð Bjarmason2022-06-021-8/+9
| | | | | | | | | | Change the assertions added in bf3ff338a25 (parse-options: stop abusing 'callback' for lowlevel callbacks, 2019-01-27) to use optbug() instead of BUG(). At this point we're looping over individual options, so if we encounter any issues we'd like to report the offending option. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* parse-options.c: use new bug() API for optbug()Ævar Arnfjörð Bjarmason2022-06-021-19/+17
| | | | | | | | | | | | | | | | | | | | | | When we run into bugs in parse-options.c usage it's good to be able to note all the issues we ran into before dying. This use-case is why we have the optbug() function introduced in 1e5ce570ca3 (parse-options: clearer reporting of API misuse, 2010-12-02) Let's change this code to use the new bug() API introduced in the preceding commit, which cuts down on the verbosity of parse_options_check(). There are existing uses of BUG() in adjacent code that should have been using optbug() that aren't being changed here. That'll be done in a subsequent commit. This only changes the optbug() callers. Since this will invoke BUG() the previous exit(128) code will be changed, but in this case that's what we want, i.e. to have encountering a BUG() return the specific "BUG" exit code. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'ja/i18n-common-messages'Junio C Hamano2022-02-261-0/+34
|\ | | | | | | | | | | | | | | | | | | Unify more messages to help l10n. * ja/i18n-common-messages: i18n: fix some misformated placeholders in command synopsis i18n: remove from i18n strings that do not hold translatable parts i18n: factorize "invalid value" messages i18n: factorize more 'incompatible options' messages
| * i18n: factorize more 'incompatible options' messagesJean-Noël Avila2022-02-041-0/+34
| | | | | | | | | | | | | | | | | | | | Find more incompatible options to factorize. When more than two options are mutually exclusive, print the ones which are actually on the command line. Signed-off-by: Jean-Noël Avila <jn.avila@free.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/cat-file'Junio C Hamano2022-02-051-0/+13
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Assorted updates to "git cat-file", especially "-h". * ab/cat-file: cat-file: s/_/-/ in typo'd usage_msg_optf() message cat-file: don't whitespace-pad "(...)" in SYNOPSIS and usage output cat-file: use GET_OID_ONLY_TO_DIE in --(textconv|filters) object-name.c: don't have GET_OID_ONLY_TO_DIE imply *_QUIETLY cat-file: correct and improve usage information cat-file: fix remaining usage bugs cat-file: make --batch-all-objects a CMDMODE cat-file: move "usage" variable to cmd_cat_file() cat-file docs: fix SYNOPSIS and "-h" output parse-options API: add a usage_msg_optf() cat-file tests: test messaging on bad objects/paths cat-file tests: test bad usage
| * parse-options API: add a usage_msg_optf()Ævar Arnfjörð Bjarmason2021-12-301-0/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a usage_msg_optf() as a shorthand for the sort of usage_msg_opt(xstrfmt(...)) used in builtin/stash.c. I'll make more use of this function in builtin/cat-file.c shortly. The disconnect between the "..." and "fmt" is a bit unusual, but it works just fine and this keeps it consistent with usage_msg_opt(), i.e. a caller of it can be moved to usage_msg_optf() and not have to have its arguments re-arranged. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/usage-die-message'Junio C Hamano2022-01-101-1/+1
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up to hide vreportf() from public API. * ab/usage-die-message: config API: use get_error_routine(), not vreportf() usage.c + gc: add and use a die_message_errno() gc: return from cmd_gc(), don't call exit() usage.c API users: use die_message() for error() + exit 128 usage.c API users: use die_message() for "fatal :" + exit 128 usage.c: add a die_message() routine
| * | usage.c API users: use die_message() for "fatal :" + exit 128Ævar Arnfjörð Bjarmason2021-12-071-1/+1
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change code that printed its own "fatal: " message and exited with a status code of 128 to use the die_message() function added in a preceding commit. This change also demonstrates why the return value of die_message_routine() needed to be that of "report_fn". We have callers such as the run-command.c::child_err_spew() which would like to replace its error routine with the return value of "get_die_message_routine()". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/parse-options-cleanup'Junio C Hamano2021-12-151-3/+4
|\ \ | |/ |/| | | | | | | | | | | Change the type of an internal function to return an enum (instead of int) and replace -2 that was used to signal an error with -1. * ab/parse-options-cleanup: parse-options.c: use "enum parse_opt_result" for parse_nodash_opt()
| * parse-options.c: use "enum parse_opt_result" for parse_nodash_opt()Ævar Arnfjörð Bjarmason2021-11-111-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change the parse_nodash_opt() function to use "enum parse_opt_result". In 352e761388b (parse-options.[ch]: consistently use "enum parse_opt_result", 2021-10-08) its only caller parse_options_step() started using that return type, and the get_value() which will be called and return from it uses the same enum. Let's do the same here so that this function always returns an "enum parse_opt_result" value. We could go for either PARSE_OPT_HELP (-2) or PARSE_OPT_ERROR (-1) here. The reason we ended up with "-2" is that in code added in 07fe54db3cd (parse-opt: do not print errors on unknown options, return "-2" instead., 2008-06-23) we used that value in a meaningful way. Then in 51a9949eda7 (parseopt: add PARSE_OPT_NODASH, 2009-05-07) the use of "-2" was seemingly copy/pasted from parse_long_opt(), which was the function immediately above the parse_nodash_opt() function added in that commit. Since we only care about whether the return value here is non-zero let's use the more generic PARSE_OPT_ERROR. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/parse-options-cleanup'Junio C Hamano2021-11-091-5/+5
|\| | | | | | | | | | | | | Last minute fix to the update already in 'master'. * ab/parse-options-cleanup: parse-options.[ch]: revert use of "enum" for parse_options()
| * parse-options.[ch]: revert use of "enum" for parse_options()Ævar Arnfjörð Bjarmason2021-11-091-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | Revert the parse_options() prototype change in my recent 352e761388b (parse-options.[ch]: consistently use "enum parse_opt_result", 2021-10-08) was incorrect. The parse_options() function returns the number of argc elements that haven't been processed, not "enum parse_opt_result". Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/parse-options-cleanup'Junio C Hamano2021-10-261-38/+49
|\| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Random changes to parse-options implementation. * ab/parse-options-cleanup: parse-options: change OPT_{SHORT,UNSET} to an enum parse-options tests: test optname() output parse-options.[ch]: make opt{bug,name}() "static" commit-graph: stop using optname() parse-options.c: move optname() earlier in the file parse-options.h: make the "flags" in "struct option" an enum parse-options.c: use exhaustive "case" arms for "enum parse_opt_result" parse-options.[ch]: consistently use "enum parse_opt_result" parse-options.[ch]: consistently use "enum parse_opt_flags" parse-options.h: move PARSE_OPT_SHELL_EVAL between enums
| * parse-options: change OPT_{SHORT,UNSET} to an enumÆvar Arnfjörð Bjarmason2021-10-081-9/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change the comparisons against OPT_SHORT and OPT_UNSET to an enum which keeps track of how a given option got parsed. The case of "0" was an implicit OPT_LONG, so let's add an explicit label for it. Due to the xor in 0f1930c5875 (parse-options: allow positivation of options starting, with no-, 2012-02-25) the code already relied on this being set back to 0. To avoid refactoring the logic involved in that let's just start the enum at "0" instead of the usual "1<<0" (1), but BUG() out if we don't have one of our expected flags. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>