diff mbox series

[v4,4/6] bisect--helper: reimplement `bisect_visualize()`shell function in C

Message ID 20210817081458.53136-5-mirucam@gmail.com (mailing list archive)
State Superseded
Headers show
Series Finish converting git bisect to C part 4 | expand

Commit Message

Miriam R. Aug. 17, 2021, 8:14 a.m. UTC 
From: Pranit Bauva <pranit.bauva@gmail.com>

Reimplement the `bisect_visualize()` shell function
in C and also add `--bisect-visualize` subcommand to
`git bisect--helper` to call it from git-bisect.sh.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
 builtin/bisect--helper.c | 48 +++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 25 +--------------------
 2 files changed, 48 insertions(+), 25 deletions(-)

Comments

Johannes Schindelin Aug. 17, 2021, 11:30 a.m. UTC | #1 
Hi Miriam,

this looks good!

Just one suggestion (but I won't insist on it):

On Tue, 17 Aug 2021, Miriam Rubio wrote:

> @@ -1036,6 +1037,44 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **ar
>  	return res;
>  }
>
> +static int bisect_visualize(struct bisect_terms *terms, const char **argv, int argc)
> +{
> +	struct strvec args = STRVEC_INIT;
> +	int flags = RUN_COMMAND_NO_STDIN, res = 0;
> +	struct strbuf sb = STRBUF_INIT;
> +
> +	if (bisect_next_check(terms, NULL) != 0)
> +		return BISECT_FAILED;
> +
> +	if (!argc) {
> +		if ((getenv("DISPLAY") || getenv("SESSIONNAME") || getenv("MSYSTEM") ||
> +		     getenv("SECURITYSESSIONID")) && exists_in_PATH("gitk"))
> +			strvec_push(&args, "gitk");
> +		else {
> +			strvec_pushl(&args, "log", NULL);

This could be written more concisely as `strvec_push(&args, "log")`.

> +			flags |= RUN_GIT_CMD;
> +		}
> +	} else {
> +		if (argv[0][0] == '-') {
> +			strvec_pushl(&args, "log", NULL);

Same here.

Otherwise, it looks good to me!

Thank you,
Dscho

> +			flags |= RUN_GIT_CMD;
> +		} else if (strcmp(argv[0], "tig") && !starts_with(argv[0], "git"))
> +			flags |= RUN_GIT_CMD;
> +
> +		strvec_pushv(&args, argv);
> +	}
> +
> +	strvec_pushl(&args, "--bisect", "--", NULL);
> +
> +	strbuf_read_file(&sb, git_path_bisect_names(), 0);
> +	sq_dequote_to_strvec(sb.buf, &args);
> +	strbuf_release(&sb);
> +
> +	res = run_command_v_opt(args.v, flags);
> +	strvec_clear(&args);
> +	return res;
> +}
> +
>  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  {
>  	enum {
> @@ -1048,7 +1087,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  		BISECT_STATE,
>  		BISECT_LOG,
>  		BISECT_REPLAY,
> -		BISECT_SKIP
> +		BISECT_SKIP,
> +		BISECT_VISUALIZE,
>  	} cmdmode = 0;
>  	int res = 0, nolog = 0;
>  	struct option options[] = {
> @@ -1070,6 +1110,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  			 N_("replay the bisection process from the given file"), BISECT_REPLAY),
>  		OPT_CMDMODE(0, "bisect-skip", &cmdmode,
>  			 N_("skip some commits for checkout"), BISECT_SKIP),
> +		OPT_CMDMODE(0, "bisect-visualize", &cmdmode,
> +			 N_("visualize the bisection"), BISECT_VISUALIZE),
>  		OPT_BOOL(0, "no-log", &nolog,
>  			 N_("no log for BISECT_WRITE")),
>  		OPT_END()
> @@ -1131,6 +1173,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  		get_terms(&terms);
>  		res = bisect_skip(&terms, argv, argc);
>  		break;
> +	case BISECT_VISUALIZE:
> +		get_terms(&terms);
> +		res = bisect_visualize(&terms, argv, argc);
> +		break;
>  	default:
>  		BUG("unknown subcommand %d", cmdmode);
>  	}
> diff --git a/git-bisect.sh b/git-bisect.sh
> index 6a7afaea8d..95f7f3fb8c 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -39,29 +39,6 @@ _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
>  TERM_BAD=bad
>  TERM_GOOD=good
>
> -bisect_visualize() {
> -	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
> -
> -	if test $# = 0
> -	then
> -		if test -n "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}" &&
> -			type gitk >/dev/null 2>&1
> -		then
> -			set gitk
> -		else
> -			set git log
> -		fi
> -	else
> -		case "$1" in
> -		git*|tig) ;;
> -		-*)	set git log "$@" ;;
> -		*)	set git "$@" ;;
> -		esac
> -	fi
> -
> -	eval '"$@"' --bisect -- $(cat "$GIT_DIR/BISECT_NAMES")
> -}
> -
>  bisect_run () {
>  	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
>
> @@ -152,7 +129,7 @@ case "$#" in
>  		# Not sure we want "next" at the UI level anymore.
>  		git bisect--helper --bisect-next "$@" || exit ;;
>  	visualize|view)
> -		bisect_visualize "$@" ;;
> +		git bisect--helper --bisect-visualize "$@" || exit;;
>  	reset)
>  		git bisect--helper --bisect-reset "$@" ;;
>  	replay)
> --
> 2.29.2
>
>
Miriam R. Aug. 17, 2021, 8:19 p.m. UTC | #2 
Hi Johannes,

