[RFC] Introduce two new commands, switch-branch and restore-paths
diff mbox series

Message ID 20181120174554.GA29910@duynguyen.home
State New
Headers show
Series
  • [RFC] Introduce two new commands, switch-branch and restore-paths
Related show

Commit Message

Duy Nguyen Nov. 20, 2018, 5:45 p.m. UTC
On Mon, Nov 19, 2018 at 04:19:53PM +0100, Duy Nguyen wrote:
> I promise to come back with something better (at least it still
> sounds better in my mind). If that idea does not work out, we can
> come back and see if we can improve this.

So this is it. The patch isn't pretty, mostly as a proof of
concept. Just look at the three functions at the bottom of checkout.c,
which is the main thing.

This patch tries to split "git checkout" command in two new ones:

- git switch-branch is all about switching branches
- git restore-paths (maybe restore-file is better) for checking out
  paths

The main idea is these two commands will co-exist with the good old
'git checkout', which will NOT be deprecated. Old timers will still
use "git checkout". But new people should be introduced to the new two
instead. And the new ones are just as capable as "git checkout".

Since the three commands will co-exist (with duplicate functionality),
maintenance cost must be kept to minimum. The way I did this is simply
split the command line options into three pieces: common,
switch-branch and checkout-paths. "git checkout" has all three, the
other two have common and another piece.

With this, a new option added to git checkout will be automatically
available in either switch-branch or checkout-paths. Bug fixes apply
to all relevant commands.

Later on, we could start to add a bit more stuff in, e.g. some form of
disambiguation is no longer needed when running as switch-branch, or
restore-paths.

So, what do you think?

-- 8< --
-- 8< --

Comments

Thomas Gummerer Nov. 25, 2018, 10:20 p.m. UTC | #1
On 11/20, Duy Nguyen wrote:
> On Mon, Nov 19, 2018 at 04:19:53PM +0100, Duy Nguyen wrote:
> > I promise to come back with something better (at least it still
> > sounds better in my mind). If that idea does not work out, we can
> > come back and see if we can improve this.
> 
> So this is it. The patch isn't pretty, mostly as a proof of
> concept. Just look at the three functions at the bottom of checkout.c,
> which is the main thing.
> 
> This patch tries to split "git checkout" command in two new ones:
> 
> - git switch-branch is all about switching branches
> - git restore-paths (maybe restore-file is better) for checking out
>   paths
> 
> The main idea is these two commands will co-exist with the good old
> 'git checkout', which will NOT be deprecated. Old timers will still
> use "git checkout". But new people should be introduced to the new two
> instead. And the new ones are just as capable as "git checkout".
> 
> Since the three commands will co-exist (with duplicate functionality),
> maintenance cost must be kept to minimum. The way I did this is simply
> split the command line options into three pieces: common,
> switch-branch and checkout-paths. "git checkout" has all three, the
> other two have common and another piece.
>
> With this, a new option added to git checkout will be automatically
> available in either switch-branch or checkout-paths. Bug fixes apply
> to all relevant commands.
> 
> Later on, we could start to add a bit more stuff in, e.g. some form of
> disambiguation is no longer needed when running as switch-branch, or
> restore-paths.
> 
> So, what do you think?

I like the idea of splitting those commands up, in fact it is
something I've been considering working on myself.  I do think we
should consider if we want to change the behaviour of those new
commands in any way compared to 'git checkout', since we're starting
with a clean slate.

