Re: [PATCH v2 6/7] bisect--helper: reimplement `bisect_skip` shell function in C
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jan 18, 2021, 16:14 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2101181707580.52@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20201221162743.96056-7-mirucam@gmail.com>
Hi Miriam,
On Mon, 21 Dec 2020, Miriam Rubio wrote:
Show 19 quoted lines
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index d570a165de..1a6c75183a 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -1030,6 +1031,43 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f
> return bisect_auto_next(terms, NULL);
> }
>
> +static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **argv, int argc)
> +{
> + int i;
> + enum bisect_error res;
> + const char *pattern = "*..*";
> + struct strvec argv_state = STRVEC_INIT;
> +
> + strvec_push(&argv_state, "skip");
> +
> + for (i = 0; i < argc; i++) {
> + if (!wildmatch(pattern, argv[i], 0)) {Better use `strstr()`:
const char *dotdot = strstr(argv[i], "..");
if (dotdot) {
[...]
}Show 8 quoted lines
> + struct rev_info revs; > + struct commit *commit; > + struct strvec rev_argv = STRVEC_INIT; > + > + strvec_pushl(&rev_argv, "skipped_commits", argv[i], NULL); > + init_revisions(&revs, NULL); > + setup_revisions(rev_argv.nr, rev_argv.v, &revs, NULL); > + strvec_clear(&rev_argv);
Since the first argument passed to `setup_revisions()` is always ignored, you can avoid all that `strvec` business altogether thusly:
setup_revisions(2, argv + i - 1, &revs, NULL);
Other than that, this patch looks good to me.
Ciao, Dscho
Show 90 quoted lines
> +
> + if (prepare_revision_walk(&revs))
> + die(_("revision walk setup failed\n"));
> + while ((commit = get_revision(&revs)) != NULL)
> + strvec_push(&argv_state,
> + oid_to_hex(&commit->object.oid));
> +
> + reset_revision_walk();
> + } else {
> + strvec_push(&argv_state, argv[i]);
> + }
> + }
> + res = bisect_state(terms, argv_state.v, argv_state.nr);
> +
> + strvec_clear(&argv_state);
> + return res;
> +}
> +
> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> {
> enum {
> @@ -1042,7 +1080,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> BISECT_NEXT,
> BISECT_STATE,
> BISECT_LOG,
> - BISECT_REPLAY
> + BISECT_REPLAY,
> + BISECT_SKIP
> } cmdmode = 0;
> int res = 0, nolog = 0;
> struct option options[] = {
> @@ -1064,6 +1103,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> N_("output the contents of BISECT_LOG"), BISECT_LOG),
> OPT_CMDMODE(0, "bisect-replay", &cmdmode,
> 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_BOOL(0, "no-log", &nolog,
> N_("no log for BISECT_WRITE")),
> OPT_END()
> @@ -1126,6 +1167,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
> set_terms(&terms, "bad", "good");
> res = bisect_replay(&terms, argv[0]);
> break;
> + case BISECT_SKIP:
> + set_terms(&terms, "bad", "good");
> + res = bisect_skip(&terms, argv, argc);
> + break;
> default:
> BUG("unknown subcommand %d", cmdmode);
> }
> diff --git a/git-bisect.sh b/git-bisect.sh
> index 79bcd31bd7..016cc34e03 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -39,21 +39,6 @@ _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
> TERM_BAD=bad
> TERM_GOOD=good
>
> -bisect_skip() {
> - all=''
> - for arg in "$@"
> - do
> - case "$arg" in
> - *..*)
> - revs=$(git rev-list "$arg") || die "$(eval_gettext "Bad rev input: \$arg")" ;;
> - *)
> - revs=$(git rev-parse --sq-quote "$arg") ;;
> - esac
> - all="$all $revs"
> - done
> - eval git bisect--helper --bisect-state 'skip' $all
> -}
> -
> bisect_visualize() {
> git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
>
> @@ -162,7 +147,7 @@ case "$#" in
> bad|good|new|old|"$TERM_BAD"|"$TERM_GOOD")
> git bisect--helper --bisect-state "$cmd" "$@" ;;
> skip)
> - bisect_skip "$@" ;;
> + git bisect--helper --bisect-skip "$@" || exit;;
> next)
> # Not sure we want "next" at the UI level anymore.
> git bisect--helper --bisect-next "$@" || exit ;;
> --
> 2.29.2
>
>