Commit Graph

151 Commits

Author SHA1 Message Date
Andrzej Hunt
64cc539fd2 parse-options: don't leak alias help messages
preprocess_options() allocates new strings for help messages for
OPTION_ALIAS. Therefore we also need to clean those help messages up
when freeing the returned options.

First introduced in:
  7c280589cf (parse-options: teach "git cmd -h" to show alias as alias, 2020-03-16)

The preprocessed options themselves no longer contain any indication
that a given option is/was an alias - therefore we add a new flag to
indicate former aliases. (An alternative approach would be to look back
at the original options to determine which options are aliases - but
that seems like a fragile approach. Or we could even look at the
alias_groups list - which might be less fragile, but would be slower
as it requires nested looping.)

As far as I can tell, parse_options() is only ever used once per
command, and the help messages are small - hence this leak has very
little impact.

This leak was found while running t0001. LSAN output can be found below:

Direct leak of 65 byte(s) in 1 object(s) allocated from:
    #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    #1 0x9aae36 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x939d8d in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x93b936 in strbuf_vaddf /home/ahunt/oss-fuzz/git/strbuf.c:392:3
    #4 0x93b7ff in strbuf_addf /home/ahunt/oss-fuzz/git/strbuf.c:333:2
    #5 0x86747e in preprocess_options /home/ahunt/oss-fuzz/git/parse-options.c:666:3
    #6 0x866ed2 in parse_options /home/ahunt/oss-fuzz/git/parse-options.c:847:17
    #7 0x51c4a7 in cmd_clone /home/ahunt/oss-fuzz/git/builtin/clone.c:989:9
    #8 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    #9 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    #10 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    #11 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    #12 0x69c9fe in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    #13 0x7fdac42d4349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-21 14:39:10 -07:00
Torsten Bögershausen
5c327502db MacOS: precompose_argv_prefix()
The following sequence leads to a "BUG" assertion running under MacOS:

  DIR=git-test-restore-p
  Adiarnfd=$(printf 'A\314\210')
  DIRNAME=xx${Adiarnfd}yy
  mkdir $DIR &&
  cd $DIR &&
  git init &&
  mkdir $DIRNAME &&
  cd $DIRNAME &&
  echo "Initial" >file &&
  git add file &&
  echo "One more line" >>file &&
  echo y | git restore -p .

 Initialized empty Git repository in /tmp/git-test-restore-p/.git/
 BUG: pathspec.c:495: error initializing pathspec_item
 Cannot close git diff-index --cached --numstat
 [snip]

The command `git restore` is run from a directory inside a Git repo.
Git needs to split the $CWD into 2 parts:
The path to the repo and "the rest", if any.
"The rest" becomes a "prefix" later used inside the pathspec code.

As an example, "/path/to/repo/dir-inside-repå" would determine
"/path/to/repo" as the root of the repo, the place where the
configuration file .git/config is found.

The rest becomes the prefix ("dir-inside-repå"), from where the
pathspec machinery expands the ".", more about this later.
If there is a decomposed form, (making the decomposing visible like this),
"dir-inside-rep°a" doesn't match "dir-inside-repå".

Git commands need to:

 (a) read the configuration variable "core.precomposeunicode"
 (b) precocompose argv[]
 (c) precompose the prefix, if there was any

The first commit,
76759c7dff "git on Mac OS and precomposed unicode"
addressed (a) and (b).

The call to precompose_argv() was added into parse-options.c,
because that seemed to be a good place when the patch was written.

Commands that don't use parse-options need to do (a) and (b) themselfs.

The commands `diff-files`, `diff-index`, `diff-tree` and `diff`
learned (a) and (b) in
commit 90a78b83e0 "diff: run arguments through precompose_argv"

Branch names (or refs in general) using decomposed code points
resulting in decomposed file names had been fixed in
commit 8e712ef6fc "Honor core.precomposeUnicode in more places"

The bug report from above shows 2 things:
- more commands need to handle precomposed unicode
- (c) should be implemented for all commands using pathspecs

Solution:
precompose_argv() now handles the prefix (if needed), and is renamed into
precompose_argv_prefix().

Inside this function the config variable core.precomposeunicode is read
into the global variable precomposed_unicode, as before.
This reading is skipped if precomposed_unicode had been read before.

The original patch for preocomposed unicode, 76759c7dff, placed
precompose_argv() into parse-options.c