One thing in particular that I have in mind is something I'm currently
working on, namely adding a --index flag to 'git checkout', which
would make 'git checkout' work in non-overlay mode (for more
discussion on that see also [*1*].  I got something working, that
needs to be polished a bit and am hoping to send that to the list
sometime soon.

I wonder if such the --index behaviour could be the default in
restore-paths command?

Most of the underlying machinery for 'checkout' could and should of
course still be shared between the commands.

*1*: <xmqq4loqplou.fsf@gitster.mtv.corp.google.com>

> -- 8< --
> diff --git a/builtin.h b/builtin.h
> index 6538932e99..6e321ec8a4 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -214,6 +214,7 @@ extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);
>  extern int cmd_repack(int argc, const char **argv, const char *prefix);
>  extern int cmd_rerere(int argc, const char **argv, const char *prefix);
>  extern int cmd_reset(int argc, const char **argv, const char *prefix);
> +extern int cmd_restore_paths(int argc, const char **argv, const char *prefix);
>  extern int cmd_rev_list(int argc, const char **argv, const char *prefix);
>  extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);
>  extern int cmd_revert(int argc, const char **argv, const char *prefix);
> @@ -227,6 +228,7 @@ extern int cmd_show_index(int argc, const char **argv, const char *prefix);
>  extern int cmd_status(int argc, const char **argv, const char *prefix);
>  extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
>  extern int cmd_submodule__helper(int argc, const char **argv, const char *prefix);
> +extern int cmd_switch_branch(int argc, const char **argv, const char *prefix);
>  extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
>  extern int cmd_tag(int argc, const char **argv, const char *prefix);
>  extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index acdafc6e4c..868ca3c223 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -33,6 +33,16 @@ static const char * const checkout_usage[] = {
>  	NULL,
>  };
>  
> +static const char * const switch_branch_usage[] = {
> +	N_("git switch-branch [<options>] <branch>"),
> +	NULL,
> +};
> +
> +static const char * const restore_paths_usage[] = {
> +	N_("git restore-paths [<options>] [<branch>] -- <file>..."),
> +	NULL,
> +};
> +
>  struct checkout_opts {
>  	int patch_mode;
>  	int quiet;
> @@ -44,6 +54,7 @@ struct checkout_opts {
>  	int ignore_skipworktree;
>  	int ignore_other_worktrees;
>  	int show_progress;
> +	int dwim_new_local_branch;
>  	/*
>  	 * If new checkout options are added, skip_merge_working_tree
>  	 * should be updated accordingly.
> @@ -55,6 +66,7 @@ struct checkout_opts {
>  	int new_branch_log;
>  	enum branch_track track;
>  	struct diff_options diff_options;
> +	char *conflict_style;
>  
>  	int branch_exists;
>  	const char *prefix;
> @@ -1223,78 +1235,105 @@ static int checkout_branch(struct checkout_opts *opts,
>  	return switch_branches(opts, new_branch_info);
>  }
>  
> -int cmd_checkout(int argc, const char **argv, const char *prefix)
> +static struct option *add_common_options(struct checkout_opts *opts,
> +					 struct option *prevopts)
>  {
> -	struct checkout_opts opts;
> -	struct branch_info new_branch_info;
> -	char *conflict_style = NULL;
> -	int dwim_new_local_branch = 1;
> -	int dwim_remotes_matched = 0;
>  	struct option options[] = {
> -		OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
> -		OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
> +		OPT__QUIET(&opts->quiet, N_("suppress progress reporting")),
> +		OPT_BOOL(0, "ignore-skip-worktree-bits", &opts->ignore_skipworktree,
> +			 N_("do not limit pathspecs to sparse entries only")),
> +		{ OPTION_CALLBACK, 0, "recurse-submodules", NULL,
> +			    "checkout", "control recursive updating of submodules",
> +			    PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater },
> +		OPT_BOOL(0, "progress", &opts->show_progress, N_("force progress reporting")),
> +		OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"),
> +			   PARSE_OPT_NOCOMPLETE),
> +		OPT_STRING(0, "conflict", &opts->conflict_style, N_("style"),
> +			   N_("conflict style (merge or diff3)")),
> +		OPT_END()
> +	};
> +	struct option *newopts = parse_options_concat(prevopts, options);
> +	free(prevopts);
> +	return newopts;
> +}
> +
> +static struct option *add_switch_branch_options(struct checkout_opts *opts,
> +						struct option *prevopts)
> +{
> +	struct option options[] = {
> +		OPT_STRING('b', NULL, &opts->new_branch, N_("branch"),
>  			   N_("create and checkout a new branch")),
> -		OPT_STRING('B', NULL, &opts.new_branch_force, N_("branch"),
> +		OPT_STRING('B', NULL, &opts->new_branch_force, N_("branch"),
>  			   N_("create/reset and checkout a branch")),
> -		OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
> -		OPT_BOOL(0, "detach", &opts.force_detach, N_("detach HEAD at named commit")),
> -		OPT_SET_INT('t', "track",  &opts.track, N_("set upstream info for new branch"),
> +		OPT_BOOL('l', NULL, &opts->new_branch_log, N_("create reflog for new branch")),
> +		OPT_BOOL(0, "detach", &opts->force_detach, N_("detach HEAD at named commit")),
> +		OPT_SET_INT('t', "track",  &opts->track, N_("set upstream info for new branch"),
>  			BRANCH_TRACK_EXPLICIT),
> -		OPT_STRING(0, "orphan", &opts.new_orphan_branch, N_("new-branch"), N_("new unparented branch")),
> -		OPT_SET_INT_F('2', "ours", &opts.writeout_stage,
> +		OPT_STRING(0, "orphan", &opts->new_orphan_branch, N_("new-branch"), N_("new unparented branch")),
> +		OPT_BOOL('m', "merge", &opts->merge, N_("perform a 3-way merge with the new branch")),
> +		OPT_HIDDEN_BOOL(0, "guess", &opts->dwim_new_local_branch,
> +				N_("second guess 'git checkout <no-such-branch>'")),
> +		OPT_BOOL(0, "ignore-other-worktrees", &opts->ignore_other_worktrees,
> +			 N_("do not check if another worktree is holding the given ref")),
> +		OPT_END()
> +	};
> +	struct option *newopts = parse_options_concat(prevopts, options);
> +	free(prevopts);
> +	return newopts;
> +}
> +
> +static struct option *add_checkout_path_options(struct checkout_opts *opts,
> +						struct option *prevopts)
> +{
> +	struct option options[] = {
> +		OPT_SET_INT_F('2', "ours", &opts->writeout_stage,
>  			      N_("checkout our version for unmerged files"),
>  			      2, PARSE_OPT_NONEG),
> -		OPT_SET_INT_F('3', "theirs", &opts.writeout_stage,
> +		OPT_SET_INT_F('3', "theirs", &opts->writeout_stage,
>  			      N_("checkout their version for unmerged files"),
>  			      3, PARSE_OPT_NONEG),
> -		OPT__FORCE(&opts.force, N_("force checkout (throw away local modifications)"),
> -			   PARSE_OPT_NOCOMPLETE),
> -		OPT_BOOL('m', "merge", &opts.merge, N_("perform a 3-way merge with the new branch")),
> -		OPT_BOOL_F(0, "overwrite-ignore", &opts.overwrite_ignore,
> -			   N_("update ignored files (default)"),
> -			   PARSE_OPT_NOCOMPLETE),
> -		OPT_STRING(0, "conflict", &conflict_style, N_("style"),
> -			   N_("conflict style (merge or diff3)")),
> -		OPT_BOOL('p', "patch", &opts.patch_mode, N_("select hunks interactively")),
> -		OPT_BOOL(0, "ignore-skip-worktree-bits", &opts.ignore_skipworktree,
> -			 N_("do not limit pathspecs to sparse entries only")),
> -		OPT_HIDDEN_BOOL(0, "guess", &dwim_new_local_branch,
> -				N_("second guess 'git checkout <no-such-branch>'")),
> -		OPT_BOOL(0, "ignore-other-worktrees", &opts.ignore_other_worktrees,
> -			 N_("do not check if another worktree is holding the given ref")),
> -		{ OPTION_CALLBACK, 0, "recurse-submodules", NULL,
> -			    "checkout", "control recursive updating of submodules",
> -			    PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater },
> -		OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
> -		OPT_END(),
> +		OPT_BOOL('p', "patch", &opts->patch_mode, N_("select hunks interactively")),
> +		OPT_END()
>  	};
> +	struct option *newopts = parse_options_concat(prevopts, options);
> +	free(prevopts);
> +	return newopts;
> +}
>  
> -	memset(&opts, 0, sizeof(opts));
> +static int checkout_main(int argc, const char **argv, const char *prefix,
> +			 struct checkout_opts *opts, struct option *options,
> +			 const char * const usagestr[])
> +{
> +	struct branch_info new_branch_info;
> +	int dwim_remotes_matched = 0;
> +
> +	memset(opts, 0, sizeof(*opts));
> +	opts->dwim_new_local_branch = 1;
>  	memset(&new_branch_info, 0, sizeof(new_branch_info));
> -	opts.overwrite_ignore = 1;
> -	opts.prefix = prefix;
> -	opts.show_progress = -1;
> +	opts->overwrite_ignore = 1;
> +	opts->prefix = prefix;
> +	opts->show_progress = -1;
>  
> -	git_config(git_checkout_config, &opts);
> +	git_config(git_checkout_config, opts);
>  
> -	opts.track = BRANCH_TRACK_UNSPECIFIED;
> +	opts->track = BRANCH_TRACK_UNSPECIFIED;
>  
> -	argc = parse_options(argc, argv, prefix, options, checkout_usage,
> +	argc = parse_options(argc, argv, prefix, options, usagestr,
>  			     PARSE_OPT_KEEP_DASHDASH);
>  
> -	if (opts.show_progress < 0) {
> -		if (opts.quiet)
> -			opts.show_progress = 0;
> +	if (opts->show_progress < 0) {
> +		if (opts->quiet)
> +			opts->show_progress = 0;
>  		else
> -			opts.show_progress = isatty(2);
> +			opts->show_progress = isatty(2);
>  	}
>  
> -	if (conflict_style) {
> -		opts.merge = 1; /* implied */
> -		git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
> +	if (opts->conflict_style) {
> +		opts->merge = 1; /* implied */
> +		git_xmerge_config("merge.conflictstyle", opts->conflict_style, NULL);
>  	}
>  
> -	if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)
> +	if ((!!opts->new_branch + !!opts->new_branch_force + !!opts->new_orphan_branch) > 1)
>  		die(_("-b, -B and --orphan are mutually exclusive"));
>  
>  	/*
> @@ -1302,14 +1341,14 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  	 * and new_branch_force and new_orphan_branch will tell us which one of
>  	 * -b/-B/--orphan is being used.
>  	 */
> -	if (opts.new_branch_force)
> -		opts.new_branch = opts.new_branch_force;
> +	if (opts->new_branch_force)
> +		opts->new_branch = opts->new_branch_force;
>  
> -	if (opts.new_orphan_branch)
> -		opts.new_branch = opts.new_orphan_branch;
> +	if (opts->new_orphan_branch)
> +		opts->new_branch = opts->new_orphan_branch;
>  
>  	/* --track without -b/-B/--orphan should DWIM */
> -	if (opts.track != BRANCH_TRACK_UNSPECIFIED && !opts.new_branch) {
> +	if (opts->track != BRANCH_TRACK_UNSPECIFIED && !opts->new_branch) {
>  		const char *argv0 = argv[0];
>  		if (!argc || !strcmp(argv0, "--"))
>  			die(_("--track needs a branch name"));
> @@ -1318,7 +1357,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  		argv0 = strchr(argv0, '/');
>  		if (!argv0 || !argv0[1])
>  			die(_("missing branch name; try -b"));
> -		opts.new_branch = argv0 + 1;
> +		opts->new_branch = argv0 + 1;
>  	}
>  
>  	/*
> @@ -1337,56 +1376,56 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  	if (argc) {
>  		struct object_id rev;
>  		int dwim_ok =
> -			!opts.patch_mode &&
> -			dwim_new_local_branch &&
> -			opts.track == BRANCH_TRACK_UNSPECIFIED &&
> -			!opts.new_branch;
> +			!opts->patch_mode &&
> +			opts->dwim_new_local_branch &&
> +			opts->track == BRANCH_TRACK_UNSPECIFIED &&
> +			!opts->new_branch;
>  		int n = parse_branchname_arg(argc, argv, dwim_ok,
> -					     &new_branch_info, &opts, &rev,
> +					     &new_branch_info, opts, &rev,
>  					     &dwim_remotes_matched);
>  		argv += n;
>  		argc -= n;
>  	}
>  
>  	if (argc) {
> -		parse_pathspec(&opts.pathspec, 0,
> -			       opts.patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,
> +		parse_pathspec(&opts->pathspec, 0,
> +			       opts->patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,
>  			       prefix, argv);
>  
> -		if (!opts.pathspec.nr)
> +		if (!opts->pathspec.nr)
>  			die(_("invalid path specification"));
>  
>  		/*
>  		 * Try to give more helpful suggestion.
>  		 * new_branch && argc > 1 will be caught later.
>  		 */
> -		if (opts.new_branch && argc == 1)
> +		if (opts->new_branch && argc == 1)
>  			die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
> -				argv[0], opts.new_branch);
> +				argv[0], opts->new_branch);
>  
> -		if (opts.force_detach)
> +		if (opts->force_detach)
>  			die(_("git checkout: --detach does not take a path argument '%s'"),
>  			    argv[0]);
>  
> -		if (1 < !!opts.writeout_stage + !!opts.force + !!opts.merge)
> +		if (1 < !!opts->writeout_stage + !!opts->force + !!opts->merge)
>  			die(_("git checkout: --ours/--theirs, --force and --merge are incompatible when\n"
>  			      "checking out of the index."));
>  	}
>  
> -	if (opts.new_branch) {
> +	if (opts->new_branch) {
>  		struct strbuf buf = STRBUF_INIT;
>  
> -		if (opts.new_branch_force)
> -			opts.branch_exists = validate_branchname(opts.new_branch, &buf);
> +		if (opts->new_branch_force)
> +			opts->branch_exists = validate_branchname(opts->new_branch, &buf);
>  		else
> -			opts.branch_exists =
> -				validate_new_branchname(opts.new_branch, &buf, 0);
> +			opts->branch_exists =
> +				validate_new_branchname(opts->new_branch, &buf, 0);
>  		strbuf_release(&buf);
>  	}
>  
>  	UNLEAK(opts);
> -	if (opts.patch_mode || opts.pathspec.nr) {
> -		int ret = checkout_paths(&opts, new_branch_info.name);
> +	if (opts->patch_mode || opts->pathspec.nr) {
> +		int ret = checkout_paths(opts, new_branch_info.name);
>  		if (ret && dwim_remotes_matched > 1 &&
>  		    advice_checkout_ambiguous_remote_branch_name)
>  			advise(_("'%s' matched more than one remote tracking branch.\n"
> @@ -1405,6 +1444,49 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  			       dwim_remotes_matched);
>  		return ret;
>  	} else {
> -		return checkout_branch(&opts, &new_branch_info);
> +		return checkout_branch(opts, &new_branch_info);
>  	}
>  }
> +
> +int cmd_checkout(int argc, const char **argv, const char *prefix)
> +{
> +	struct checkout_opts opts;
> +	struct option *options = NULL;
> +	int ret;
> +
> +	options = add_common_options(&opts, options);
> +	options = add_switch_branch_options(&opts, options);
> +	options = add_checkout_path_options(&opts, options);
> +	ret = checkout_main(argc, argv, prefix, &opts,
> +			    options, checkout_usage);
> +	FREE_AND_NULL(options);
> +	return ret;
> +}
> +
> +int cmd_switch_branch(int argc, const char **argv, const char *prefix)
> +{
> +	struct checkout_opts opts;
> +	struct option *options = NULL;
> +	int ret;
> +
> +	options = add_common_options(&opts, options);
> +	options = add_switch_branch_options(&opts, options);
> +	ret = checkout_main(argc, argv, prefix, &opts,
> +			    options, switch_branch_usage);
> +	FREE_AND_NULL(options);
> +	return ret;
> +}
> +
> +int cmd_restore_paths(int argc, const char **argv, const char *prefix)
> +{
> +	struct checkout_opts opts;
> +	struct option *options = NULL;
> +	int ret;
> +
> +	options = add_common_options(&opts, options);
> +	options = add_checkout_path_options(&opts, options);
> +	ret = checkout_main(argc, argv, prefix, &opts,
> +			    options, restore_paths_usage);
> +	FREE_AND_NULL(options);
> +	return ret;
> +}
> diff --git a/git.c b/git.c
> index 2f604a41ea..e8a76a99da 100644
> --- a/git.c
> +++ b/git.c
> @@ -542,6 +542,7 @@ static struct cmd_struct commands[] = {
>  	{ "replace", cmd_replace, RUN_SETUP },
>  	{ "rerere", cmd_rerere, RUN_SETUP },
>  	{ "reset", cmd_reset, RUN_SETUP },
> +	{ "restore-paths", cmd_restore_paths, RUN_SETUP | NEED_WORK_TREE },
>  	{ "rev-list", cmd_rev_list, RUN_SETUP | NO_PARSEOPT },
>  	{ "rev-parse", cmd_rev_parse, NO_PARSEOPT },
>  	{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
> @@ -557,6 +558,7 @@ static struct cmd_struct commands[] = {
>  	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
>  	{ "stripspace", cmd_stripspace },
>  	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX | NO_PARSEOPT },
> +	{ "switch-branch", cmd_switch_branch, RUN_SETUP | NEED_WORK_TREE },
>  	{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
>  	{ "tag", cmd_tag, RUN_SETUP | DELAY_PAGER_CONFIG },
>  	{ "unpack-file", cmd_unpack_file, RUN_SETUP | NO_PARSEOPT },
> diff --git a/parse-options-cb.c b/parse-options-cb.c
> index 8c9edce52f..c609d52926 100644
> --- a/parse-options-cb.c
> +++ b/parse-options-cb.c
> @@ -126,7 +126,7 @@ struct option *parse_options_concat(struct option *a, struct option *b)
>  	struct option *ret;
>  	size_t i, a_len = 0, b_len = 0;
>  
> -	for (i = 0; a[i].type != OPTION_END; i++)
> +	for (i = 0; a && a[i].type != OPTION_END; i++)
>  		a_len++;
>  	for (i = 0; b[i].type != OPTION_END; i++)
>  		b_len++;
> -- 8< --
Junio C Hamano Nov. 26, 2018, 3:03 a.m. UTC | #2
Thomas Gummerer <t.gummerer@gmail.com> writes:

> I like the idea of splitting those commands up, in fact it is
> something I've been considering working on myself.  I do think we
> should consider if we want to change the behaviour of those new
> commands in any way compared to 'git checkout', since we're starting
> with a clean slate.
>
> One thing in particular that I have in mind is something I'm currently
> working on, namely adding a --index flag to 'git checkout', which
> would make 'git checkout' work in non-overlay mode (for more
> discussion on that see also [*1*].

Ah, thanks for reminding me of that.  That explains why I felt
uneasy to see "restore" in the proposed command name.  In short, I
think "checkout --index <tree> <pathspec>", i.e. if the <pathspec>
matches a directory in <tree> and the current index and/or the
working tree has tracked paths in that directory that do not exist
in <tree>, the operation _removes_ these paths so that the result
matches <tree>, should become the default of "I want to check out
these paths from the named tree-ish".  The current one is not
exactly "checking out the paths" in that it ignores and does not
check out the absense of paths in <tree>.  That operation sounds
more like "restoring paths out of a given tree".  If the tree does
not have some paths, these paths won't be "restored" from that tree,
so "restore" matches the current "overlay what's taken out of the
given tree on top of what is already in the index and the working
tree, without checking out the absense of paths" better from that
point of view.
Duy Nguyen Nov. 26, 2018, 3:37 p.m. UTC | #3
On Mon, Nov 26, 2018 at 4:03 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > I like the idea of splitting those commands up, in fact it is
> > something I've been considering working on myself.  I do think we
> > should consider if we want to change the behaviour of those new
> > commands in any way compared to 'git checkout', since we're starting
> > with a clean slate.

Better defaults? Hell yes!

> > One thing in particular that I have in mind is something I'm currently
> > working on, namely adding a --index flag to 'git checkout', which
> > would make 'git checkout' work in non-overlay mode (for more
> > discussion on that see also [*1*].
>
> Ah, thanks for reminding me of that.  That explains why I felt
> uneasy to see "restore" in the proposed command name.

About that name. I didn't want to start the command name with checkout
to avoid completion conflict (the obvious choice was checkout-path,
the function name behind it). And I didn't find any other good name,
so I picked "restore" out of git-checkout.txt's one-line description.
If we end up with a better command name, perhaps reword that line too.
Ævar Arnfjörð Bjarmason Nov. 26, 2018, 4 p.m. UTC | #4
On Tue, Nov 20 2018, Duy Nguyen wrote:

> On Mon, Nov 19, 2018 at 04:19:53PM +0100, Duy Nguyen wrote:
>> I promise to come back with something better (at least it still
>> sounds better in my mind). If that idea does not work out, we can
>> come back and see if we can improve this.
>
> So this is it. The patch isn't pretty, mostly as a proof of
> concept. Just look at the three functions at the bottom of checkout.c,
> which is the main thing.
>
> This patch tries to split "git checkout" command in two new ones:
>
> - git switch-branch is all about switching branches

Isn't this going to also take the other ref arguments 'git checkout'
takes now? I.e. tags, detached HEADs etc? I'm reminded of the discussion
about what "range-diff" should be called :)

> - git restore-paths (maybe restore-file is better) for checking out
>   paths

If it takes globs/dirs then a plural is probably better.

> The main idea is these two commands will co-exist with the good old
> 'git checkout', which will NOT be deprecated. Old timers will still
> use "git checkout". But new people should be introduced to the new two
> instead. And the new ones are just as capable as "git checkout".
>
> Since the three commands will co-exist (with duplicate functionality),
> maintenance cost must be kept to minimum. The way I did this is simply
> split the command line options into three pieces: common,
> switch-branch and checkout-paths. "git checkout" has all three, the
> other two have common and another piece.
>
> With this, a new option added to git checkout will be automatically
> available in either switch-branch or checkout-paths. Bug fixes apply
> to all relevant commands.
>
> Later on, we could start to add a bit more stuff in, e.g. some form of
> disambiguation is no longer needed when running as switch-branch, or
> restore-paths.
>
> So, what do you think?

That "git checkout" does too many things is something that keeps coming
up in online discussions about Git's UI. Two things:

a) It would really help to have some comparison of cases where these
   split commands are much clearer or less ambiguous than
   git-checkout. I can think of some (e.g. branch with the same name as
   a file) but having some overall picture of what the new UI looks like
   with solved / not solved cases would be nice. Also a comparison with
   other SCMs people find less confusing (svn, hg, bzr, ...)

b) I think we really need to have some end-game where we'd actually
   switch away from "checkout" (which we could still auto-route to new
   commands in perpetuity, but print a warning or error). Otherwise
   we'll just end up with https://xkcd.com/927/ and more UI confusion
   for all.
Duy Nguyen Nov. 26, 2018, 4:08 p.m. UTC | #5
On Mon, Nov 26, 2018 at 5:01 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> > So, what do you think?
>
> That "git checkout" does too many things is something that keeps coming
> up in online discussions about Git's UI. Two things:
>
> a) It would really help to have some comparison of cases where these
>    split commands are much clearer or less ambiguous than
>    git-checkout. I can think of some (e.g. branch with the same name as
>    a file) but having some overall picture of what the new UI looks like
>    with solved / not solved cases would be nice. Also a comparison with
>    other SCMs people find less confusing (svn, hg, bzr, ...)

Less ambiguous is indeed one of the reasons I wanted to do this.

> b) I think we really need to have some end-game where we'd actually
>    switch away from "checkout" (which we could still auto-route to new
>    commands in perpetuity, but print a warning or error). Otherwise
>    we'll just end up with https://xkcd.com/927/ and more UI confusion
>    for all.

I'm not going to remove "git checkout". Not until the majority of
users are really in favor of the new ones. So my end-game plan is to
just promote the two new commands in man pages, tutorial, advice...
Perhaps at some point "git checkout" is also removed from "git help".
But that's about it. If you want to stick with "git checkout", it's
there (and not nagging you to move to new ones).
Stefan Beller Nov. 26, 2018, 11:10 p.m. UTC | #6
On Mon, Nov 26, 2018 at 8:01 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Tue, Nov 20 2018, Duy Nguyen wrote:
>
> > On Mon, Nov 19, 2018 at 04:19:53PM +0100, Duy Nguyen wrote:
> >> I promise to come back with something better (at least it still
> >> sounds better in my mind). If that idea does not work out, we can
> >> come back and see if we can improve this.
> >
> > So this is it. The patch isn't pretty, mostly as a proof of
> > concept. Just look at the three functions at the bottom of checkout.c,
> > which is the main thing.
> >
> > This patch tries to split "git checkout" command in two new ones:
> >
> > - git switch-branch is all about switching branches
>
> Isn't this going to also take the other ref arguments 'git checkout'
> takes now? I.e. tags, detached HEADs etc? I'm reminded of the discussion
> about what "range-diff" should be called :)

Heh, good call. :-)
Note that the color of a bikeshed has fewer functional implications
than coming up with proper names in your API exposed to millions
of users, as cognitive associations playing mind tricks, can have a
huge impact on their productivity. ;-)

