git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/3] checkout.c: add strict usage of -- before file_path

From
DLDannier Castro L <danniercl@gmail.com>
Date
May 13, 2018, 02:23 UTC
Message-ID
<1526178214-30956-1-git-send-email-danniercl@gmail.com>

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
Next: Dannier Castro L
Message 1 of 11 in “checkout.c: add strict usage of -- before file_path”
  1. 1/3 checkout.c: add strict usage of -- before file_pathDannier Castro L, May 13, 2018
  2. 2/3 test: update tests for strict usage of -- checkoutDannier Castro L, May 13, 2018
  3. 3/3 doc: update doc for strict usage of -- checkoutDannier Castro L, May 13, 2018
  4. Duy NguyenMay 13, 2018
  5. Dannier Castro LMay 13, 2018
  6. SZEDER GáborMay 13, 2018
  7. Philip OakleyMay 13, 2018
  8. Junio C HamanoMay 14, 2018
  9. Kevin DaudtMay 13, 2018
  10. Duy NguyenMay 14, 2018
  11. Duy NguyenMay 14, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.