Now add it into git.c::run_builtin() as well.  Existing precompose
calls in diff-files.c and others may become redundant, and if we
audit the callflows that reach these places to make sure that they
can never be reached without going through the new call added to
run_builtin(), we might be able to remove these existing ones.

But in this commit, we do not bother to do so and leave these
precompose callsites as they are.  Because precompose() is
idempotent and can be called on an already precomposed string
safely, this is safer than removing existing calls without fully
vetting the callflows.

There is certainly room for cleanups - this change intends to be a bug fix.
Cleanups needs more tests in e.g. t/t3910-mac-os-precompose.sh, and should
be done in future commits.

[1] git-bugreport-2021-01-06-1209.txt (git can't deal with special characters)
[2] https://lore.kernel.org/git/A102844A-9501-4A86-854D-E3B387D378AA@icloud.com/

Reported-by: Daniel Troger <random_n0body@icloud.com>
Helped-By: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-03 14:09:37 -08:00
Ryan Zoeller
a0abe5e3b7 parse-options: add --git-completion-helper-all
--git-completion-helper excludes hidden options, such as --allow-empty
for git commit. This is typically helpful, but occasionally we want
auto-completion for obscure flags. --git-completion-helper-all returns
all options, even if they are marked as hidden or nocomplete.

Signed-off-by: Ryan Zoeller <rtzoeller@rtzoeller.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-19 17:46:17 -07:00
Junio C Hamano
7c280589cf parse-options: teach "git cmd -h" to show alias as alias
There is a long-standing NEEDSWORK comment that complains about
inconsistency between how an aliased option ("git clone --recurse"
which is the only one that currently exists) gives a help text in
a usage-error message vs "git cmd -h").  Get rid of it and then
make sure we say an option is an alias for another, instead of
repeating the same short help text for both, which leads to "they
seem to do the same---is there any subtle difference?" puzzlement
to end-users.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-16 14:27:07 -07:00
Junio C Hamano
0e0d717537 Merge branch 'pb/am-show-current-patch'
"git am --short-current-patch" is a way to show the piece of e-mail
for the stopped step, which is not suitable to directly feed "git
apply" (it is designed to be a good "git am" input).  It learned a
new option to show only the patch part.

* pb/am-show-current-patch:
  am: support --show-current-patch=diff to retrieve .git/rebase-apply/patch
  am: support --show-current-patch=raw as a synonym for--show-current-patch
  am: convert "resume" variable to a struct
  parse-options: convert "command mode" to a flag
  parse-options: add testcases for OPT_CMDMODE()
2020-03-09 11:21:19 -07:00
Paolo Bonzini
bc8620b440 parse-options: convert "command mode" to a flag
OPTION_CMDMODE is essentially OPTION_SET_INT plus an extra check that
the variable had not set before.  In order to allow custom processing
of the option, for example a "command mode" option that also has an
argument, it would be nice to use OPTION_CALLBACK and not have to rewrite
the extra check on incompatible options.  In other words, making the
processing of the option orthogonal to the "only one of these" behavior
provided by OPTION_CMDMODE.

Add a new flag that takes care of the check, and modify OPT_CMDMODE to
use it together with OPTION_SET_INT.  The new flag still requires that the
option value points to an int, but any OPTION_* value can be specified as
long as it does not require a non-int type for opt->value.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-20 13:20:40 -08:00
Junio C Hamano
db72f8c940 Merge branch 'jb/parse-options-message-fix'
Error message fix.

* jb/parse-options-message-fix:
  parse-options: lose an unnecessary space in an error message
2020-02-12 12:41:37 -08:00
Jacques Bodin-Hullin
395518cf7a parse-options: lose an unnecessary space in an error message
Signed-off-by: Jacques Bodin-Hullin <j.bodinhullin@monsieurbiz.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05 10:49:29 -08:00
Junio C Hamano
145136a95a C: use skip_prefix() to avoid hardcoded string length
We often skip an optional prefix in a string with a hardcoded
constant, e.g.

	if (starts_with(string, "prefix"))
		string += 6;

which is less error prone when written

	skip_prefix(string, "prefix", &string);

Note that this changes a few error messages from "git reflog expire
--expire=nonsense.timestamp", which used to complain by saying

    '--expire=nonsense.timestamp' is not a valid timestamp

but with this change, we say

    'nonsense.timestamp' is not a valid timestamp