In a neighboring thread there is discussion of the concept of a
'change' (and evolving the change locally), which is yet another
thing in the refs-space.

'switch-branch' sounds like a good name for a beginner who is just
getting started, but as soon as they discover that there is more than
branches (detached HEAD via commits, tags,
remote tracking "branches"), this name may be confusing.
So it would not be a good choice for the intermediate Git user.
The old time power user would not care as they have 'checkout'
in their muscle memory, aliased to 'co', but maybe they'd find
it nice for explaining to new users.

So I'd be thrilled to find a name that serves users on all levels.

Maybe we need to step back and consider what the command does.
And from that I would name it "rewire-HEAD-and-update-index&worktree"
and then simplify from there. For the beginner user, the concept of
HEAD might be overwhelming, such that we don't want to have that
in there.

So I'd be tempted to suggest to call it "switch-to-ref", but that would
be wrong in the corner case as well: When using that with a remote
tracking branch, you don't "switch to it" by putting it into your HEAD,
but you merely checkout the commit that it's pointing at.



>
> > - git restore-paths (maybe restore-file is better) for checking out
> >   paths

"content-to-path", maybe(?) as it moves the content (as given by commit
or implicitly assuming the index when omitted) into that path(, again).
(I am not enthused about this, as you can similarly argue for
content-to-paths, content-to-worktree, which then could split up into
"index-to-worktree [pathspec]" as well as "tree-to-worktree <commit>".
also the notion of X-to-Y seems a novel concept in our naming, so maybe
verb-noun is better, hence restore-path or "fix-paths" may be better)

