All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Phillip Wood <phillip.wood123@gmail.com>,
	Junio C Hamano" <gitster@pobox.com>,
	"Emily Shaffer" <emilyshaffer@google.com>,
	"Derrick Stolee" <stolee@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Alexandr Miloslavskiy" <alexandr.miloslavskiy@syntevo.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Alexandr Miloslavskiy" <alexandr.miloslavskiy@syntevo.com>
Subject: [PATCH 11/16] parse_branchname_arg(): easier to understand variables
Date: Thu, 12 Dec 2019 14:36:20 +0000	[thread overview]
Message-ID: <a0a884b384fd7ebdb824bef90acd6d6366be8cf5.1576161385.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.490.git.1576161385.gitgitgadget@gmail.com>

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

`dash_dash_pos` was only calculated under `opts->accept_pathspec`. This
is unexpected to readers and made it harder to reason about the code.
Fix this by restoring the expected meaning.

Simplify the code by dropping `argcount` and useless `argc` / `argv`
manipulations.

This should not change behavior in any way.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 builtin/checkout.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index b847695d2b..f35fe2cc26 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1152,7 +1152,6 @@ static int parse_branchname_arg(int argc, const char **argv,
 				struct object_id *rev)
 {
 	const char **new_branch = &opts->new_branch;
-	int argcount = 0;
 	const char *arg;
 	int dash_dash_pos;
 	int has_dash_dash = 0;
@@ -1211,17 +1210,21 @@ static int parse_branchname_arg(int argc, const char **argv,
 	arg = argv[0];
 	dash_dash_pos = -1;
 	for (i = 0; i < argc; i++) {
-		if (opts->accept_pathspec && !strcmp(argv[i], "--")) {
+		if (!strcmp(argv[i], "--")) {
 			dash_dash_pos = i;
 			break;
 		}
 	}
-	if (dash_dash_pos == 0)
-		return 1; /* case (2) */
-	else if (dash_dash_pos == 1)
-		has_dash_dash = 1; /* case (3) or (1) */
-	else if (dash_dash_pos >= 2)
-		die(_("only one reference expected, %d given."), dash_dash_pos);
+
+	if (opts->accept_pathspec) {
+	    if (dash_dash_pos == 0)
+		    return 1; /* case (2) */
+	    else if (dash_dash_pos == 1)
+		    has_dash_dash = 1; /* case (3) or (1) */
+	    else if (dash_dash_pos >= 2)
+		    die(_("only one reference expected, %d given."), dash_dash_pos);
+	}
+
 	opts->count_checkout_paths = !opts->quiet && !has_dash_dash;
 
 	if (!strcmp(arg, "-"))
@@ -1268,15 +1271,10 @@ static int parse_branchname_arg(int argc, const char **argv,
 		if (!recover_with_dwim) {
 			if (has_dash_dash)
 				die(_("invalid reference: %s"), arg);
-			return argcount;
+			return 0;
 		}
 	}
 
-	/* we can't end up being in (2) anymore, eat the argument */
-	argcount++;
-	argv++;
-	argc--;
-
 	setup_new_branch_info_and_source_tree(new_branch_info, opts, rev, arg);
 
 	if (!opts->source_tree)                   /* case (1): want a tree */
@@ -1289,15 +1287,11 @@ static int parse_branchname_arg(int argc, const char **argv,
 		 * even if there happen to be a file called 'branch';
 		 * it would be extremely annoying.
 		 */
-		if (argc)
+		if (argc > 1)
 			verify_non_filename(opts->prefix, arg);
-	} else if (opts->accept_pathspec) {
-		argcount++;
-		argv++;
-		argc--;
 	}
 
-	return argcount;
+	return (dash_dash_pos == 1) ? 2 : 1;
 }
 
 static int switch_unborn_to_new_branch(const struct checkout_opts *opts)
-- 
gitgitgadget


  parent reply	other threads:[~2019-12-12 14:36 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-12 14:36 [PATCH 00/16] Extend --pathspec-from-file to git add, checkout Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 01/16] t7107, t7526: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 02/16] commit: forbid --pathspec-from-file --all Alexandr Miloslavskiy via GitGitGadget
2019-12-16 12:02   ` Phillip Wood
2019-12-16 15:53     ` Alexandr Miloslavskiy
2019-12-12 14:36 ` [PATCH 03/16] cmd_add: prepare for next patch Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 04/16] add: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 05/16] doc: checkout: remove duplicate synopsis Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 06/16] doc: checkout: fix broken text reference Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 07/16] doc: checkout: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 08/16] doc: restore: " Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 09/16] parse_branchname_arg(): extract part as new function Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 10/16] checkout: die() on ambiguous tracking branches Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` Alexandr Miloslavskiy via GitGitGadget [this message]
2019-12-12 14:36 ` [PATCH 12/16] parse_branchname_arg(): introduce expect_commit_only Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 13/16] parse_branchname_arg(): update code comments Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 14/16] parse_branchname_arg(): refactor the decision making Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 15/16] t2024: cover more cases Alexandr Miloslavskiy via GitGitGadget
2019-12-12 14:36 ` [PATCH 16/16] checkout, restore: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:47 ` [PATCH v2 00/18] Extend --pathspec-from-file to git add, checkout Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:47   ` [PATCH v2 01/18] t7107, t7526: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
2019-12-18 21:57     ` Junio C Hamano
2019-12-19  6:25       ` Junio C Hamano
2019-12-19 17:19       ` Alexandr Miloslavskiy
2019-12-16 15:47   ` [PATCH v2 02/18] t7526: add tests for error conditions Alexandr Miloslavskiy via GitGitGadget
2019-12-18 22:02     ` Junio C Hamano
2019-12-19 18:03       ` Alexandr Miloslavskiy
2019-12-16 15:47   ` [PATCH v2 03/18] t7107: " Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:47   ` [PATCH v2 04/18] commit: forbid --pathspec-from-file --all Alexandr Miloslavskiy via GitGitGadget
2019-12-17 20:00     ` Phillip Wood
2019-12-18 22:04       ` Junio C Hamano
2019-12-18 22:06         ` Junio C Hamano
2019-12-18 22:16           ` Junio C Hamano
2019-12-19 17:38             ` Alexandr Miloslavskiy
2019-12-16 15:47   ` [PATCH v2 05/18] cmd_add: prepare for next patch Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:47   ` [PATCH v2 06/18] add: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:47   ` [PATCH v2 07/18] doc: checkout: remove duplicate synopsis Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:47   ` [PATCH v2 08/18] doc: checkout: fix broken text reference Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:47   ` [PATCH v2 09/18] doc: checkout: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:47   ` [PATCH v2 10/18] doc: restore: " Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:47   ` [PATCH v2 11/18] parse_branchname_arg(): extract part as new function Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:48   ` [PATCH v2 12/18] checkout: die() on ambiguous tracking branches Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:48   ` [PATCH v2 13/18] parse_branchname_arg(): easier to understand variables Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:48   ` [PATCH v2 14/18] parse_branchname_arg(): introduce expect_commit_only Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:48   ` [PATCH v2 15/18] parse_branchname_arg(): update code comments Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:48   ` [PATCH v2 16/18] parse_branchname_arg(): refactor the decision making Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:48   ` [PATCH v2 17/18] t2024: cover more cases Alexandr Miloslavskiy via GitGitGadget
2019-12-16 15:48   ` [PATCH v2 18/18] checkout, restore: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01   ` [PATCH v3 00/18] Extend --pathspec-from-file to git add, checkout Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 01/18] t7107, t7526: directly test parse_pathspec_file() Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 02/18] t7526: add tests for error conditions Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 03/18] t7107: " Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 04/18] commit: forbid --pathspec-from-file --all Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 05/18] cmd_add: prepare for next patch Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 06/18] add: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 07/18] doc: checkout: remove duplicate synopsis Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 08/18] doc: checkout: fix broken text reference Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 09/18] doc: checkout: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 10/18] doc: restore: " Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 11/18] parse_branchname_arg(): extract part as new function Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 12/18] checkout: die() on ambiguous tracking branches Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 13/18] parse_branchname_arg(): easier to understand variables Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 14/18] parse_branchname_arg(): simplify argument eating Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 15/18] parse_branchname_arg(): update code comments Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 16/18] parse_branchname_arg(): refactor the decision making Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 17/18] t2024: cover more cases Alexandr Miloslavskiy via GitGitGadget
2019-12-19 18:01     ` [PATCH v3 18/18] checkout, restore: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a0a884b384fd7ebdb824bef90acd6d6366be8cf5.1576161385.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=alexandr.miloslavskiy@syntevo.com \
    --cc=avarab@gmail.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.