which is more technically correct (the string with --expire= as
a prefix obviously cannot be a valid timestamp, but the error is
about the part of the input without that prefix).

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-31 13:03:45 -08:00
Elijah Newren
15beaaa3d1 Fix spelling errors in code comments
Reported-by: Jens Schleusener <Jens.Schleusener@fossies.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-10 16:00:54 +09:00
Jeff King
51b4594b40 parse-options: allow --end-of-options as a synonym for "--"
The revision option parser recently learned about --end-of-options, but
that's not quite enough for all callers. Some of them, like git-log,
pick out some options using parse_options(), and then feed the remainder
to setup_revisions(). For those cases we need to stop parse_options()
from finding more options when it sees --end-of-options, and to retain
that option in argv so that setup_revisions() can see it as well.

Let's handle this the same as we do "--". We can even piggy-back on the
handling of PARSE_OPT_KEEP_DASHDASH, because any caller that wants to
retain one will want to retain the other.

I've included two tests here. The "log" test covers "--source", which is
one of the options it handles with parse_options(), and would fail
before this patch. There's also a test that uses the parse-options
helper directly. That confirms that the option is handled correctly even
in cases without KEEP_DASHDASH or setup_revisions(). I.e., it is safe to
use --end-of-options in place of "--" in other programs.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-06 13:05:39 -07:00
Junio C Hamano
20aa7c594f Merge branch 'nd/diff-parseopt'
A brown-paper-bag bugfix to a change already in 'master'.

* nd/diff-parseopt:
  parse-options: check empty value in OPT_INTEGER and OPT_ABBREV
  diff-parseopt: restore -U (no argument) behavior
  diff-parseopt: correct variable types that are used by parseopt
2019-05-30 10:50:44 -07:00
Nguyễn Thái Ngọc Duy
f7e68a0878 parse-options: check empty value in OPT_INTEGER and OPT_ABBREV
When parsing the argument for OPT_INTEGER and OPT_ABBREV, we check if we
can parse the entire argument to a number with "if (*s)". There is one
missing check: if "arg" is empty to begin with, we fail to notice.

This could happen with long option by writing like

  git diff --inter-hunk-context= blah blah

Before 16ed6c97cc (diff-parseopt: convert --inter-hunk-context,
2019-03-24), --inter-hunk-context is handled by a custom parser
opt_arg() and does detect this correctly.

This restores the bahvior for --inter-hunk-context and make sure all
other integer options are handled the same (sane) way. For OPT_ABBREV
this is new behavior. But it makes it consistent with the rest.

PS. OPT_MAGNITUDE has similar code but git_parse_ulong() does detect
empty "arg". So it's good to go.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-29 11:04:33 -07:00
Nguyễn Thái Ngọc Duy
5c387428f1 parse-options: don't emit "ambiguous option" for aliases
Change the option parsing machinery so that e.g. "clone --recurs ..."
doesn't error out because "clone" understands both "--recursive" and
"--recurse-submodules" to mean the same thing.