> > Later on, we could start to add a bit more stuff in, e.g. some form of
> > disambiguation is no longer needed when running as switch-branch, or
> > restore-paths.
> >
> > So, what do you think?

The patch looks interestingly small :-)

> That "git checkout" does too many things is something that keeps coming
> up in online discussions about Git's UI. Two things:
>
> a) It would really help to have some comparison of cases where these
>    split commands are much clearer or less ambiguous than
>    git-checkout. I can think of some (e.g. branch with the same name as
>    a file) but having some overall picture of what the new UI looks like
>    with solved / not solved cases would be nice. Also a comparison with
>    other SCMs people find less confusing (svn, hg, bzr, ...)

How do other SCMs solve this issue? (What is their design space?
How many commands do they have for what git-checkout does
all-in-one?)

> b) I think we really need to have some end-game where we'd actually
>    switch away from "checkout" (which we could still auto-route to new
>    commands in perpetuity, but print a warning or error). Otherwise
>    we'll just end up with https://xkcd.com/927/ and more UI confusion
>    for all.

Heh, that situation is only avoided when the new command has clear
advantages over the old, and ISTM that we can only compete on
UX and better defaults, so maybe I'd push for making it more logical,
maybe so:

  git tree-to-worktree # git checkout <commit> -- <path>
  git index-to-worktree # git checkout -- <path>
  git rev-to-ref # git checkout <commit>