El mar, 17 ago 2021 a las 13:30, Johannes Schindelin
(<Johannes.Schindelin@gmx.de>) escribió:
>
> Hi Miriam,
>
> this looks good!
>
> Just one suggestion (but I won't insist on it):
>
> On Tue, 17 Aug 2021, Miriam Rubio wrote:
>
> > @@ -1036,6 +1037,44 @@ static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **ar
> >       return res;
> >  }
> >
> > +static int bisect_visualize(struct bisect_terms *terms, const char **argv, int argc)
> > +{
> > +     struct strvec args = STRVEC_INIT;
> > +     int flags = RUN_COMMAND_NO_STDIN, res = 0;
> > +     struct strbuf sb = STRBUF_INIT;
> > +
> > +     if (bisect_next_check(terms, NULL) != 0)
> > +             return BISECT_FAILED;
> > +
> > +     if (!argc) {
> > +             if ((getenv("DISPLAY") || getenv("SESSIONNAME") || getenv("MSYSTEM") ||
> > +                  getenv("SECURITYSESSIONID")) && exists_in_PATH("gitk"))
> > +                     strvec_push(&args, "gitk");
> > +             else {
> > +                     strvec_pushl(&args, "log", NULL);
>
> This could be written more concisely as `strvec_push(&args, "log")`.
>
> > +                     flags |= RUN_GIT_CMD;
> > +             }
> > +     } else {
> > +             if (argv[0][0] == '-') {
> > +                     strvec_pushl(&args, "log", NULL);
>
> Same here.
Sure, I will change it in both cases.
Thank you for reviewing,
Miriam.

>
> Otherwise, it looks good to me!
>
> Thank you,
> Dscho
>
> > +                     flags |= RUN_GIT_CMD;
> > +             } else if (strcmp(argv[0], "tig") && !starts_with(argv[0], "git"))
> > +                     flags |= RUN_GIT_CMD;
> > +
> > +             strvec_pushv(&args, argv);
> > +     }
> > +
> > +     strvec_pushl(&args, "--bisect", "--", NULL);
> > +
> > +     strbuf_read_file(&sb, git_path_bisect_names(), 0);
> > +     sq_dequote_to_strvec(sb.buf, &args);
> > +     strbuf_release(&sb);
> > +
> > +     res = run_command_v_opt(args.v, flags);
> > +     strvec_clear(&args);
> > +     return res;
> > +}
> > +
> >  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> >  {
> >       enum {
> > @@ -1048,7 +1087,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> >               BISECT_STATE,
> >               BISECT_LOG,
> >               BISECT_REPLAY,
> > -             BISECT_SKIP
> > +             BISECT_SKIP,
> > +             BISECT_VISUALIZE,
> >       } cmdmode = 0;
> >       int res = 0, nolog = 0;
> >       struct option options[] = {
> > @@ -1070,6 +1110,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> >                        N_("replay the bisection process from the given file"), BISECT_REPLAY),
> >               OPT_CMDMODE(0, "bisect-skip", &cmdmode,
> >                        N_("skip some commits for checkout"), BISECT_SKIP),
> > +             OPT_CMDMODE(0, "bisect-visualize", &cmdmode,
> > +                      N_("visualize the bisection"), BISECT_VISUALIZE),
> >               OPT_BOOL(0, "no-log", &nolog,
> >                        N_("no log for BISECT_WRITE")),
> >               OPT_END()
> > @@ -1131,6 +1173,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> >               get_terms(&terms);
> >               res = bisect_skip(&terms, argv, argc);
> >               break;
> > +     case BISECT_VISUALIZE:
> > +             get_terms(&terms);
> > +             res = bisect_visualize(&terms, argv, argc);
> > +             break;
> >       default:
> >               BUG("unknown subcommand %d", cmdmode);
> >       }
> > diff --git a/git-bisect.sh b/git-bisect.sh
> > index 6a7afaea8d..95f7f3fb8c 100755
> > --- a/git-bisect.sh
> > +++ b/git-bisect.sh
> > @@ -39,29 +39,6 @@ _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
> >  TERM_BAD=bad
> >  TERM_GOOD=good
> >
> > -bisect_visualize() {
> > -     git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
> > -
> > -     if test $# = 0
> > -     then
> > -             if test -n "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}" &&
> > -                     type gitk >/dev/null 2>&1
> > -             then
> > -                     set gitk
> > -             else
> > -                     set git log
> > -             fi
> > -     else
> > -             case "$1" in
> > -             git*|tig) ;;
> > -             -*)     set git log "$@" ;;
> > -             *)      set git "$@" ;;
> > -             esac
> > -     fi
> > -
> > -     eval '"$@"' --bisect -- $(cat "$GIT_DIR/BISECT_NAMES")
> > -}
> > -
> >  bisect_run () {
> >       git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
> >
> > @@ -152,7 +129,7 @@ case "$#" in
> >               # Not sure we want "next" at the UI level anymore.
> >               git bisect--helper --bisect-next "$@" || exit ;;
> >       visualize|view)
> > -             bisect_visualize "$@" ;;
> > +             git bisect--helper --bisect-visualize "$@" || exit;;
> >       reset)
> >               git bisect--helper --bisect-reset "$@" ;;
> >       replay)
> > --
> > 2.29.2
> >
> >
diff mbox series

