git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] checkout.c: add strict usage of -- before file_path
@ 2018-05-13  2:23 Dannier Castro L
  2018-05-13  2:23 ` [PATCH 2/3] test: update tests for strict usage of -- checkout Dannier Castro L
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Dannier Castro L @ 2018-05-13  2:23 UTC (permalink / raw)
  To: git; +Cc: Dannier Castro L, gitster, Matthieu.Moy, jrnieder, bmwill

Currently, <checkout> is a complex command able to handle both
branches and files without any distintion other than their names,
taking into account that depending on the type (branch or file),
the functionality is completely different, the easier example:

$ git checkout <branch>  # Switch from current branch to <branch>.
$ git checkout <file>    # Restore <file> from HEAD, discarding
                         # changes if it's necessary.
$ git checkout -- <file> # The same as the last one, only with an
                         # useless '--'.

For GIT new users, this complicated versatility of <checkout> could
be very confused, also considering that actually the flag '--' is
completely useless (added or not, there is not any difference for
this command), when the same program messages promote the use of
this flag.

Regarding the <checkout>'s power to overwrite any file, discarding
changes if they exist without some way of recovering them, the
solution propuses that the usage of '--' is strict before to
specify the file(s) path(s) for any <checkout> command (including
all types of flags), as a "defense barrier" to make sure about
user's knowledge and intension running <checkout>.

The solution consists in detect '--' into command args, allowing
the discard of changes and considering the following names as
file paths, otherwise, they are branch names.

Signed-off-by: Dannier Castro L <danniercl@gmail.com>
---
 builtin/checkout.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index d76e13c..ec577b3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -40,6 +40,7 @@ struct checkout_opts {
 	int ignore_skipworktree;
 	int ignore_other_worktrees;
 	int show_progress;
+	int discard_changes;
 
 	const char *new_branch;
 	const char *new_branch_force;
@@ -885,8 +886,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 	/*
 	 * case 1: git checkout <ref> -- [<paths>]
 	 *
-	 *   <ref> must be a valid tree, everything after the '--' must be
-	 *   a path.
+	 *   <ref> must be a valid tree. '--' must always be before any path,
+	 *   it means, everything after the '--' must be a path.
 	 *
 	 * case 2: git checkout -- [<paths>]
 	 *
@@ -900,26 +901,28 @@ static int parse_branchname_arg(int argc, const char **argv,
 	 *       omit at most one side), and if there is a unique merge base
 	 *       between A and B, A...B names that merge base.
 	 *
-	 *   (b) If <something> is _not_ a commit, either "--" is present
-	 *       or <something> is not a path, no -t or -b was given, and
-	 *       and there is a tracking branch whose name is <something>
-	 *       in one and only one remote, then this is a short-hand to
-	 *       fork local <something> from that remote-tracking branch.
+	 *   (b) If <something> is _not_ a commit, either "--" is present,
+	 *       no -t or -b was given, and there is a tracking branch
+	 *       whose name is <something> in one and only one remote,
+	 *       then this is a short-hand to fork local <something> from
+	 *       that remote-tracking branch.
 	 *
 	 *   (c) Otherwise, if "--" is present, treat it like case (1).
 	 *
 	 *   (d) Otherwise :
 	 *       - if it's a reference, treat it like case (1)
-	 *       - else if it's a path, treat it like case (2)
 	 *       - else: fail.
 	 *
-	 * case 4: git checkout <something> <paths>
+	 *   <something> can never be a path (at least without '--' before).
+	 *
+	 * case 4: git checkout <something> -- <paths>
 	 *
 	 *   The first argument must not be ambiguous.
 	 *   - If it's *only* a reference, treat it like case (1).
-	 *   - If it's only a path, treat it like case (2).
 	 *   - else: fail.
 	 *
+	 *   <something> can never be a path (at least without '--' before).
+	 *
 	 */
 	if (!argc)
 		return 0;
@@ -928,6 +931,7 @@ static int parse_branchname_arg(int argc, const char **argv,
 	dash_dash_pos = -1;
 	for (i = 0; i < argc; i++) {
 		if (!strcmp(argv[i], "--")) {
+			opts->discard_changes = 1;
 			dash_dash_pos = i;
 			break;
 		}
@@ -957,8 +961,8 @@ static int parse_branchname_arg(int argc, const char **argv,
 		    (check_filename(opts->prefix, arg) || !no_wildcard(arg)))
 			recover_with_dwim = 0;
 		/*
-		 * Accept "git checkout foo" and "git checkout foo --"
-		 * as candidates for dwim.
+		 * Accept "git checkout foo_branch" and
+		 * "git checkout foo_branch --" as candidates for dwim.
 		 */
 		if (!(argc == 1 && !has_dash_dash) &&
 		    !(argc == 2 && has_dash_dash))
@@ -1254,7 +1258,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	}
 
 	UNLEAK(opts);
-	if (opts.patch_mode || opts.pathspec.nr)
+	if (opts.patch_mode || (opts.pathspec.nr && opts.discard_changes))
 		return checkout_paths(&opts, new_branch_info.name);
 	else
 		return checkout_branch(&opts, &new_branch_info);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-05-14 15:43 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-13  2:23 [PATCH 1/3] checkout.c: add strict usage of -- before file_path Dannier Castro L
2018-05-13  2:23 ` [PATCH 2/3] test: update tests for strict usage of -- checkout Dannier Castro L
2018-05-13  2:23 ` [PATCH 3/3] doc: update doc " Dannier Castro L
2018-05-13  6:03 ` [PATCH 1/3] checkout.c: add strict usage of -- before file_path Duy Nguyen
2018-05-13 19:11   ` Dannier Castro L
2018-05-13 20:18     ` SZEDER Gábor
2018-05-13 23:06     ` Philip Oakley
2018-05-14  1:51   ` Junio C Hamano
2018-05-13 21:02 ` Kevin Daudt
2018-05-14 14:52   ` Duy Nguyen
2018-05-14 15:43     ` Duy Nguyen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).