Just food for thought, specifically the last one would be
hilarious if we'd end up with it.

Stefan
Junio C Hamano Nov. 27, 2018, 12:34 a.m. UTC | #7
Stefan Beller <sbeller@google.com> writes:

> Maybe we need to step back and consider what the command does.
> And from that I would name it "rewire-HEAD-and-update-index&worktree"
> and then simplify from there. For the beginner user, the concept of
> HEAD might be overwhelming, such that we don't want to have that
> in there.

I'd have to say that it is totally backwards.

Use of HEAD, ref, etc. is merely a means to what the end users want
to achieve, which is to switch to a "branch" (in air quotes, as the
word in this sentence means a bit broader than "a ref that is in
refs/heads/"), to choose which lineage of commits to work on to grow
or reshape the history.  Most of the time, you would be working on a
branch (that is a ref that is in refs/heads/), sometimes you would
be working on an unnamed "branch" (i.e. detached HEAD state, only
difference between being on it and a normal branch is whether it is
named---the history manipulation can happen exactly the same way).

In other words, your initial motivation of stepping back and
thinking about what the command is about is very good.  But in the
context of checking out a branch, the concept the end user works
with are at the level of "branch", not HEAD, ref, index, working
tree (all of which are underlying implementation details that let
you work on manipulating the history, represented by the "branch").

> "content-to-path", maybe(?) ...

Path is not a place.  A path t/Makefile is a shared name of a place
in a tree, the index, or in the working tree.  Renaming "git add" to
"content-to-path" because it adds the content to the index at path
may be equally OK, but it misses the essense (which is that it is to
add to the index and not to anythng else like a tree or the working
tree).

I actually think the easiest-to-understand shorthand for the
operation "checking out the contents at the path to the working
tree" is "checkout".  

These...

>   git tree-to-worktree # git checkout <commit> -- <path>
>   git index-to-worktree # git checkout -- <path>

...are interesting and worth learning lessons from.  These would be
something people would suggest when users start making noises about
"restore-to-path" or "content-to-path" overloads three different
operations and is confusing.

I think "restore-to-path" that can take contents for individual
paths out of either a treeish or the index and update the index and
the working tree, depending on what the user tells it to do, is not
confusing to the end users.  At the conceptual level, the users need
to have a mental model that has three places (tree-ish, index and
working tree) that can hold contents to make use of these
operations, so it is only the matter of how to express it clearly
and concisely.

For that matter, "checkout <branch>", "checkout <treeish> -- <path>"
and "checkout -- <path>" already is a trio of clear and cocncise way
to spell three distinct operations, so with the right mental model,
it may not be so confusing to the users.  And "checkout-branch",
"checkout-contents-from-tree", and "checkout-contents-from-index"
longhands may be a good way to help new users form the right mental
model, as they are more explicit in their names; once they form the
right mental model (that is, there are three places the contents
live, and there are a few operations to move contents from the two
places to the working tree, i.e. what traditionally is called
"checking things out"), they may gradulate to the shorthand form.

Patch
diff mbox series

diff --git a/builtin.h b/builtin.h
index 6538932e99..6e321ec8a4 100644
--- a/builtin.h
+++ b/builtin.h
@@ -214,6 +214,7 @@  extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);
 extern int cmd_repack(int argc, const char **argv, const char *prefix);
 extern int cmd_rerere(int argc, const char **argv, const char *prefix);
 extern int cmd_reset(int argc, const char **argv, const char *prefix);
