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

Re: [PATCH v10 12/12] bisect--helper: `get_terms` & `bisect_terms` shell function in C

From
Torsten Bögershausen <tboegi@web.de>
Date
Jul 22, 2016, 02:29 UTC
Message-ID
<7a408f75-74d9-7e59-35d6-3a931414a7f9@web.de>
In-Reply-To
<010201560a4782ce-8d6545f1-2454-492b-a2c6-6e0f2ac824d4-000000@eu-west-1.amazonses.com>
On 07/20/2016 11:47 PM, Pranit Bauva wrote:
Show 36 quoted lines
> Reimplement the `get_terms` and `bisect_terms` shell function in C and
> add `bisect-terms` subcommand to `git bisect--helper` to call it from
> git-bisect.sh .
>
> In the shell version, the terms were identified by strings but in C
> version its done by bit manipulation and passing the integer value to
> the function.
>
> Using `--bisect-terms` subcommand is a temporary measure to port shell
> function in C so as to use the existing test suite. As more functions
> are ported, this subcommand will be retired and will be called by some
> other methods.
>
> Mentored-by: Lars Schneider <larsxschneider@gmail.com>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> ---
>  builtin/bisect--helper.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++-
>  git-bisect.sh            | 35 ++---------------------
>  2 files changed, 75 insertions(+), 34 deletions(-)
>
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 001096a..185a8ad 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -8,6 +8,13 @@
>  #include "run-command.h"
>  #include "prompt.h"
>
> +enum terms_defined {
> +	TERM_BAD = 1,
> +	TERM_GOOD = 2,
> +	TERM_NEW = 4,
> +	TERM_OLD = 8
> +};
> +

What does TERM stand for ? It could be TERMinal, TERMinator or just TERM. Something like BIS_TERM_DEF_BAD .. may be more intuitive, and may avoid name clashes in the long run.

And why are the defines 1,2,4,8 ? It looks as if a #define bitmap may be a better choice here ? How do we do these kind of bit-wise opions otherwise ?

