git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Pranit Bauva <pranit.bauva@gmail.com>
To: git@vger.kernel.org
Subject: [PATCH v3] bisect--helper: convert a function in shell to C
Date: Wed, 23 Mar 2016 07:16:06 +0000	[thread overview]
Message-ID: <01020153a254974b-68f7d16a-66d7-4dc1-805d-2185ff1b3ebf-000000@eu-west-1.amazonses.com> (raw)
In-Reply-To: <010201539d57ae98-ce4860a6-f7b6-4e06-b556-3c1340cd7749-000000@eu-west-1.amazonses.com>

Convert the code literally without changing its design even though it
seems that it is obscure as to the use of comparing revision to different
bisect arguments which seems like a problem in shell because of the way
function arguments are handled.

The argument handling is kind of hard coded right now because it is not
really be meant to be used like this and this is just for testing
purposes whether this new method is as functional as its counter part.
The shell counter part of the method has been retained for historical
purposes.

Also using OPT_CMDMODE() to handle check-term-format and next-all
because these sub commands are independent and error should be shown if used
together and should be handled independently.

This commit reduces the number of failed tests in
t6030-bisect-porcelain.sh and t6041-bisect-submodule.sh

The corresponding shell function is :
https://github.com/git/git/blob/v2.8.0-rc4/git-bisect.sh#L572-L597

Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>

---
Changes wrt v1:
 - Remove the function declaration
 - Introduce another method one_of() to reduce the clutter in if
 - Add the documentation as to which part should remain untouched
 - Use OPT_CMDMODE() for --check-term-format and --next-all
 - Remove the '=' in git-bisect.sh
 - Respect the coding convention to indent when a line is spread across
   many lines
 - s/its/it is/g
 - Output of tests:
   - t6002 : http://paste.ubuntu.com/15477883/
   - t6030 : http://paste.ubuntu.com/15477887/
   - t6041 : http://paste.ubuntu.com/15477897/
 - Add the comment that a part shouldn't be touched
---
 builtin/bisect--helper.c | 73 ++++++++++++++++++++++++++++++++++++++++++++----
 git-bisect.sh            |  4 +--
 2 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 3324229..ab3891c 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -2,30 +2,93 @@
 #include "cache.h"
 #include "parse-options.h"
 #include "bisect.h"
+#include "refs.h"
 
 static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --next-all [--no-checkout]"),
+	N_("git bisect--helper --check-term-format <term> <revision>"),
 	NULL
 };
 
+static int one_of(const char *term, ...) {
+	va_list matches;
+	const char *match;
+
+	va_start(matches, term);
+	while ((match = va_arg(matches, const char *)) != NULL)
+		if (!strcmp(term, match))
+			return 1;
+
+	va_end(matches);
+
+	return 0;
+}
+
+static int check_term_format(const char *term, const char *revision, int flag) {
+	if (check_refname_format(term, flag))
+		die("'%s' is not a valid term", term);
+
+	if (one_of(term, "help", "start", "skip", "next", "reset", "visualize",
+	    "replay", "log", "run", NULL))
+		die("can't use the builtin command '%s' as a term", term);
+
+	/* In theory, nothing prevents swapping
+	 * completely good and bad, but this situation
+	 * could be confusing and hasn't been tested
+	 * enough. Forbid it for now.
+	 */
+
+	if (!strcmp(term, "bad") || !strcmp(term, "new"))
+		if (strcmp(revision, "bad"))
+			die("can't change the meaning of term '%s'", term);
+
+	if(!strcmp(term, "good") || !strcmp(term, "old"))
+		if (strcmp(revision, "good"))
+			die("can't change the meaning of term '%s'", term);
+
+	return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
-	int next_all = 0;
+	int sub_command = 0;
 	int no_checkout = 0;
+
+	enum sub_commands {
+		NEXT_ALL,
+		CHECK_TERM_FMT
+	};
+
 	struct option options[] = {
-		OPT_BOOL(0, "next-all", &next_all,
-			 N_("perform 'git bisect next'")),
+		OPT_CMDMODE(0, "next-all", &sub_command,
+			 N_("perform 'git bisect next'"), NEXT_ALL),
 		OPT_BOOL(0, "no-checkout", &no_checkout,
 			 N_("update BISECT_HEAD instead of checking out the current commit")),
+		OPT_CMDMODE(0, "check-term-format", &sub_command,
+			 N_("check the format of the ref"), CHECK_TERM_FMT),
 		OPT_END()
 	};
 
 	argc = parse_options(argc, argv, prefix, options,
 			     git_bisect_helper_usage, 0);
 
-	if (!next_all)
+	if (sub_command == CHECK_TERM_FMT) {
+		if (argc == 2) {
+			if (argv[0] != NULL && argv[1] != NULL)
+				return check_term_format(argv[0], argv[1], 0);
+			else
+				die("no revision or term provided with check_for_term");
+		}
+		else
+			die("--check-term-format expects 2 arguments");
+	}
+
+	if (sub_command != NEXT_ALL && sub_command != CHECK_TERM_FMT)
 		usage_with_options(git_bisect_helper_usage, options);
 
 	/* next-all */
-	return bisect_next_all(prefix, no_checkout);
+	if (sub_command == NEXT_ALL)
+		return bisect_next_all(prefix, no_checkout);
+
+	return 1;
 }