+extern int cmd_restore_paths(int argc, const char **argv, const char *prefix);
 extern int cmd_rev_list(int argc, const char **argv, const char *prefix);
 extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);
 extern int cmd_revert(int argc, const char **argv, const char *prefix);
@@ -227,6 +228,7 @@  extern int cmd_show_index(int argc, const char **argv, const char *prefix);
 extern int cmd_status(int argc, const char **argv, const char *prefix);
 extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
 extern int cmd_submodule__helper(int argc, const char **argv, const char *prefix);
+extern int cmd_switch_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_tag(int argc, const char **argv, const char *prefix);
 extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index acdafc6e4c..868ca3c223 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -33,6 +33,16 @@  static const char * const checkout_usage[] = {
 	NULL,
 };
 
+static const char * const switch_branch_usage[] = {
+	N_("git switch-branch [<options>] <branch>"),
+	NULL,
+};
+
+static const char * const restore_paths_usage[] = {
+	N_("git restore-paths [<options>] [<branch>] -- <file>..."),
+	NULL,
+};
+
 struct checkout_opts {
 	int patch_mode;
 	int quiet;
@@ -44,6 +54,7 @@  struct checkout_opts {
 	int ignore_skipworktree;
 	int ignore_other_worktrees;
 	int show_progress;
+	int dwim_new_local_branch;
 	/*
 	 * If new checkout options are added, skip_merge_working_tree
 	 * should be updated accordingly.
@@ -55,6 +66,7 @@  struct checkout_opts {
 	int new_branch_log;
 	enum branch_track track;
 	struct diff_options diff_options;
+	char *conflict_style;
 
 	int branch_exists;
 	const char *prefix;
@@ -1223,78 +1235,105 @@  static int checkout_branch(struct checkout_opts *opts,
 	return switch_branches(opts, new_branch_info);
 }
 
-int cmd_checkout(int argc, const char **argv, const char *prefix)
+static struct option *add_common_options(struct checkout_opts *opts,
+					 struct option *prevopts)
 {
-	struct checkout_opts opts;
-	struct branch_info new_branch_info;
-	char *conflict_style = NULL;
-	int dwim_new_local_branch = 1;
-	int dwim_remotes_matched = 0;
 	struct option options[] = {
-		OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
-		OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
+		OPT__QUIET(&opts->quiet, N_("suppress progress reporting")),
+		OPT_BOOL(0, "ignore-skip-worktree-bits", &opts->ignore_skipworktree,
+			 N_("do not limit pathspecs to sparse entries only")),
+		{ OPTION_CALLBACK, 0, "recurse-submodules", NULL,
+			    "checkout", "control recursive updating of submodules",
+			    PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater },
+		OPT_BOOL(0, "progress", &opts->show_progress, N_("force progress reporting")),
+		OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"),
+			   PARSE_OPT_NOCOMPLETE),
+		OPT_STRING(0, "conflict", &opts->conflict_style, N_("style"),
+			   N_("conflict style (merge or diff3)")),
+		OPT_END()
+	};
+	struct option *newopts = parse_options_concat(prevopts, options);
+	free(prevopts);
+	return newopts;
+}
+
+static struct option *add_switch_branch_options(struct checkout_opts *opts,
+						struct option *prevopts)
+{
+	struct option options[] = {
+		OPT_STRING('b', NULL, &opts->new_branch, N_("branch"),
 			   N_("create and checkout a new branch")),
-		OPT_STRING('B', NULL, &opts.new_branch_force, N_("branch"),
+		OPT_STRING('B', NULL, &opts->new_branch_force, N_("branch"),
 			   N_("create/reset and checkout a branch")),
-		OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")),
-		OPT_BOOL(0, "detach", &opts.force_detach, N_("detach HEAD at named commit")),
-		OPT_SET_INT('t', "track",  &opts.track, N_("set upstream info for new branch"),
+		OPT_BOOL('l', NULL, &opts->new_branch_log, N_("create reflog for new branch")),
+		OPT_BOOL(0, "detach", &opts->force_detach, N_("detach HEAD at named commit")),
+		OPT_SET_INT('t', "track",  &opts->track, N_("set upstream info for new branch"),
 			BRANCH_TRACK_EXPLICIT),
-		OPT_STRING(0, "orphan", &opts.new_orphan_branch, N_("new-branch"), N_("new unparented branch")),
-		OPT_SET_INT_F('2', "ours", &opts.writeout_stage,
+		OPT_STRING(0, "orphan", &opts->new_orphan_branch, N_("new-branch"), N_("new unparented branch")),
+		OPT_BOOL('m', "merge", &opts->merge, N_("perform a 3-way merge with the new branch")),
+		OPT_HIDDEN_BOOL(0, "guess", &opts->dwim_new_local_branch,
+				N_("second guess 'git checkout <no-such-branch>'")),
+		OPT_BOOL(0, "ignore-other-worktrees", &opts->ignore_other_worktrees,
+			 N_("do not check if another worktree is holding the given ref")),
+		OPT_END()
+	};
+	struct option *newopts = parse_options_concat(prevopts, options);
+	free(prevopts);
+	return newopts;
+}
+
+static struct option *add_checkout_path_options(struct checkout_opts *opts,
+						struct option *prevopts)
+{
+	struct option options[] = {
+		OPT_SET_INT_F('2', "ours", &opts->writeout_stage,
 			      N_("checkout our version for unmerged files"),
 			      2, PARSE_OPT_NONEG),
-		OPT_SET_INT_F('3', "theirs", &opts.writeout_stage,
+		OPT_SET_INT_F('3', "theirs", &opts->writeout_stage,
 			      N_("checkout their version for unmerged files"),
 			      3, PARSE_OPT_NONEG),
-		OPT__FORCE(&opts.force, N_("force checkout (throw away local modifications)"),
-			   PARSE_OPT_NOCOMPLETE),
-		OPT_BOOL('m', "merge", &opts.merge, N_("perform a 3-way merge with the new branch")),
-		OPT_BOOL_F(0, "overwrite-ignore", &opts.overwrite_ignore,
-			   N_("update ignored files (default)"),
-			   PARSE_OPT_NOCOMPLETE),
-		OPT_STRING(0, "conflict", &conflict_style, N_("style"),
-			   N_("conflict style (merge or diff3)")),
-		OPT_BOOL('p', "patch", &opts.patch_mode, N_("select hunks interactively")),
-		OPT_BOOL(0, "ignore-skip-worktree-bits", &opts.ignore_skipworktree,
-			 N_("do not limit pathspecs to sparse entries only")),
-		OPT_HIDDEN_BOOL(0, "guess", &dwim_new_local_branch,
-				N_("second guess 'git checkout <no-such-branch>'")),
-		OPT_BOOL(0, "ignore-other-worktrees", &opts.ignore_other_worktrees,
-			 N_("do not check if another worktree is holding the given ref")),
-		{ OPTION_CALLBACK, 0, "recurse-submodules", NULL,
-			    "checkout", "control recursive updating of submodules",
-			    PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater },
-		OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
-		OPT_END(),
+		OPT_BOOL('p', "patch", &opts->patch_mode, N_("select hunks interactively")),
+		OPT_END()
 	};
+	struct option *newopts = parse_options_concat(prevopts, options);
+	free(prevopts);
+	return newopts;
+}
 
-	memset(&opts, 0, sizeof(opts));
+static int checkout_main(int argc, const char **argv, const char *prefix,
+			 struct checkout_opts *opts, struct option *options,
+			 const char * const usagestr[])
+{
+	struct branch_info new_branch_info;
+	int dwim_remotes_matched = 0;
+
+	memset(opts, 0, sizeof(*opts));
+	opts->dwim_new_local_branch = 1;
 	memset(&new_branch_info, 0, sizeof(new_branch_info));
-	opts.overwrite_ignore = 1;
-	opts.prefix = prefix;
-	opts.show_progress = -1;
+	opts->overwrite_ignore = 1;
+	opts->prefix = prefix;
+	opts->show_progress = -1;
 
-	git_config(git_checkout_config, &opts);
+	git_config(git_checkout_config, opts);
 
-	opts.track = BRANCH_TRACK_UNSPECIFIED;
+	opts->track = BRANCH_TRACK_UNSPECIFIED;
 
-	argc = parse_options(argc, argv, prefix, options, checkout_usage,
+	argc = parse_options(argc, argv, prefix, options, usagestr,
 			     PARSE_OPT_KEEP_DASHDASH);
 
-	if (opts.show_progress < 0) {
-		if (opts.quiet)
-			opts.show_progress = 0;
+	if (opts->show_progress < 0) {
+		if (opts->quiet)
+			opts->show_progress = 0;
 		else
-			opts.show_progress = isatty(2);
+			opts->show_progress = isatty(2);
 	}
 
-	if (conflict_style) {
-		opts.merge = 1; /* implied */
-		git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
+	if (opts->conflict_style) {
+		opts->merge = 1; /* implied */
+		git_xmerge_config("merge.conflictstyle", opts->conflict_style, NULL);
 	}
 
-	if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)
+	if ((!!opts->new_branch + !!opts->new_branch_force + !!opts->new_orphan_branch) > 1)
 		die(_("-b, -B and --orphan are mutually exclusive"));
 
 	/*
@@ -1302,14 +1341,14 @@  int cmd_checkout(int argc, const char **argv, const char *prefix)
 	 * and new_branch_force and new_orphan_branch will tell us which one of
 	 * -b/-B/--orphan is being used.
 	 */
-	if (opts.new_branch_force)
-		opts.new_branch = opts.new_branch_force;
+	if (opts->new_branch_force)
+		opts->new_branch = opts->new_branch_force;
 
-	if (opts.new_orphan_branch)
-		opts.new_branch = opts.new_orphan_branch;
+	if (opts->new_orphan_branch)
+		opts->new_branch = opts->new_orphan_branch;
 
 	/* --track without -b/-B/--orphan should DWIM */
-	if (opts.track != BRANCH_TRACK_UNSPECIFIED && !opts.new_branch) {
+	if (opts->track != BRANCH_TRACK_UNSPECIFIED && !opts->new_branch) {
 		const char *argv0 = argv[0];
 		if (!argc || !strcmp(argv0, "--"))
 			die(_("--track needs a branch name"));
@@ -1318,7 +1357,7 @@  int cmd_checkout(int argc, const char **argv, const char *prefix)
 		argv0 = strchr(argv0, '/');
 		if (!argv0 || !argv0[1])
 			die(_("missing branch name; try -b"));
-		opts.new_branch = argv0 + 1;
+		opts->new_branch = argv0 + 1;
 	}
 
 	/*
@@ -1337,56 +1376,56 @@  int cmd_checkout(int argc, const char **argv, const char *prefix)
 	if (argc) {
 		struct object_id rev;
 		int dwim_ok =
-			!opts.patch_mode &&
-			dwim_new_local_branch &&
-			opts.track == BRANCH_TRACK_UNSPECIFIED &&
-			!opts.new_branch;
+			!opts->patch_mode &&
+			opts->dwim_new_local_branch &&
+			opts->track == BRANCH_TRACK_UNSPECIFIED &&
+			!opts->new_branch;
 		int n = parse_branchname_arg(argc, argv, dwim_ok,
-					     &new_branch_info, &opts, &rev,
+					     &new_branch_info, opts, &rev,
 					     &dwim_remotes_matched);
 		argv += n;
 		argc -= n;
 	}
 
 	if (argc) {
-		parse_pathspec(&opts.pathspec, 0,
-			       opts.patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,
+		parse_pathspec(&opts->pathspec, 0,
+			       opts->patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,
 			       prefix, argv);
 
-		if (!opts.pathspec.nr)
+		if (!opts->pathspec.nr)
 			die(_("invalid path specification"));
 
 		/*
 		 * Try to give more helpful suggestion.
 		 * new_branch && argc > 1 will be caught later.
 		 */
-		if (opts.new_branch && argc == 1)
+		if (opts->new_branch && argc == 1)
 			die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
-				argv[0], opts.new_branch);
+				argv[0], opts->new_branch);
 