Initially "clone" just understood --recursive until the
--recurses-submodules alias was added in ccdd3da652 ("clone: Add the
--recurse-submodules option as alias for --recursive",
2010-11-04). Since bb62e0a99f ("clone: teach --recurse-submodules to
optionally take a pathspec", 2017-03-17) the longer form has been
promoted to the default.

But due to the way the options parsing machinery works this resulted
in the rather absurd situation of:

    $ git clone --recurs [...]
    error: ambiguous option: recurs (could be --recursive or --recurse-submodules)

Add OPT_ALIAS() to express this link between two or more options and use
it in git-clone. Multiple aliases of an option could be written as

    OPT_ALIAS(0, "alias1", "original-name"),
    OPT_ALIAS(0, "alias2", "original-name"),
    ...

The current implementation is not exactly optimal in this case. But we
can optimize it when it becomes a problem. So far we don't even have two
aliases of any option.

A big chunk of code is actually from Junio C Hamano.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-07 12:23:22 +09:00
Junio C Hamano
b72e90712e Merge branch 'js/difftool-no-index'
"git difftool" can now run outside a repository.

* js/difftool-no-index:
  difftool: allow running outside Git worktrees with --no-index
  parse-options: make OPT_ARGUMENT() more useful
  difftool: remove obsolete (and misleading) comment
2019-04-25 16:41:14 +09:00
Junio C Hamano
4284497396 Merge branch 'jk/unused-params-even-more'
Code cleanup.

* jk/unused-params-even-more:
  parse_opt_ref_sorting: always use with NONEG flag
  pretty: drop unused strbuf from parse_padding_placeholder()
  pretty: drop unused "type" parameter in needs_rfc2047_encoding()
  parse-options: drop unused ctx parameter from show_gitcomp()
  fetch_pack(): drop unused parameters
  report_path_error(): drop unused prefix parameter
  unpack-trees: drop unused error_type parameters
  unpack-trees: drop name_entry from traverse_by_cache_tree()
  test-date: drop unused "now" parameter from parse_dates()
  update-index: drop unused prefix_length parameter from do_reupdate()
  log: drop unused "len" from show_tagger()
  log: drop unused rev_info from early output
  revision: drop some unused "revs" parameters
2019-04-25 16:41:12 +09:00
Johannes Schindelin
b02e7d5d70 tests: disallow the use of abbreviated options (by default)
Git's command-line parsers support uniquely abbreviated options, e.g.
`git init --ba` would automatically expand `--ba` to `--bare`.

This is a very convenient feature in every day life for Git users, in
particular when tab completion is not available.

However, it is not a good idea to rely on that in Git's test suite, as
something that is a unique abbreviation of a command line option today
might no longer be a unique abbreviation tomorrow.

For example, if a future contribution added a new mode
`git init --babyproofing` and a previously-introduced test case used the
fact that `git init --ba` expanded to `git init --bare`, that future
contribution would now have to touch seemingly unrelated tests just to
keep the test suite from failing.

So let's disallow abbreviated options in the test suite by default.

Note: for ease of implementation, this patch really only touches the
`parse-options` machinery: more and more hand-rolled option parsers are
converted to use that internal API, and more and more scripts are
converted to built-ins (naturally using the parse-options API, too), so
in practice this catches most issues, and is definitely the biggest bang
for the buck.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-15 11:54:04 +09:00
Jeff King
5205749d2c parse-options: drop unused ctx parameter from show_gitcomp()
The completion display doesn't actually care about where we are in the
parsing. It's generated completely from the set of available options. So
we don't need to see the parse-options context struct at all.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-20 18:34:09 +09:00
Johannes Schindelin
1a85b49b87 parse-options: make OPT_ARGUMENT() more useful
`OPT_ARGUMENT()` is intended to keep the specified long option in `argv`
and not to do anything else.

However, it would make a lot of sense for the caller to know whether
this option was seen at all or not. For example, we want to teach `git
difftool` to work outside of any Git worktree, but only when
`--no-index` was specified.

Note: nothing in Git uses OPT_ARGUMENT(). Even worse, looking through
the commit history, one can easily see that nothing even
ever used it, apart from the regression test.

So not only do we make `OPT_ARGUMENT()` more useful, we are also about
to introduce its first real user!

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-18 11:44:14 +09:00
Nguyễn Thái Ngọc Duy
3ebbe28989 parse-options: allow ll_callback with OPTION_CALLBACK
OPTION_CALLBACK is much simpler/safer to use, but parse_opt_cb does
not allow access to parse_opt_ctx_t, which sometimes is useful
(e.g. to obtain the prefix).

Extending parse_opt_cb to take parse_opt_cb could result in a lot of
changes. Instead let's just allow ll_callback to be used with
OPTION_CALLBACK. The user will have to be careful, not to change
anything in ctx, or return wrong result code. But that's the price for
ll_callback.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-27 16:28:18 -08:00
Nguyễn Thái Ngọc Duy
f41179f16b parse-options: avoid magic return codes
Give names to these magic negative numbers. Make parse_opt_ll_cb
return an enum to make clear it can actually control parse_options()
with different return values (parse_opt_cb can too, but nobody needs
it).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-27 16:28:18 -08:00
Nguyễn Thái Ngọc Duy
bf3ff338a2 parse-options: stop abusing 'callback' for lowlevel callbacks
Lowlevel callbacks have different function signatures. Add a new field
in 'struct option' with the right type for lowlevel callbacks.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-27 16:28:18 -08:00
Nguyễn Thái Ngọc Duy
f62470c650 parse-options: add OPT_BITOP()
This is needed for diff_opt_parse() where we do

   value = (value & ~mask) | some_more;

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-27 16:28:18 -08:00
Nguyễn Thái Ngọc Duy
baa4adc66a parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN
parse-options can unambiguously find an abbreviation only if it sees
all available options. This is usually the case when you use
parse_options(). But there are other callers like blame or shortlog
which uses parse_options_start() in combination with a custom option
parser, like rev-list. parse-options cannot see all options in this
case and will get abbrev detection wrong. Disable it.

t7800 needs update because --symlink no longer expands to --symlinks
and will be passed down to git-diff, which will not recognize it. I
still think this is the correct thing to do. But if --symlink has been
actually used in the wild, we would just add an option alias for it.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-27 16:28:17 -08:00
Nguyễn Thái Ngọc Duy
202fbb3315 parse-options: add one-shot mode
This is to help reimplement diff_opt_parse() using parse_options().
The behavior of parse_options() is changed to be the same as the
other:

- no argv0 in argv[], everything can be processed
- argv[] must not be updated, it's the caller's job to do that
- return the number of arguments processed
- leave all unknown options / non-options alone (this one can already
  be achieved with PARSE_OPT_KEEP_UNKNOWN and
  PARSE_OPT_STOP_AT_NON_OPTION)

This mode is NOT supposed to stay here for long. It's to help
converting diff/rev option parsing. Once that work is over and we can
just use parse_options() throughout the code base, this will be
deleted.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-27 16:28:17 -08:00
Junio C Hamano
f2b6aa98be Merge branch 'nd/indentation-fix'
Code cleanup.

* nd/indentation-fix:
  Indent code with TABs
2019-01-14 15:29:32 -08:00
Junio C Hamano
3813a89fae Merge branch 'nd/i18n'
More _("i18n") markings.

* nd/i18n:
  fsck: mark strings for translation
  fsck: reduce word legos to help i18n
  parse-options.c: mark more strings for translation
  parse-options.c: turn some die() to BUG()
  parse-options: replace opterror() with optname()
  repack: mark more strings for translation
  remote.c: mark messages for translation
  remote.c: turn some error() or die() to BUG()
  reflog: mark strings for translation
  read-cache.c: add missing colon separators
  read-cache.c: mark more strings for translation
  read-cache.c: turn die("internal error") to BUG()
  attr.c: mark more string for translation
  archive.c: mark more strings for translation
  alias.c: mark split_cmdline_strerror() strings for translation
  git.c: mark more strings for translation
2019-01-04 13:33:31 -08:00
Junio C Hamano
bf29f074ed Merge branch 'nd/show-gitcomp-compilation-fix' into maint
Portability fix for a recent update to parse-options API.

* nd/show-gitcomp-compilation-fix:
  parse-options: fix SunCC compiler warning
2018-12-15 12:24:33 +09:00
Nguyễn Thái Ngọc Duy
a92ec7efe0 parse-options: fix SunCC compiler warning
The compiler reports this because show_gitcomp() never actually
returns a value:

    "parse-options.c", line 520: warning: Function has no return
    statement : show_gitcomp

We could shut the compiler up. But instead let's not bury exit() too
deep. Do the same as internal -h handling, return a special error code
and handle the exit() in parse_options() (and other
parse_options_step() callers) instead.

Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-12-12 17:21:33 +09:00
Nguyễn Thái Ngọc Duy
ec36c42a63 Indent code with TABs
We indent with TABs and sometimes for fine alignment, TABs followed by
spaces, but never all spaces (unless the indentation is less than 8
columns). Indenting with spaces slips through in some places. Fix
them.

Imported code and compat/ are left alone on purpose. The former should
remain as close as upstream as possible. The latter pretty much has
separate maintainers, it's up to them to decide.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-12-09 12:37:32 +09:00
Nguyễn Thái Ngọc Duy
8900342628 parse-options.c: mark more strings for translation
One error is updated to start with lowercase to be consistent with the
rest.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-12 14:47:10 +09:00
Nguyễn Thái Ngọc Duy
48a5499ef5 parse-options.c: turn some die() to BUG()
These two strings are clearly not for the user to see. Reduce the
violence in one string while at there.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-12 14:47:09 +09:00
Nguyễn Thái Ngọc Duy
9440b831ad parse-options: replace opterror() with optname()
Introduce optname() that does the early half of original opterror() to
come up with the name of the option reported back to the user, and use
it to kill opterror().  The callers of opterror() now directly call
error() using the string returned by opterror() instead.

There are a few issues with opterror()

- it tries to assemble an English sentence from pieces. This is not
  great for translators because we give them pieces instead of a full
  sentence.

- It's a wrapper around error() and needs some hack to let the
  compiler know it always returns -1.

- Since it takes a string instead of printf format, one call site has
  to assemble the string manually before passing to it.

Using error() directly solves the second and third problems.

It kind helps the first problem as well because "%s does foo" does
give a translator a full sentence in a sense and let them reorder if
needed. But it has limitations, if the subject part has to change
based on the rest of the sentence, that language is screwed. This is
also why I try to avoid calling optname() when 'flags' is known in
advance.

Mark of these strings for translation as well while at there.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-12 14:47:09 +09:00
Junio C Hamano
8963bb0c2d Merge branch 'rs/parse-opt-lithelp'
The parse-options machinery learned to refrain from enclosing
placeholder string inside a "<bra" and "ket>" pair automatically
without PARSE_OPT_LITERAL_ARGHELP.  Existing help text for option
arguments that are not formatted correctly have been identified and
fixed.

* rs/parse-opt-lithelp:
  parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP
  shortlog: correct option help for -w
  send-pack: specify --force-with-lease argument help explicitly
  pack-objects: specify --index-version argument help explicitly
  difftool: remove angular brackets from argument help
  add, update-index: fix --chmod argument help
  push: use PARSE_OPT_LITERAL_ARGHELP instead of unbalanced brackets
2018-08-17 13:09:56 -07:00
René Scharfe
5f0df44cd7 parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP
Parseopt wraps argument help strings in a pair of angular brackets by
default, to tell users that they need to replace it with an actual
value.  This is useful in most cases, because most option arguments
are indeed single values of a certain type.  The option
PARSE_OPT_LITERAL_ARGHELP needs to be used in option definitions with
arguments that have multiple parts or are literal strings.

Stop adding these angular brackets if special characters are present,
as they indicate that we don't deal with a simple placeholder.  This
simplifies the code a bit and makes defining special options slightly
easier.

Remove the flag PARSE_OPT_LITERAL_ARGHELP in the cases where the new
and more cautious handling suffices.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-03 08:36:20 -07:00
Nguyễn Thái Ngọc Duy
b221b5ab9b completion: collapse extra --no-.. options
The commands that make use of --git-completion-helper feature could
now produce a lot of --no-xxx options that a command can take. This in
many case could nearly double the amount of completable options, using
more screen estate and also harder to search for the wanted option.

This patch attempts to mitigate that by collapsing extra --no-
options, the ones that are added by --git-completion-helper and not in
original struct option arrays. The "--no-..." option will be displayed
in this case to hint about more options, e.g.

    > ~/w/git $ git clone --
    --bare                 --origin=
    --branch=              --progress
    --checkout             --quiet
    --config=              --recurse-submodules
    --depth=               --reference=
    --dissociate           --reference-if-able=
    --filter=              --separate-git-dir=
    --hardlinks            --shallow-exclude=
    --ipv4                 --shallow-since=
    --ipv6                 --shallow-submodules
    --jobs=                --shared
    --local                --single-branch
    --mirror               --tags
    --no-...               --template=
    --no-checkout          --upload-pack=
    --no-hardlinks         --verbose
    --no-tags

and when you complete it with --no-<tab>, all negative options will be
presented:

    > ~/w/git $ git clone --no-
    --no-bare                 --no-quiet
    --no-branch               --no-recurse-submodules
    --no-checkout             --no-reference
    --no-config               --no-reference-if-able
    --no-depth                --no-separate-git-dir
    --no-dissociate           --no-shallow-exclude
    --no-filter               --no-shallow-since
    --no-hardlinks            --no-shallow-submodules
    --no-ipv4                 --no-shared
    --no-ipv6                 --no-single-branch
    --no-jobs                 --no-tags
    --no-local                --no-template
    --no-mirror               --no-upload-pack
    --no-origin               --no-verbose
    --no-progress

Corner case: to make sure that people will never accidentally complete
the fake option "--no-..." there must be one real --no- in the first
complete listing even if it's not from the original struct option.

PS. This could could be made simpler with ";&" to fall through from
"--no-*" block and share the code but ";&" is not available on bash-3
(i.e. Mac)

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-11 10:38:10 -07:00
Nguyễn Thái Ngọc Duy
2b1c01d22e parse-options: option to let --git-completion-helper show negative form
When 7fb6aefd2a (Merge branch 'nd/parseopt-completion' - 2018-03-14)
is merged, the completion for negative form is left out because the
series is alread long and it could be done in a follow up series. This
is it.

--git-completion-helper now provides --no-xxx so that git-completion.bash
can drop the extra custom --no-xxx in the script. It adds a lot more
--no-xxx than what's current provided by the git-completion.bash
script. We'll trim that down later.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-29 13:28:44 +09:00
Junio C Hamano
62c0fd46a8 Merge branch 'ps/contains-id-error-message'
"git tag --contains no-such-commit" gave a full list of options
after giving an error message.

* ps/contains-id-error-message:
  parse-options: do not show usage upon invalid option value
2018-04-10 16:28:20 +09:00
Paul-Sebastian Ungureanu
3bb0923f06 parse-options: do not show usage upon invalid option value
Usually, the usage should be shown only if the user does not know what
options are available. If the user specifies an invalid value, the user
is already aware of the available options. In this case, there is no
point in displaying the usage anymore.

This patch applies to "git tag --contains", "git branch --contains",
"git branch --points-at", "git for-each-ref --contains" and many more.

Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-22 12:10:08 -07:00
Junio C Hamano
7fb6aefd2a Merge branch 'nd/parseopt-completion'
Teach parse-options API an option to help the completion script,
and make use of the mechanism in command line completion.

* nd/parseopt-completion: (45 commits)
  completion: more subcommands in _git_notes()
  completion: complete --{reuse,reedit}-message= for all notes subcmds
  completion: simplify _git_notes
  completion: don't set PARSE_OPT_NOCOMPLETE on --rerere-autoupdate
  completion: use __gitcomp_builtin in _git_worktree
  completion: use __gitcomp_builtin in _git_tag
  completion: use __gitcomp_builtin in _git_status
  completion: use __gitcomp_builtin in _git_show_branch
  completion: use __gitcomp_builtin in _git_rm
  completion: use __gitcomp_builtin in _git_revert
  completion: use __gitcomp_builtin in _git_reset
  completion: use __gitcomp_builtin in _git_replace
  remote: force completing --mirror= instead of --mirror
  completion: use __gitcomp_builtin in _git_remote
  completion: use __gitcomp_builtin in _git_push
  completion: use __gitcomp_builtin in _git_pull
  completion: use __gitcomp_builtin in _git_notes
  completion: use __gitcomp_builtin in _git_name_rev
  completion: use __gitcomp_builtin in _git_mv
  completion: use __gitcomp_builtin in _git_merge_base
  ...
2018-03-14 12:01:07 -07:00
Nguyễn Thái Ngọc Duy
ebc4a04e84 remote: force completing --mirror= instead of --mirror
"git remote --mirror" is a special case. Technically it is possible to
specify --mirror without any argument. But we will get a "dangerous,
deprecated!" warning in that case.

This new parse-opt flag allows --git-completion-helper to always
complete --mirror=, ignoring the dangerous use case.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-09 10:24:52 -08:00
Nguyễn Thái Ngọc Duy
b9d7f4b4db parse-options: support --git-completion-helper
This option is designed to be used by git-completion.bash. For many
simple cases, what we do in there is usually

    __gitcomp "lots of completion options"

which has to be manually updated when a new user-visible option is
added. With support from parse-options, we can write

    __gitcomp "$(git command --git-completion-helper)"

and get that list directly from the parser for free. Dangerous/Unpopular
options could be hidden with the new "NOCOMPLETE" flag.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-09 10:24:49 -08:00
SZEDER Gábor
f919ffebed Use MOVE_ARRAY
Use the helper macro MOVE_ARRAY to move arrays.  This is shorter and
safer, as it automatically infers the size of elements.

Patch generated by Coccinelle and contrib/coccinelle/array.cocci in
Travis CI's static analysis build job.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-22 11:32:51 -08:00
Brandon Casey
a6304fa4c2 parse-options: only insert newline in help text if needed
Currently, when parse_options() produces a help message it always emits
a blank line after the usage text to separate it from the options text.
If the option spec does not define any switches, or only defines hidden
switches that will not be displayed, then the help text will end up with
two trailing blank lines instead of one.  Let's defer emitting the blank
line between the usage text and the options text until it is clear that
the options section will not be empty.

Fixes t1502.5, t1502.6.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-25 14:35:53 +09:00
Brandon Casey
1a9bf1e176 parse-options: write blank line to correct output stream
When commit 54e6dc7 added translation support to parse-options, an
fprintf was mistakenly replaced by a call to putchar().  Let's use fputc
instead.

Fixes t0040.11, t0040.12, t0040.33, and t1502.8.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-25 14:35:52 +09:00
Junio C Hamano
f31d23a399 Merge branch 'bw/config-h'
Fix configuration codepath to pay proper attention to commondir
that is used in multi-worktree situation, and isolate config API
into its own header file.

* bw/config-h:
  config: don't implicitly use gitdir or commondir
  config: respect commondir
  setup: teach discover_git_directory to respect the commondir
  config: don't include config.h by default
  config: remove git_config_iter
  config: create config.h
2017-06-24 14:28:41 -07:00
Brandon Williams
b2141fc1d2 config: don't include config.h by default
Stop including config.h by default in cache.h.  Instead only include
config.h in those files which require use of the config system.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15 12:56:22 -07:00
Ævar Arnfjörð Bjarmason
66f5f6dca9 C style: use standard style for "TRANSLATORS" comments
Change all the "TRANSLATORS: [...]" comments in the C code to use the
regular Git coding style, and amend the style guide so that the
example there uses that style.

This custom style was necessary back in 2010 when the gettext support
was initially added, and was subsequently documented in commit
cbcfd4e3ea ("i18n: mention "TRANSLATORS:" marker in
Documentation/CodingGuidelines", 2014-04-18).

GNU xgettext hasn't had the parsing limitation that necessitated this
exception for almost 3 years. Since its 0.19 release on 2014-06-02
it's been able to recognize TRANSLATOR comments in the standard Git
comment syntax[1].

Usually we'd like to keep compatibility with software that's that
young, but in this case literally the only person who needs to be
using a gettext newer than 3 years old is Jiang Xin (the only person
who runs & commits "make pot" results), so I think in this case we can
make an exception.

This xgettext parsing feature was added after a thread on the Git
mailing list[2] which continued on the bug-gettext[3] list, but we
never subsequently changed our style & styleguide, do so.

There are already longstanding changes in git that use the standard
comment style & have their TRANSLATORS comments extracted properly
without getting the literal "*"'s mixed up in the text, as would
happen before xgettext 0.19.

Commit 7ff2683253 ("builtin-am: implement -i/--interactive",
2015-08-04) added one such comment, which in commit df0617bfa7 ("l10n:
git.pot: v2.6.0 round 1 (123 new, 41 removed)", 2015-09-05) got picked
up in the po/git.pot file with the right format, showing that Jiang
already runs a modern xgettext.

The xgettext parser does not handle the sort of non-standard comment
style that I'm amending here in sequencer.c, but that isn't standard
Git comment syntax anyway. With this change to sequencer.c & "make
pot" the comment in the pot file is now correct:

     #. TRANSLATORS: %s will be "revert", "cherry-pick" or
    -#. * "rebase -i".
    +#. "rebase -i".

1. http://git.savannah.gnu.org/cgit/gettext.git/commit/?id=10af7fe6bd
2. <2ce9ec406501d112e032c8208417f8100bed04c6.1397712142.git.worldhello.net@gmail.com>
   (https://public-inbox.org/git/2ce9ec406501d112e032c8208417f8100bed04c6.1397712142.git.worldhello.net@gmail.com/)
3. https://lists.gnu.org/archive/html/bug-gettext/2014-04/msg00016.html

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Acked-by: Jiang Xin <worldhello.net@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-31 08:01:42 +09:00
Jeff King
e4da43b1f0 prefix_filename: return newly allocated string
The prefix_filename() function returns a pointer to static
storage, which makes it easy to use dangerously. We already
fixed one buggy caller in hash-object recently, and the
calls in apply.c are suspicious (I didn't dig in enough to
confirm that there is a bug, but we call the function once
in apply_all_patches() and then again indirectly from
parse_chunk()).

Let's make it harder to get wrong by allocating the return
value. For simplicity, we'll do this even when the prefix is
empty (and we could just return the original file pointer).
That will cause us to allocate sometimes when we wouldn't
otherwise need to, but this function isn't called in
performance critical code-paths (and it already _might_
allocate on any given call, so a caller that cares about
performance is questionable anyway).

The downside is that the callers need to remember to free()
the result to avoid leaking. Most of them already used
xstrdup() on the result, so we know they are OK. The
remainder have been converted to use free() as appropriate.

I considered retaining a prefix_filename_unsafe() for cases
where we know the static lifetime is OK (and handling the
cleanup is awkward). This is only a handful of cases,
though, and it's not worth the mental energy in worrying
about whether the "unsafe" variant is OK to use in any
situation.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21 11:18:41 -07:00
Jeff King
116fb64e43 prefix_filename: drop length parameter
This function takes the prefix as a ptr/len pair, but in
every caller the length is exactly strlen(ptr). Let's
simplify the interface and just take the string. This saves
callers specifying it (and in some cases handling a NULL
prefix).

In a handful of cases we had the length already without
calling strlen, so this is technically slower. But it's not
likely to matter (after all, if the prefix is non-empty
we'll allocate and copy it into a buffer anyway).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21 11:12:53 -07:00