mirror of
https://github.com/git/git
synced 2024-08-24 18:26:02 +00:00
stash: eliminate crude option parsing
Eliminate crude option parsing and rely on real parsing instead, because
1) Crude parsing is crude, for example it's not capable of
handling things like `git stash -m Message`
2) Adding options in two places is inconvenient and prone to bugs
As a side result, the case of `git stash -m Message` gets fixed.
Also give a good error message instead of just throwing usage at user.
----
Some review of what's been happening to this code:
Before [1], `git-stash.sh` only verified that all args begin with `-` :
# The default command is "push" if nothing but options are given
seen_non_option=
for opt
do
case "$opt" in
--) break ;;
-*) ;;
*) seen_non_option=t; break ;;
esac
done
Later, [1] introduced the duplicate code I'm now removing, also making
the previous test more strict by white-listing options.
----
[1] Commit 40af1468
("stash: convert `stash--helper.c` into `stash.c`" 2019-02-26)
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
parent
3f3d8068f5
commit
8c3713cede
|
@ -1448,8 +1448,10 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
|
|||
return ret;
|
||||
}
|
||||
|
||||
static int push_stash(int argc, const char **argv, const char *prefix)
|
||||
static int push_stash(int argc, const char **argv, const char *prefix,
|
||||
int push_assumed)
|
||||
{
|
||||
int force_assume = 0;
|
||||
int keep_index = -1;
|
||||
int patch_mode = 0;
|
||||
int include_untracked = 0;
|
||||
|
@ -1471,10 +1473,22 @@ static int push_stash(int argc, const char **argv, const char *prefix)
|
|||
OPT_END()
|
||||
};
|
||||
|
||||
if (argc)
|
||||
if (argc) {
|
||||
force_assume = !strcmp(argv[0], "-p");
|
||||
argc = parse_options(argc, argv, prefix, options,
|
||||
git_stash_push_usage,
|
||||
0);
|
||||
PARSE_OPT_KEEP_DASHDASH);
|
||||
}
|
||||
|
||||
if (argc) {
|
||||
if (!strcmp(argv[0], "--")) {
|
||||
argc--;
|
||||
argv++;
|
||||
} else if (push_assumed && !force_assume) {
|
||||
die("subcommand wasn't specified; 'push' can't be assumed due to unexpected token '%s'",
|
||||
argv[0]);
|
||||
}
|
||||
}
|
||||
|
||||
parse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN,
|
||||
prefix, argv);
|
||||
|
@ -1547,7 +1561,6 @@ static int use_builtin_stash(void)
|
|||
|
||||
int cmd_stash(int argc, const char **argv, const char *prefix)
|
||||
{
|
||||
int i = -1;
|
||||
pid_t pid = getpid();
|
||||
const char *index_file;
|
||||
struct argv_array args = ARGV_ARRAY_INIT;
|
||||
|
@ -1580,7 +1593,7 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
|
|||
(uintmax_t)pid);
|
||||
|
||||
if (!argc)
|
||||
return !!push_stash(0, NULL, prefix);
|
||||
return !!push_stash(0, NULL, prefix, 0);
|
||||
else if (!strcmp(argv[0], "apply"))
|
||||
return !!apply_stash(argc, argv, prefix);
|
||||
else if (!strcmp(argv[0], "clear"))
|
||||
|
@ -1600,45 +1613,15 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
|
|||
else if (!strcmp(argv[0], "create"))
|
||||
return !!create_stash(argc, argv, prefix);
|
||||
else if (!strcmp(argv[0], "push"))
|
||||
return !!push_stash(argc, argv, prefix);
|
||||
return !!push_stash(argc, argv, prefix, 0);
|
||||
else if (!strcmp(argv[0], "save"))
|
||||
return !!save_stash(argc, argv, prefix);
|
||||
else if (*argv[0] != '-')
|
||||
usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
|
||||
git_stash_usage, options);
|
||||
|
||||
if (strcmp(argv[0], "-p")) {
|
||||
while (++i < argc && strcmp(argv[i], "--")) {
|
||||
/*
|
||||
* `akpqu` is a string which contains all short options,
|
||||
* except `-m` which is verified separately.
|
||||
*/
|
||||
if ((strlen(argv[i]) == 2) && *argv[i] == '-' &&
|
||||
strchr("akpqu", argv[i][1]))
|
||||
continue;
|
||||
|
||||
if (!strcmp(argv[i], "--all") ||
|
||||
!strcmp(argv[i], "--keep-index") ||
|
||||
!strcmp(argv[i], "--no-keep-index") ||
|
||||
!strcmp(argv[i], "--patch") ||
|
||||
!strcmp(argv[i], "--quiet") ||
|
||||
!strcmp(argv[i], "--include-untracked"))
|
||||
continue;
|
||||
|
||||
/*
|
||||
* `-m` and `--message=` are verified separately because
|
||||
* they need to be immediately followed by a string
|
||||
* (i.e.`-m"foobar"` or `--message="foobar"`).
|
||||
*/
|
||||
if (starts_with(argv[i], "-m") ||
|
||||
starts_with(argv[i], "--message="))
|
||||
continue;
|
||||
|
||||
usage_with_options(git_stash_usage, options);
|
||||
}
|
||||
}
|
||||
|
||||
/* Assume 'stash push' */
|
||||
argv_array_push(&args, "push");
|
||||
argv_array_pushv(&args, argv);
|
||||
return !!push_stash(args.argc, args.argv, prefix);
|
||||
return !!push_stash(args.argc, args.argv, prefix, 1);
|
||||
}
|
||||
|
|
|
@ -285,6 +285,11 @@ test_expect_success 'stash --no-keep-index' '
|
|||
test bar,bar2 = $(cat file),$(cat file2)
|
||||
'
|
||||
|
||||
test_expect_success 'dont assume push with non-option args' '
|
||||
test_must_fail git stash -q drop 2>err &&
|
||||
test_i18ngrep -e "subcommand wasn'\''t specified; '\''push'\'' can'\''t be assumed due to unexpected token '\''drop'\''" err
|
||||
'
|
||||
|
||||
test_expect_success 'stash --invalid-option' '
|
||||
echo bar5 >file &&
|
||||
echo bar6 >file2 &&
|
||||
|
|
Loading…
Reference in a new issue