diff --git a/git-bisect.sh b/git-bisect.sh
index 5d1cb00..f63b83e 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -564,8 +564,8 @@ write_terms () {
 	then
 		die "$(gettext "please use two different terms")"
 	fi
-	check_term_format "$TERM_BAD" bad
-	check_term_format "$TERM_GOOD" good
+	git bisect--helper --check-term-format "$TERM_BAD" bad
+	git bisect--helper --check-term-format "$TERM_GOOD" good
 	printf '%s\n%s\n' "$TERM_BAD" "$TERM_GOOD" >"$GIT_DIR/BISECT_TERMS"
 }
 

--
https://github.com/git/git/pull/216

  parent reply	other threads:[~2016-03-23  7:24 UTC|newest]

Thread overview: 114+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-21 19:00 [PATCH] bisect--helper: convert a function in shell to C Pranit Bauva
2016-03-22  0:28 ` Stefan Beller
2016-03-22  6:10   ` Christian Couder
2016-03-22  6:13     ` Pranit Bauva
2016-03-22  6:13   ` Pranit Bauva
2016-03-22  8:01 ` [PATCH v2] " Pranit Bauva
2016-03-22 15:09   ` Johannes Schindelin
2016-03-22 15:11     ` Johannes Schindelin
2016-03-22 17:46       ` Pranit Bauva
2016-03-23 11:23         ` Johannes Schindelin
2016-03-22 16:03     ` Junio C Hamano
2016-03-22 16:49       ` Johannes Schindelin
2016-03-22 17:52       ` Pranit Bauva
2016-03-22 17:59         ` Stefan Beller
2016-03-23 11:24           ` Johannes Schindelin
2016-03-22 17:45     ` Pranit Bauva
2016-03-23 11:22       ` Johannes Schindelin
2016-03-23 13:53         ` Pranit Bauva
2016-03-23  7:16   ` Pranit Bauva [this message]
2016-03-23 11:57     ` [PATCH v3] " Johannes Schindelin
2016-03-23 13:16       ` Pranit Bauva
2016-03-23 16:24         ` Junio C Hamano
2016-03-23 18:38           ` Pranit Bauva
2016-03-23 16:15       ` Junio C Hamano
2016-03-23 18:46         ` Pranit Bauva
2016-05-04  5:07     ` [PATCH 0/2] bisect--helper: rewrite of check_term_format() Pranit Bauva
2016-05-04  5:07       ` [PATCH 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL Pranit Bauva
2016-05-04  5:34         ` Pranit Bauva
2016-05-04  6:07         ` Eric Sunshine
2016-05-04  6:50           ` Christian Couder
2016-05-04 11:05             ` Johannes Schindelin
2016-05-04 12:04               ` Pranit Bauva
2016-05-04 12:05               ` Christian Couder
2016-05-04 12:21                 ` Johannes Schindelin
2016-05-04 12:41                   ` Christian Couder
2016-05-04 14:56                     ` Johannes Schindelin
2016-05-04 19:07                       ` Christian Couder
2016-05-08  6:54                         ` Johannes Schindelin
2016-05-04  7:28           ` Junio C Hamano
2016-05-04 11:02         ` Johannes Schindelin
2016-05-04  5:07       ` [PATCH 2/2] bisect: rewrite `check_term_format` shell function in C Pranit Bauva
2016-05-04  6:52         ` Eric Sunshine
2016-05-04  7:36           ` Pranit Bauva
2016-05-04  7:40             ` Pranit Bauva
2016-05-04  8:28             ` Eric Sunshine
2016-05-04  8:54               ` Christian Couder
2016-05-04 11:58               ` Pranit Bauva
2016-05-04 17:49                 ` Eric Sunshine
2016-05-04 18:08                   ` Pranit Bauva
2016-05-04 11:13             ` Johannes Schindelin
2016-05-04 12:00               ` Pranit Bauva
2016-05-04 12:21               ` Christian Couder
2016-05-04 19:10               ` Junio C Hamano
2016-05-04  5:22       ` [PATCH 0/2] bisect--helper: rewrite of check_term_format() Christian Couder
2016-05-04  5:25         ` Pranit Bauva
2016-05-06 14:49       ` [PATCH v5 0/2] bisect--helper: rewrite of check-term-format() Pranit Bauva
2016-05-06 14:49         ` [PATCH v5 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL Pranit Bauva
2016-05-08  7:04           ` Johannes Schindelin
2016-05-08  7:17             ` Pranit Bauva
2016-05-08 15:30               ` Christian Couder
2016-05-08 15:33                 ` Pranit Bauva
2016-05-09 14:59               ` Johannes Schindelin
2016-05-09 15:33                 ` Pranit Bauva
2016-05-06 14:49         ` [PATCH v5 2/2] bisect: rewrite `check_term_format` shell function in C Pranit Bauva
2016-05-07  0:05           ` Eric Sunshine
2016-05-06 22:15         ` [PATCH v5 0/2] bisect--helper: rewrite of check-term-format() Junio C Hamano
2016-05-07 13:07           ` Pranit Bauva
2016-05-08  2:25             ` Junio C Hamano
2016-05-08  6:23               ` Pranit Bauva
2016-05-08 13:34                 ` Pranit Bauva
2016-05-16  0:35                   ` Eric Sunshine
2016-05-16 17:07                     ` Christian Couder
2016-05-20  6:59                     ` Pranit Bauva
2016-05-12  5:32         ` [PATCH v6 0/3] bisect--helper: check_term_format() & write_terms() Pranit Bauva
2016-05-12  5:32           ` [PATCH v6 1/3] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL Pranit Bauva
2016-05-16  7:07             ` Eric Sunshine
2016-05-20  7:17               ` Pranit Bauva
2016-05-12  5:32           ` [PATCH v6 2/3] bisect: rewrite `check_term_format` shell function in C Pranit Bauva
2016-05-12 22:36             ` Junio C Hamano
2016-05-13  6:59               ` Pranit Bauva
2016-05-13 18:01                 ` Junio C Hamano
2016-05-12  5:32           ` [PATCH v6 3/3] bisect--helper: `write_terms` " Pranit Bauva
2016-05-16  7:28             ` Eric Sunshine
2016-05-16 13:16               ` Johannes Schindelin
2016-05-16 15:42                 ` Eric Sunshine
2016-05-16 16:45                   ` Johannes Schindelin
2016-05-16 16:59                     ` Eric Sunshine
2016-05-20  7:45                 ` Pranit Bauva
2016-05-23 11:07                   ` Johannes Schindelin
2016-05-23 13:58                     ` Christian Couder
2016-05-23 15:10                       ` Johannes Schindelin
2016-05-23 14:33                     ` Pranit Bauva
2016-05-20  7:42               ` Pranit Bauva
2016-05-23 14:48           ` [PATCH v7 0/3] bisect--helper: check_term_format() & write_terms() Pranit Bauva
2016-05-23 14:48             ` [PATCH v7 1/3] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL Pranit Bauva
2016-05-23 14:48             ` [PATCH v7 2/3] bisect: rewrite `check_term_format` shell function in C Pranit Bauva
2016-05-23 14:48             ` [PATCH v7 3/3] bisect--helper: `write_terms` " Pranit Bauva
2016-05-23 16:01               ` Eric Sunshine
2016-05-23 17:59                 ` Pranit Bauva
2016-05-24  7:21             ` [PATCH v8 0/3] bisect--helper: check-term-format() & write_terms() Pranit Bauva
2016-05-24 18:42               ` [PATCH v9 0/3] bisect--helper: check_term_format() and write_terms() Pranit Bauva
2016-05-24 18:42               ` [PATCH v9 1/3] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL Pranit Bauva
2016-05-24 18:42               ` [PATCH v9 2/3] bisect: rewrite `check_term_format` shell function in C Pranit Bauva
2016-05-24 18:42               ` [PATCH v9 3/3] bisect--helper: `write_terms` " Pranit Bauva
2016-05-24  7:21             ` [PATCH v8 1/3] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL Pranit Bauva
2016-05-24  7:21             ` [PATCH v8 2/3] bisect: rewrite `check_term_format` shell function in C Pranit Bauva
2016-05-25  5:04               ` Johannes Schindelin
2016-05-25  5:13                 ` Pranit Bauva
2016-06-16  7:10               ` Lars Schneider
2016-06-17 12:48                 ` Pranit Bauva
2016-05-24  7:21             ` [PATCH v8 3/3] bisect--helper: `write_terms` " Pranit Bauva
2016-05-24  7:33               ` Christian Couder
2016-05-24 17:39                 ` Junio C Hamano
2016-05-24 18:31                 ` Pranit Bauva

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=01020153a254974b-68f7d16a-66d7-4dc1-805d-2185ff1b3ebf-000000@eu-west-1.amazonses.com \
    --to=pranit.bauva@gmail.com \
    --cc=git@vger.kernel.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).