Show 48 quoted lines
>  static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
>  static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
>  static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
> @@ -26,6 +33,7 @@ static const char * const git_bisect_helper_usage[] = {
>  	N_("git bisect--helper --bisect-write <state> <revision> <TERM_GOOD> <TERM_BAD> [<nolog>]"),
>  	N_("git bisect--helper --bisect-check-and-set-terms <command> <TERM_GOOD> <TERM_BAD>"),
>  	N_("git bisect--helper --bisect-next-check [<term>] <TERM_GOOD> <TERM_BAD"),
> +	N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
>  	NULL
>  };
>
> @@ -359,6 +367,43 @@ static int bisect_next_check(const struct bisect_terms *terms,
>  	return 0;
>  }
>
> +static int get_terms(struct bisect_terms *terms)
> +{
> +	FILE *fp;
> +	int res;
> +	fp = fopen(git_path_bisect_terms(), "r");
> +	if (!fp)
> +		return -1;
> +
> +	bisect_terms_reset(terms);
> +	res = strbuf_getline(&terms->term_bad, fp) == EOF ||
> +	      strbuf_getline(&terms->term_good, fp) == EOF;
> +
> +	fclose(fp);
> +	return res ? -1 : 0;
> +}
> +
> +static int bisect_terms(struct bisect_terms *terms, int term_defined)
> +{
> +	if (get_terms(terms)) {
> +		fprintf(stderr, "no terms defined\n");
> +		return -1;
> +	}
> +	if (!term_defined) {
> +		printf("Your current terms are %s for the old state\nand "
> +		       "%s for the new state.\n", terms->term_good.buf,
> +		       terms->term_bad.buf);
> +		return 0;
> +	}
> +
> +	if (term_defined == TERM_GOOD || term_defined == TERM_OLD)
> +		printf("%s\n", terms->term_good.buf);
> +	if (term_defined == TERM_BAD || term_defined == TERM_NEW)
> +		printf("%s\n", terms->term_bad.buf);

May be a switch-case ? Or at least "else if" ?

Show 51 quoted lines
> +
> +	return 0;
> +}
> +
>  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  {
>  	enum {
> @@ -369,9 +414,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  		CHECK_EXPECTED_REVS,
>  		BISECT_WRITE,
>  		CHECK_AND_SET_TERMS,
> -		BISECT_NEXT_CHECK
> +		BISECT_NEXT_CHECK,
> +		BISECT_TERMS
>  	} cmdmode = 0;
>  	int no_checkout = 0, res = 0;
> +	enum terms_defined term_defined = 0;
>  	struct option options[] = {
>  		OPT_CMDMODE(0, "next-all", &cmdmode,
>  			 N_("perform 'git bisect next'"), NEXT_ALL),
> @@ -389,6 +436,16 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  			 N_("check and set terms in a bisection state"), CHECK_AND_SET_TERMS),
>  		OPT_CMDMODE(0, "bisect-next-check", &cmdmode,
>  			 N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK),
> +		OPT_CMDMODE(0, "bisect-terms", &cmdmode,
> +			 N_("print out the bisect terms"), BISECT_TERMS),
> +		OPT_BIT(0, "term-bad", &term_defined,
> +			 N_("show the bad term"), TERM_BAD),
> +		OPT_BIT(0, "term-good", &term_defined,
> +			 N_("show the good term"), TERM_GOOD),
> +		OPT_BIT(0, "term-new", &term_defined,
> +			 N_("show the new term"), TERM_NEW),
> +		OPT_BIT(0, "term-old", &term_defined,
> +			 N_("show the old term"), TERM_OLD),
>  		OPT_BOOL(0, "no-checkout", &no_checkout,
>  			 N_("update BISECT_HEAD instead of checking out the current commit")),
>  		OPT_END()
> @@ -399,6 +456,16 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  	argc = parse_options(argc, argv, prefix, options,
>  			     git_bisect_helper_usage, 0);
>
> +	if (cmdmode != BISECT_TERMS && term_defined)
> +		die(_("--term-bad, --term-good, --term-new and --term-old "
> +		      "can be used only with --bisect-terms"));
> +
> +	if (term_defined != 0 && term_defined != TERM_BAD &&
> +	    term_defined != TERM_GOOD && term_defined != TERM_NEW &&
> +	    term_defined != TERM_OLD)
> +		die(_("only one option among --term-bad, --term-good, "
> +		      "--term-new and --term-old can be used."));
> +
[]
Previous: Pranit BauvaNext: Pranit Bauva
Message 4 of 20 in “bisect--helper: use OPT_CMDMODE instead of OPT_BOOL”
  1. 01/12 bisect--helper: use OPT_CMDMODE instead of OPT_BOOLPranit Bauva, Jul 20, 2016
  2. 06/12 wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()Pranit Bauva, Jul 20, 2016
  3. 12/12 bisect--helper: `get_terms` & `bisect_terms` shell function in CPranit Bauva, Jul 20, 2016
  4. Torsten BögershausenJul 22, 2016
  5. Pranit BauvaJul 22, 2016
  6. Junio C HamanoJul 25, 2016
  7. Christian CouderJul 25, 2016
  8. Torsten BögershausenJul 26, 2016
  9. Junio C HamanoJul 26, 2016
  10. Pranit BauvaJul 27, 2016
  11. Junio C HamanoJul 27, 2016
  12. 10/12 bisect--helper: `check_and_set_terms` shell function in CPranit Bauva, Jul 20, 2016
  13. 11/12 bisect--helper: `bisect_next_check` shell function in CPranit Bauva, Jul 20, 2016
  14. 04/12 bisect--helper: `bisect_clean_state` shell function in CPranit Bauva, Jul 20, 2016
  15. 05/12 t6030: explicitly test for bisection cleanupPranit Bauva, Jul 20, 2016
  16. 02/12 bisect: rewrite `check_term_format` shell function in CPranit Bauva, Jul 20, 2016
  17. 03/12 bisect--helper: `write_terms` shell function in CPranit Bauva, Jul 20, 2016
  18. 07/12 bisect--helper: `bisect_reset` shell function in CPranit Bauva, Jul 20, 2016
  19. 09/12 bisect--helper: `bisect_write` shell function in CPranit Bauva, Jul 20, 2016
  20. 08/12 bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in CPranit Bauva, Jul 20, 2016

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.