Patch

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index f184eaeac6..4258429c1c 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -30,6 +30,7 @@  static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-state (good|old) [<rev>...]"),
 	N_("git bisect--helper --bisect-replay <filename>"),
 	N_("git bisect--helper --bisect-skip [(<rev>|<range>)...]"),
+	N_("git bisect--helper --bisect-visualize"),
 	NULL
 };
 
@@ -1036,6 +1037,44 @@  static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **ar
 	return res;
 }
 
+static int bisect_visualize(struct bisect_terms *terms, const char **argv, int argc)
+{
+	struct strvec args = STRVEC_INIT;
+	int flags = RUN_COMMAND_NO_STDIN, res = 0;
+	struct strbuf sb = STRBUF_INIT;
+
+	if (bisect_next_check(terms, NULL) != 0)
+		return BISECT_FAILED;
+
+	if (!argc) {
+		if ((getenv("DISPLAY") || getenv("SESSIONNAME") || getenv("MSYSTEM") ||
+		     getenv("SECURITYSESSIONID")) && exists_in_PATH("gitk"))
+			strvec_push(&args, "gitk");
+		else {
+			strvec_pushl(&args, "log", NULL);
+			flags |= RUN_GIT_CMD;
+		}
+	} else {
+		if (argv[0][0] == '-') {
+			strvec_pushl(&args, "log", NULL);
+			flags |= RUN_GIT_CMD;
+		} else if (strcmp(argv[0], "tig") && !starts_with(argv[0], "git"))
+			flags |= RUN_GIT_CMD;
+
+		strvec_pushv(&args, argv);
+	}
+
+	strvec_pushl(&args, "--bisect", "--", NULL);
+
+	strbuf_read_file(&sb, git_path_bisect_names(), 0);
+	sq_dequote_to_strvec(sb.buf, &args);
+	strbuf_release(&sb);
+
+	res = run_command_v_opt(args.v, flags);
+	strvec_clear(&args);
+	return res;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -1048,7 +1087,8 @@  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		BISECT_STATE,
 		BISECT_LOG,
 		BISECT_REPLAY,
-		BISECT_SKIP
+		BISECT_SKIP,
+		BISECT_VISUALIZE,
 	} cmdmode = 0;
 	int res = 0, nolog = 0;
 	struct option options[] = {
@@ -1070,6 +1110,8 @@  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("replay the bisection process from the given file"), BISECT_REPLAY),
 		OPT_CMDMODE(0, "bisect-skip", &cmdmode,
 			 N_("skip some commits for checkout"), BISECT_SKIP),
+		OPT_CMDMODE(0, "bisect-visualize", &cmdmode,
+			 N_("visualize the bisection"), BISECT_VISUALIZE),
 		OPT_BOOL(0, "no-log", &nolog,
 			 N_("no log for BISECT_WRITE")),
 		OPT_END()
@@ -1131,6 +1173,10 @@  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		get_terms(&terms);
 		res = bisect_skip(&terms, argv, argc);
 		break;
+	case BISECT_VISUALIZE:
+		get_terms(&terms);
+		res = bisect_visualize(&terms, argv, argc);
+		break;
 	default:
 		BUG("unknown subcommand %d", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 6a7afaea8d..95f7f3fb8c 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -39,29 +39,6 @@  _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
 TERM_BAD=bad
 TERM_GOOD=good
 
-bisect_visualize() {
-	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
-
-	if test $# = 0
-	then
-		if test -n "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}" &&
-			type gitk >/dev/null 2>&1
-		then
-			set gitk
-		else
-			set git log
-		fi
-	else
-		case "$1" in
-		git*|tig) ;;
-		-*)	set git log "$@" ;;
-		*)	set git "$@" ;;
-		esac
-	fi
-
-	eval '"$@"' --bisect -- $(cat "$GIT_DIR/BISECT_NAMES")
-}
-
 bisect_run () {
 	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
 
@@ -152,7 +129,7 @@  case "$#" in
 		# Not sure we want "next" at the UI level anymore.
 		git bisect--helper --bisect-next "$@" || exit ;;
 	visualize|view)
-		bisect_visualize "$@" ;;
+		git bisect--helper --bisect-visualize "$@" || exit;;
 	reset)
 		git bisect--helper --bisect-reset "$@" ;;
 	replay)