development/git
development/git
1
0
Fork 0
mirror of https://github.com/git/git synced 2024-08-24 18:26:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

stash: eliminate crude option parsing

Browse source 
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:
Alexandr Miloslavskiy 2020-02-17 17:25:21 +00:00 committed by Junio C Hamano
parent 3f3d8068f5
commit 8c3713cede
2 changed files with 26 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

59
builtin/stash.c
View file

@ -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);
}

5
t/t3903-stash.sh
View file

@ -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 &&