-		if (opts.force_detach)
+		if (opts->force_detach)
 			die(_("git checkout: --detach does not take a path argument '%s'"),
 			    argv[0]);
 
-		if (1 < !!opts.writeout_stage + !!opts.force + !!opts.merge)
+		if (1 < !!opts->writeout_stage + !!opts->force + !!opts->merge)
 			die(_("git checkout: --ours/--theirs, --force and --merge are incompatible when\n"
 			      "checking out of the index."));
 	}
 
-	if (opts.new_branch) {
+	if (opts->new_branch) {
 		struct strbuf buf = STRBUF_INIT;
 
-		if (opts.new_branch_force)
-			opts.branch_exists = validate_branchname(opts.new_branch, &buf);
+		if (opts->new_branch_force)
+			opts->branch_exists = validate_branchname(opts->new_branch, &buf);
 		else
-			opts.branch_exists =
-				validate_new_branchname(opts.new_branch, &buf, 0);
+			opts->branch_exists =
+				validate_new_branchname(opts->new_branch, &buf, 0);
 		strbuf_release(&buf);
 	}
 
 	UNLEAK(opts);
-	if (opts.patch_mode || opts.pathspec.nr) {
-		int ret = checkout_paths(&opts, new_branch_info.name);
+	if (opts->patch_mode || opts->pathspec.nr) {
+		int ret = checkout_paths(opts, new_branch_info.name);
 		if (ret && dwim_remotes_matched > 1 &&
 		    advice_checkout_ambiguous_remote_branch_name)
 			advise(_("'%s' matched more than one remote tracking branch.\n"
@@ -1405,6 +1444,49 @@  int cmd_checkout(int argc, const char **argv, const char *prefix)
 			       dwim_remotes_matched);
 		return ret;
 	} else {
-		return checkout_branch(&opts, &new_branch_info);
+		return checkout_branch(opts, &new_branch_info);
 	}
 }
+
+int cmd_checkout(int argc, const char **argv, const char *prefix)
+{
+	struct checkout_opts opts;
+	struct option *options = NULL;
+	int ret;
+
+	options = add_common_options(&opts, options);
+	options = add_switch_branch_options(&opts, options);
+	options = add_checkout_path_options(&opts, options);
+	ret = checkout_main(argc, argv, prefix, &opts,
+			    options, checkout_usage);
+	FREE_AND_NULL(options);
+	return ret;
+}
+
+int cmd_switch_branch(int argc, const char **argv, const char *prefix)
+{
+	struct checkout_opts opts;
+	struct option *options = NULL;
+	int ret;
+
+	options = add_common_options(&opts, options);
+	options = add_switch_branch_options(&opts, options);
+	ret = checkout_main(argc, argv, prefix, &opts,
+			    options, switch_branch_usage);
+	FREE_AND_NULL(options);
+	return ret;
+}
+
+int cmd_restore_paths(int argc, const char **argv, const char *prefix)
+{
+	struct checkout_opts opts;
+	struct option *options = NULL;
+	int ret;
+
+	options = add_common_options(&opts, options);
+	options = add_checkout_path_options(&opts, options);
+	ret = checkout_main(argc, argv, prefix, &opts,
+			    options, restore_paths_usage);
+	FREE_AND_NULL(options);
+	return ret;
+}
diff --git a/git.c b/git.c
index 2f604a41ea..e8a76a99da 100644
--- a/git.c
+++ b/git.c
@@ -542,6 +542,7 @@  static struct cmd_struct commands[] = {
 	{ "replace", cmd_replace, RUN_SETUP },
 	{ "rerere", cmd_rerere, RUN_SETUP },
 	{ "reset", cmd_reset, RUN_SETUP },
+	{ "restore-paths", cmd_restore_paths, RUN_SETUP | NEED_WORK_TREE },
 	{ "rev-list", cmd_rev_list, RUN_SETUP | NO_PARSEOPT },
 	{ "rev-parse", cmd_rev_parse, NO_PARSEOPT },
 	{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
@@ -557,6 +558,7 @@  static struct cmd_struct commands[] = {
 	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
 	{ "stripspace", cmd_stripspace },
 	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX | NO_PARSEOPT },
+	{ "switch-branch", cmd_switch_branch, RUN_SETUP | NEED_WORK_TREE },
 	{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
 	{ "tag", cmd_tag, RUN_SETUP | DELAY_PAGER_CONFIG },
 	{ "unpack-file", cmd_unpack_file, RUN_SETUP | NO_PARSEOPT },
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 8c9edce52f..c609d52926 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -126,7 +126,7 @@  struct option *parse_options_concat(struct option *a, struct option *b)
 	struct option *ret;
 	size_t i, a_len = 0, b_len = 0;
 
-	for (i = 0; a[i].type != OPTION_END; i++)
+	for (i = 0; a && a[i].type != OPTION_END; i++)
 		a_len++;
 	for (i = 0; b[i].type != OPTION_END; i++)
 		b_len++;