{"thread":{"id":"54864","subject":"[PATCH v2 5/7] bisect--helper: retire `--bisect-auto-next` subcommand","startedAt":"2020-12-21T18:04:27Z","lastAt":"2021-01-18T23:54:39Z","messageCount":18,"participants":["Miriam Rubio","Rafael Silva","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":2,"patchTotal":7},"messages":[{"id":"412723","messageId":"20201221162743.96056-6-mirucam@gmail.com","threadId":"54864","inReplyTo":"20201221162743.96056-1-mirucam@gmail.com","subject":"[PATCH v2 5/7] bisect--helper: retire `--bisect-auto-next` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-12-21T16:27:41Z","receivedAt":"2020-12-21T18:04:27Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nThe --bisect-auto-next subcommand is no longer used from the\ngit-bisect.sh shell script. Instead the function bisect_auto_next()\nis directly called from the C implementation.\n\nMentored-by: Lars Schneider \nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 10 ----------\n 1 file changed, 10 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex e2e568823e..d570a165de 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -27,7 +27,6 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-start [--term-{new,bad}= --term-{old,good}=]\"\n \t\t\t\t\t \" [--no-checkout] [--first-parent] [ [...]] [--] [...]\"),\n \tN_(\"git bisect--helper --bisect-next\"),\n-\tN_(\"git bisect--helper --bisect-auto-next\"),\n \tN_(\"git bisect--helper --bisect-state (bad|new) []\"),\n \tN_(\"git bisect--helper --bisect-state (good|old) [...]\"),\n \tN_(\"git bisect--helper --bisect-replay \"),\n@@ -1041,7 +1040,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_START,\n \t\tBISECT_AUTOSTART,\n \t\tBISECT_NEXT,\n-\t\tBISECT_AUTO_NEXT,\n \t\tBISECT_STATE,\n \t\tBISECT_LOG,\n \t\tBISECT_REPLAY\n@@ -1060,8 +1058,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"start the bisect session\"), BISECT_START),\n \t\tOPT_CMDMODE(0, \"bisect-next\", &cmdmode,\n \t\t\t N_(\"find the next bisection commit\"), BISECT_NEXT),\n-\t\tOPT_CMDMODE(0, \"bisect-auto-next\", &cmdmode,\n-\t\t\t N_(\"verify the next bisection state then checkout the next bisection commit\"), BISECT_AUTO_NEXT),\n \t\tOPT_CMDMODE(0, \"bisect-state\", &cmdmode,\n \t\t\t N_(\"mark the state of ref (or refs)\"), BISECT_STATE),\n \t\tOPT_CMDMODE(0, \"bisect-log\", &cmdmode,\n@@ -1114,12 +1110,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tget_terms(&terms);\n \t\tres = bisect_next(&terms, prefix);\n \t\tbreak;\n-\tcase BISECT_AUTO_NEXT:\n-\t\tif (argc)\n-\t\t\treturn error(_(\"--bisect-auto-next requires 0 arguments\"));\n-\t\tget_terms(&terms);\n-\t\tres = bisect_auto_next(&terms, prefix);\n-\t\tbreak;\n \tcase BISECT_STATE:\n \t\tset_terms(&terms, \"bad\", \"good\");\n \t\tget_terms(&terms);\n-- \n2.29.2\n\n"},{"id":"412724","messageId":"20201221162743.96056-7-mirucam@gmail.com","threadId":"54864","inReplyTo":"20201221162743.96056-1-mirucam@gmail.com","subject":"[PATCH v2 6/7] bisect--helper: reimplement `bisect_skip` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-12-21T16:27:42Z","receivedAt":"2020-12-21T18:05:56Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nReimplement the `bisect_skip()` shell function in C and also add\n`bisect-skip` subcommand to `git bisect--helper` to call it from\ngit-bisect.sh\n\nUsing `--bisect-skip` subcommand is a temporary measure to port shell\nfunction to C so as to use the existing test suite.\n\nMentored-by: Lars Schneider \nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 47 +++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh | 17 +--------------\n 2 files changed, 47 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex d570a165de..1a6c75183a 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -30,6 +30,7 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-state (bad|new) []\"),\n \tN_(\"git bisect--helper --bisect-state (good|old) [...]\"),\n \tN_(\"git bisect--helper --bisect-replay \"),\n+\tN_(\"git bisect--helper --bisect-skip [(|)...]\"),\n \tNULL\n };\n \n@@ -1030,6 +1031,43 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f\n \treturn bisect_auto_next(terms, NULL);\n }\n \n+static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **argv, int argc)\n+{\n+\tint i;\n+\tenum bisect_error res;\n+\tconst char *pattern = \"*..*\";\n+\tstruct strvec argv_state = STRVEC_INIT;\n+\n+\tstrvec_push(&argv_state, \"skip\");\n+\n+\tfor (i = 0; i < argc; i++) {\n+\t\tif (!wildmatch(pattern, argv[i], 0)) {\n+\t\t\tstruct rev_info revs;\n+\t\t\tstruct commit *commit;\n+\t\t\tstruct strvec rev_argv = STRVEC_INIT;\n+\n+\t\t\tstrvec_pushl(&rev_argv, \"skipped_commits\", argv[i], NULL);\n+\t\t\tinit_revisions(&revs, NULL);\n+\t\t\tsetup_revisions(rev_argv.nr, rev_argv.v, &revs, NULL);\n+\t\t\tstrvec_clear(&rev_argv);\n+\n+\t\t\tif (prepare_revision_walk(&revs))\n+\t\t\t\tdie(_(\"revision walk setup failed\\n\"));\n+\t\t\twhile ((commit = get_revision(&revs)) != NULL)\n+\t\t\t\tstrvec_push(&argv_state,\n+\t\t\t\t\t\toid_to_hex(&commit->object.oid));\n+\n+\t\t\treset_revision_walk();\n+\t\t} else {\n+\t\t\tstrvec_push(&argv_state, argv[i]);\n+\t\t}\n+\t}\n+\tres = bisect_state(terms, argv_state.v, argv_state.nr);\n+\n+\tstrvec_clear(&argv_state);\n+\treturn res;\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -1042,7 +1080,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_NEXT,\n \t\tBISECT_STATE,\n \t\tBISECT_LOG,\n-\t\tBISECT_REPLAY\n+\t\tBISECT_REPLAY,\n+\t\tBISECT_SKIP\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -1064,6 +1103,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"output the contents of BISECT_LOG\"), BISECT_LOG),\n \t\tOPT_CMDMODE(0, \"bisect-replay\", &cmdmode,\n \t\t\t N_(\"replay the bisection process from the given file\"), BISECT_REPLAY),\n+\t\tOPT_CMDMODE(0, \"bisect-skip\", &cmdmode,\n+\t\t\t N_(\"skip some commits for checkout\"), BISECT_SKIP),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -1126,6 +1167,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tset_terms(&terms, \"bad\", \"good\");\n \t\tres = bisect_replay(&terms, argv[0]);\n \t\tbreak;\n+\tcase BISECT_SKIP:\n+\t\tset_terms(&terms, \"bad\", \"good\");\n+\t\tres = bisect_skip(&terms, argv, argc);\n+\t\tbreak;\n \tdefault:\n \t\tBUG(\"unknown subcommand %d\", cmdmode);\n \t}\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 79bcd31bd7..016cc34e03 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -39,21 +39,6 @@ _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n TERM_BAD=bad\n TERM_GOOD=good\n \n-bisect_skip() {\n-\tall=''\n-\tfor arg in \"$@\"\n-\tdo\n-\t\tcase \"$arg\" in\n-\t\t*..*)\n-\t\t\trevs=$(git rev-list \"$arg\") || die \"$(eval_gettext \"Bad rev input: \\$arg\")\" ;;\n-\t\t*)\n-\t\t\trevs=$(git rev-parse --sq-quote \"$arg\") ;;\n-\t\tesac\n-\t\tall=\"$all $revs\"\n-\tdone\n-\teval git bisect--helper --bisect-state 'skip' $all\n-}\n-\n bisect_visualize() {\n \tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n \n@@ -162,7 +147,7 @@ case \"$#\" in\n \tbad|good|new|old|\"$TERM_BAD\"|\"$TERM_GOOD\")\n \t\tgit bisect--helper --bisect-state \"$cmd\" \"$@\" ;;\n \tskip)\n-\t\tbisect_skip \"$@\" ;;\n+\t\tgit bisect--helper --bisect-skip \"$@\" || exit;;\n \tnext)\n \t\t# Not sure we want \"next\" at the UI level anymore.\n \t\tgit bisect--helper --bisect-next \"$@\" || exit ;;\n-- \n2.29.2\n\n"},{"id":"412735","messageId":"20201221162743.96056-2-mirucam@gmail.com","threadId":"54864","inReplyTo":"20201221162743.96056-1-mirucam@gmail.com","subject":"[PATCH v2 1/7] bisect--helper: reimplement `bisect_log` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-12-21T16:27:37Z","receivedAt":"2020-12-21T18:14:47Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nReimplement the `bisect_log()` shell function in C and also add\n`--bisect-log` subcommand to `git bisect--helper` to call it from\ngit-bisect.sh .\n\nUsing `--bisect-log` subcommand is a temporary measure to port shell\nfunction to C so as to use the existing test suite.\n\nMentored-by: Lars Schneider \nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 22 +++++++++++++++++++++-\n git-bisect.sh | 7 +------\n 2 files changed, 22 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 709eb713a3..1854377fa6 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -904,6 +904,18 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a\n \treturn bisect_auto_next(terms, NULL);\n }\n \n+static enum bisect_error bisect_log(void)\n+{\n+\tint fd, status;\n+\tfd = open(git_path_bisect_log(), O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn BISECT_FAILED;\n+\n+\tstatus = copy_fd(fd, STDOUT_FILENO);\n+\tclose(fd);\n+\treturn status ? BISECT_FAILED : BISECT_OK;\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -916,7 +928,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_AUTOSTART,\n \t\tBISECT_NEXT,\n \t\tBISECT_AUTO_NEXT,\n-\t\tBISECT_STATE\n+\t\tBISECT_STATE,\n+\t\tBISECT_LOG\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -938,6 +951,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"verify the next bisection state then checkout the next bisection commit\"), BISECT_AUTO_NEXT),\n \t\tOPT_CMDMODE(0, \"bisect-state\", &cmdmode,\n \t\t\t N_(\"mark the state of ref (or refs)\"), BISECT_STATE),\n+\t\tOPT_CMDMODE(0, \"bisect-log\", &cmdmode,\n+\t\t\t N_(\"output the contents of BISECT_LOG\"), BISECT_LOG),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -1000,6 +1015,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tget_terms(&terms);\n \t\tres = bisect_state(&terms, argv, argc);\n \t\tbreak;\n+\tcase BISECT_LOG:\n+\t\tif (argc)\n+\t\t\treturn error(_(\"--bisect-log requires 0 arguments\"));\n+\t\tres = bisect_log();\n+\t\tbreak;\n \tdefault:\n \t\tBUG(\"unknown subcommand %d\", cmdmode);\n \t}\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex 1f3f6e9fc5..c6149846ff 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -169,11 +169,6 @@ exit code \\$res from '\\$command' is < 0 or >= 128\" >&2\n \tdone\n }\n \n-bisect_log () {\n-\ttest -s \"$GIT_DIR/BISECT_LOG\" || die \"$(gettext \"We are not bisecting.\")\"\n-\tcat \"$GIT_DIR/BISECT_LOG\"\n-}\n-\n get_terms () {\n \tif test -s \"$GIT_DIR/BISECT_TERMS\"\n \tthen\n@@ -210,7 +205,7 @@ case \"$#\" in\n \treplay)\n \t\tbisect_replay \"$@\" ;;\n \tlog)\n-\t\tbisect_log ;;\n+\t\tgit bisect--helper --bisect-log || exit;;\n \trun)\n \t\tbisect_run \"$@\" ;;\n \tterms)\n-- \n2.29.2\n\n"},{"id":"412736","messageId":"20201221162743.96056-1-mirucam@gmail.com","threadId":"54864","inReplyTo":null,"subject":"[PATCH v2 0/7] Finish converting git bisect to C part 3","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-12-21T16:27:36Z","receivedAt":"2020-12-21T18:14:47Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"These patches correspond to a third part of patch series \nof Outreachy project \"Finish converting `git bisect` from shell to C\" \nstarted by Pranit Bauva and Tanushree Tumane\n(https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and\ncontinued by me.\n\nThis third part is formed by reimplementations of some `git bisect` \nsubcommands and removal of some temporary subcommands.\n\nThese patch series emails were generated from:\nhttps://gitlab.com/mirucam/git/commits/git-bisect-work-part3.\n\nGeneral changes\n---------------\n* Rebase on master branch: 6d3ef5b467 (Git 2.30-rc1, 2020-12-18).\n* Change argv_array structs to strvec.\n\nSpecific changes\n----------------\n\n[1/7] bisect--helper: reimplement `bisect_log` shell function in C\n* Add `|| exit` to bisect_log call in shell script.\n---\n\n[2/7] bisect--helper: reimplement `bisect_replay` shell function in C\n* Add `|| exit` to bisect_replay call in shell script.\n---\n\n[6/7] bisect--helper: reimplement `bisect_skip` shell function in C\n* Add `|| exit` to bisect_skip call in shell script.\n---\n\nPranit Bauva (7):\n bisect--helper: reimplement `bisect_log` shell function in C\n bisect--helper: reimplement `bisect_replay` shell function in C\n bisect--helper: retire `--bisect-write` subcommand\n bisect--helper: use `res` instead of return in BISECT_RESET case\n option\n bisect--helper: retire `--bisect-auto-next` subcommand\n bisect--helper: reimplement `bisect_skip` shell function in C\n bisect--helper: retire `--check-and-set-terms` subcommand\n\n builtin/bisect--helper.c | 223 +++++++++++++++++++++++++++++++++------\n git-bisect.sh | 58 +---------\n 2 files changed, 195 insertions(+), 86 deletions(-)\n\n-- \n2.29.2\n\n"},{"id":"412741","messageId":"20201221162743.96056-4-mirucam@gmail.com","threadId":"54864","inReplyTo":"20201221162743.96056-1-mirucam@gmail.com","subject":"[PATCH v2 3/7] bisect--helper: retire `--bisect-write` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-12-21T16:27:39Z","receivedAt":"2020-12-21T18:21:58Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nThe `--bisect-write` subcommand is no longer used from the\ngit-bisect.sh shell script. Instead the function `bisect_write()`\nis directly called from the C implementation.\n\nMentored-by: Lars Schneider \nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 9 ---------\n 1 file changed, 9 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 92c783237d..896c54f869 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -21,7 +21,6 @@ static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n \n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-reset []\"),\n-\tN_(\"git bisect--helper --bisect-write [--no-log] \"),\n \tN_(\"git bisect--helper --bisect-check-and-set-terms \"),\n \tN_(\"git bisect--helper --bisect-next-check []\"),\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n@@ -1036,7 +1035,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n \t\tBISECT_RESET = 1,\n-\t\tBISECT_WRITE,\n \t\tCHECK_AND_SET_TERMS,\n \t\tBISECT_NEXT_CHECK,\n \t\tBISECT_TERMS,\n@@ -1052,8 +1050,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_CMDMODE(0, \"bisect-reset\", &cmdmode,\n \t\t\t N_(\"reset the bisection state\"), BISECT_RESET),\n-\t\tOPT_CMDMODE(0, \"bisect-write\", &cmdmode,\n-\t\t\t N_(\"write out the bisection state in BISECT_LOG\"), BISECT_WRITE),\n \t\tOPT_CMDMODE(0, \"check-and-set-terms\", &cmdmode,\n \t\t\t N_(\"check and set terms in a bisection state\"), CHECK_AND_SET_TERMS),\n \t\tOPT_CMDMODE(0, \"bisect-next-check\", &cmdmode,\n@@ -1090,11 +1086,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tif (argc > 1)\n \t\t\treturn error(_(\"--bisect-reset requires either no argument or a commit\"));\n \t\treturn !!bisect_reset(argc ? argv[0] : NULL);\n-\tcase BISECT_WRITE:\n-\t\tif (argc != 4 && argc != 5)\n-\t\t\treturn error(_(\"--bisect-write requires either 4 or 5 arguments\"));\n-\t\tset_terms(&terms, argv[3], argv[2]);\n-\t\tres = bisect_write(argv[0], argv[1], &terms, nolog);\n \t\tbreak;\n \tcase CHECK_AND_SET_TERMS:\n \t\tif (argc != 3)\n-- \n2.29.2\n\n"},{"id":"412742","messageId":"20201221162743.96056-8-mirucam@gmail.com","threadId":"54864","inReplyTo":"20201221162743.96056-1-mirucam@gmail.com","subject":"[PATCH v2 7/7] bisect--helper: retire `--check-and-set-terms` subcommand","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-12-21T16:27:43Z","receivedAt":"2020-12-21T18:24:51Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nThe `--check-and-set-terms` subcommand is no longer from the\ngit-bisect.sh shell script. Instead the function\n`check_and_set_terms()` is called from the C implementation.\n\nMentored-by: Lars Schneider \nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 10 ----------\n 1 file changed, 10 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 1a6c75183a..eb2a46a4b4 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -21,7 +21,6 @@ static GIT_PATH_FUNC(git_path_bisect_first_parent, \"BISECT_FIRST_PARENT\")\n \n static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-reset []\"),\n-\tN_(\"git bisect--helper --bisect-check-and-set-terms \"),\n \tN_(\"git bisect--helper --bisect-next-check []\"),\n \tN_(\"git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]\"),\n \tN_(\"git bisect--helper --bisect-start [--term-{new,bad}= --term-{old,good}=]\"\n@@ -1072,7 +1071,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n \t\tBISECT_RESET = 1,\n-\t\tCHECK_AND_SET_TERMS,\n \t\tBISECT_NEXT_CHECK,\n \t\tBISECT_TERMS,\n \t\tBISECT_START,\n@@ -1087,8 +1085,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_CMDMODE(0, \"bisect-reset\", &cmdmode,\n \t\t\t N_(\"reset the bisection state\"), BISECT_RESET),\n-\t\tOPT_CMDMODE(0, \"check-and-set-terms\", &cmdmode,\n-\t\t\t N_(\"check and set terms in a bisection state\"), CHECK_AND_SET_TERMS),\n \t\tOPT_CMDMODE(0, \"bisect-next-check\", &cmdmode,\n \t\t\t N_(\"check whether bad or good terms exist\"), BISECT_NEXT_CHECK),\n \t\tOPT_CMDMODE(0, \"bisect-terms\", &cmdmode,\n@@ -1124,12 +1120,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\treturn error(_(\"--bisect-reset requires either no argument or a commit\"));\n \t\tres = bisect_reset(argc ? argv[0] : NULL);\n \t\tbreak;\n-\tcase CHECK_AND_SET_TERMS:\n-\t\tif (argc != 3)\n-\t\t\treturn error(_(\"--check-and-set-terms requires 3 arguments\"));\n-\t\tset_terms(&terms, argv[2], argv[1]);\n-\t\tres = check_and_set_terms(&terms, argv[0]);\n-\t\tbreak;\n \tcase BISECT_NEXT_CHECK:\n \t\tif (argc != 2 && argc != 3)\n \t\t\treturn error(_(\"--bisect-next-check requires 2 or 3 arguments\"));\n-- \n2.29.2\n\n"},{"id":"412744","messageId":"20201221162743.96056-5-mirucam@gmail.com","threadId":"54864","inReplyTo":"20201221162743.96056-1-mirucam@gmail.com","subject":"[PATCH v2 4/7] bisect--helper: use `res` instead of return in BISECT_RESET case option","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-12-21T16:27:40Z","receivedAt":"2020-12-21T18:26:55Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nUse `res` variable to store `bisect_reset()` output in BISECT_RESET\ncase option to make bisect--helper.c more consistent.\n\nMentored-by: Christian Couder \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 896c54f869..e2e568823e 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -1085,7 +1085,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \tcase BISECT_RESET:\n \t\tif (argc > 1)\n \t\t\treturn error(_(\"--bisect-reset requires either no argument or a commit\"));\n-\t\treturn !!bisect_reset(argc ? argv[0] : NULL);\n+\t\tres = bisect_reset(argc ? argv[0] : NULL);\n \t\tbreak;\n \tcase CHECK_AND_SET_TERMS:\n \t\tif (argc != 3)\n-- \n2.29.2\n\n"},{"id":"412746","messageId":"20201221162743.96056-3-mirucam@gmail.com","threadId":"54864","inReplyTo":"20201221162743.96056-1-mirucam@gmail.com","subject":"[PATCH v2 2/7] bisect--helper: reimplement `bisect_replay` shell function in C","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2020-12-21T16:27:38Z","receivedAt":"2020-12-21T18:29:34Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"From: Pranit Bauva \n\nReimplement the `bisect_replay` shell function in C and also add\n`--bisect-replay` subcommand to `git bisect--helper` to call it from\ngit-bisect.sh\n\nUsing `--bisect-replay` subcommand is a temporary measure to port shell\nfunction to C so as to use the existing test suite.\n\nMentored-by: Lars Schneider \nMentored-by: Christian Couder \nMentored-by: Johannes Schindelin \nSigned-off-by: Pranit Bauva \nSigned-off-by: Tanushree Tumane \nSigned-off-by: Miriam Rubio \n---\n builtin/bisect--helper.c | 127 ++++++++++++++++++++++++++++++++++++++-\n git-bisect.sh | 34 +----------\n 2 files changed, 127 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex 1854377fa6..92c783237d 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -31,6 +31,7 @@ static const char * const git_bisect_helper_usage[] = {\n \tN_(\"git bisect--helper --bisect-auto-next\"),\n \tN_(\"git bisect--helper --bisect-state (bad|new) []\"),\n \tN_(\"git bisect--helper --bisect-state (good|old) [...]\"),\n+\tN_(\"git bisect--helper --bisect-replay \"),\n \tNULL\n };\n \n@@ -916,6 +917,121 @@ static enum bisect_error bisect_log(void)\n \treturn status ? BISECT_FAILED : BISECT_OK;\n }\n \n+static int get_next_word(const char *line, int pos, struct strbuf *word)\n+{\n+\tint i, len = strlen(line), begin = 0;\n+\n+\tstrbuf_reset(word);\n+\tfor (i = pos; i < len; i++) {\n+\t\tif (line[i] == ' ' && begin)\n+\t\t\treturn i + 1;\n+\n+\t\tif (!begin)\n+\t\t\tbegin = 1;\n+\t\tstrbuf_addch(word, line[i]);\n+\t}\n+\n+\treturn i;\n+}\n+\n+static int process_line(struct bisect_terms *terms, struct strbuf *line, struct strbuf *word)\n+{\n+\tint res = 0;\n+\tint pos = 0;\n+\n+\twhile (pos < line->len) {\n+\t\tpos = get_next_word(line->buf, pos, word);\n+\n+\t\tif (!strcmp(word->buf, \"git\"))\n+\t\t\tcontinue;\n+\t\telse if (!strcmp(word->buf, \"git-bisect\"))\n+\t\t\tcontinue;\n+\t\telse if (!strcmp(word->buf, \"bisect\"))\n+\t\t\tcontinue;\n+\t\telse if (starts_with(word->buf, \"#\"))\n+\t\t\tbreak;\n+\n+\t\tget_terms(terms);\n+\t\tif (check_and_set_terms(terms, word->buf))\n+\t\t\treturn -1;\n+\n+\t\tif (!strcmp(word->buf, \"start\")) {\n+\t\t\tstruct strvec argv = STRVEC_INIT;\n+\t\t\tint res;\n+\t\t\tsq_dequote_to_strvec(line->buf+pos, &argv);\n+\t\t\tres = bisect_start(terms, argv.v, argv.nr);\n+\t\t\tstrvec_clear(&argv);\n+\t\t\tif (res)\n+\t\t\t\treturn -1;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tif (one_of(word->buf, terms->term_good,\n+\t\t\t terms->term_bad, \"skip\", NULL)) {\n+\t\t\tif (bisect_write(word->buf, line->buf+pos, terms, 0))\n+\t\t\t\treturn -1;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tif (!strcmp(word->buf, \"terms\")) {\n+\t\t\tstruct strvec argv = STRVEC_INIT;\n+\t\t\tint res;\n+\t\t\tsq_dequote_to_strvec(line->buf+pos, &argv);\n+\t\t\tres = bisect_terms(terms, argv.nr == 1 ? argv.v[0] : NULL);\n+\t\t\tstrvec_clear(&argv);\n+\t\t\tif (res)\n+\t\t\t\treturn -1;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\terror(_(\"Replay file contains rubbish (\\\"%s\\\")\"),\n+\t\t word->buf);\n+\t\tres = -1;\n+\t}\n+\treturn res;\n+}\n+\n+static int process_replay_file(FILE *fp, struct bisect_terms *terms)\n+{\n+\tstruct strbuf line = STRBUF_INIT;\n+\tstruct strbuf word = STRBUF_INIT;\n+\tint res = 0;\n+\n+\twhile (strbuf_getline(&line, fp) != EOF) {\n+\t\tres = process_line(terms, &line, &word);\n+\t\tif (res)\n+\t\t\tbreak;\n+\t}\n+\n+\tstrbuf_release(&line);\n+\tstrbuf_release(&word);\n+\treturn res;\n+}\n+\n+static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *filename)\n+{\n+\tFILE *fp = NULL;\n+\tenum bisect_error res = BISECT_OK;\n+\n+\tif (is_empty_or_missing_file(filename))\n+\t\treturn error(_(\"cannot read file '%s' for replaying\"), filename);\n+\n+\tif (bisect_reset(NULL))\n+\t\treturn BISECT_FAILED;\n+\n+\tfp = fopen(filename, \"r\");\n+\tif (!fp)\n+\t\treturn BISECT_FAILED;\n+\n+\tres = process_replay_file(fp, terms);\n+\tfclose(fp);\n+\n+\tif (res)\n+\t\treturn BISECT_FAILED;\n+\n+\treturn bisect_auto_next(terms, NULL);\n+}\n+\n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tenum {\n@@ -929,7 +1045,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\tBISECT_NEXT,\n \t\tBISECT_AUTO_NEXT,\n \t\tBISECT_STATE,\n-\t\tBISECT_LOG\n+\t\tBISECT_LOG,\n+\t\tBISECT_REPLAY\n \t} cmdmode = 0;\n \tint res = 0, nolog = 0;\n \tstruct option options[] = {\n@@ -953,6 +1070,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"mark the state of ref (or refs)\"), BISECT_STATE),\n \t\tOPT_CMDMODE(0, \"bisect-log\", &cmdmode,\n \t\t\t N_(\"output the contents of BISECT_LOG\"), BISECT_LOG),\n+\t\tOPT_CMDMODE(0, \"bisect-replay\", &cmdmode,\n+\t\t\t N_(\"replay the bisection process from the given file\"), BISECT_REPLAY),\n \t\tOPT_BOOL(0, \"no-log\", &nolog,\n \t\t\t N_(\"no log for BISECT_WRITE\")),\n \t\tOPT_END()\n@@ -1020,6 +1139,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n \t\t\treturn error(_(\"--bisect-log requires 0 arguments\"));\n \t\tres = bisect_log();\n \t\tbreak;\n+\tcase BISECT_REPLAY:\n+\t\tif (argc != 1)\n+\t\t\treturn error(_(\"no logfile given\"));\n+\t\tset_terms(&terms, \"bad\", \"good\");\n+\t\tres = bisect_replay(&terms, argv[0]);\n+\t\tbreak;\n \tdefault:\n \t\tBUG(\"unknown subcommand %d\", cmdmode);\n \t}\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex c6149846ff..79bcd31bd7 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -77,38 +77,6 @@ bisect_visualize() {\n \teval '\"$@\"' --bisect -- $(cat \"$GIT_DIR/BISECT_NAMES\")\n }\n \n-bisect_replay () {\n-\tfile=\"$1\"\n-\ttest \"$#\" -eq 1 || die \"$(gettext \"No logfile given\")\"\n-\ttest -r \"$file\" || die \"$(eval_gettext \"cannot read \\$file for replaying\")\"\n-\tgit bisect--helper --bisect-reset || exit\n-\toIFS=\"$IFS\" IFS=\"$IFS$(printf '\\015')\"\n-\twhile read git bisect command rev tail\n-\tdo\n-\t\ttest \"$git $bisect\" = \"git bisect\" || test \"$git\" = \"git-bisect\" || continue\n-\t\tif test \"$git\" = \"git-bisect\"\n-\t\tthen\n-\t\t\trev=\"$command\"\n-\t\t\tcommand=\"$bisect\"\n-\t\tfi\n-\t\tget_terms\n-\t\tgit bisect--helper --check-and-set-terms \"$command\" \"$TERM_GOOD\" \"$TERM_BAD\" || exit\n-\t\tget_terms\n-\t\tcase \"$command\" in\n-\t\tstart)\n-\t\t\teval \"git bisect--helper --bisect-start $rev $tail\" ;;\n-\t\t\"$TERM_GOOD\"|\"$TERM_BAD\"|skip)\n-\t\t\tgit bisect--helper --bisect-write \"$command\" \"$rev\" \"$TERM_GOOD\" \"$TERM_BAD\" || exit;;\n-\t\tterms)\n-\t\t\tgit bisect--helper --bisect-terms $rev || exit;;\n-\t\t*)\n-\t\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n-\t\tesac\n-\tdone <\"$file\"\n-\tIFS=\"$oIFS\"\n-\tgit bisect--helper --bisect-auto-next || exit\n-}\n-\n bisect_run () {\n \tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n \n@@ -203,7 +171,7 @@ case \"$#\" in\n \treset)\n \t\tgit bisect--helper --bisect-reset \"$@\" ;;\n \treplay)\n-\t\tbisect_replay \"$@\" ;;\n+\t\tgit bisect--helper --bisect-replay \"$@\" || exit;;\n \tlog)\n \t\tgit bisect--helper --bisect-log || exit;;\n \trun)\n-- \n2.29.2\n\n"},{"id":"412830","messageId":"gohp6ky2hqzovu.fsf@cpm12071.fritz.box","threadId":"54864","inReplyTo":"20201221162743.96056-3-mirucam@gmail.com","subject":"Re: [PATCH v2 2/7] bisect--helper: reimplement `bisect_replay` shell function in C","fromName":"Rafael Silva","fromEmail":"rafaeloliveira.cs@gmail.com","sentAt":"2020-12-22T00:15:51Z","receivedAt":"2020-12-22T00:16:53Z","isPatch":true,"sender":{"key":"rafaeloliveira.cs@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5935135?v=4"},"body":"\nMiriam Rubio writes:\n\n> From: Pranit Bauva \n>\n> Reimplement the `bisect_replay` shell function in C and also add\n> `--bisect-replay` subcommand to `git bisect--helper` to call it from\n> git-bisect.sh\n>\n> Using `--bisect-replay` subcommand is a temporary measure to port shell\n> function to C so as to use the existing test suite.\n>\n> Mentored-by: Lars Schneider \n> Mentored-by: Christian Couder \n> Mentored-by: Johannes Schindelin \n> Signed-off-by: Pranit Bauva \n> Signed-off-by: Tanushree Tumane \n> Signed-off-by: Miriam Rubio \n> ---\n> builtin/bisect--helper.c | 127 ++++++++++++++++++++++++++++++++++++++-\n> git-bisect.sh | 34 +----------\n> 2 files changed, 127 insertions(+), 34 deletions(-)\n>\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index 1854377fa6..92c783237d 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -31,6 +31,7 @@ static const char * const git_bisect_helper_usage[] = {\n> \tN_(\"git bisect--helper --bisect-auto-next\"),\n> \tN_(\"git bisect--helper --bisect-state (bad|new) []\"),\n> \tN_(\"git bisect--helper --bisect-state (good|old) [...]\"),\n> +\tN_(\"git bisect--helper --bisect-replay \"),\n> \tNULL\n> };\n> \n> @@ -916,6 +917,121 @@ static enum bisect_error bisect_log(void)\n> \treturn status ? BISECT_FAILED : BISECT_OK;\n> }\n> \n> +static int get_next_word(const char *line, int pos, struct strbuf *word)\n> +{\n> +\tint i, len = strlen(line), begin = 0;\n> +\n> +\tstrbuf_reset(word);\n> +\tfor (i = pos; i < len; i++) {\n> +\t\tif (line[i] == ' ' && begin)\n> +\t\t\treturn i + 1;\n> +\n> +\t\tif (!begin)\n> +\t\t\tbegin = 1;\n> +\t\tstrbuf_addch(word, line[i]);\n> +\t}\n> +\n> +\treturn i;\n> +}\n> +\n\nI would like to suggest a slight different approach to handling the \"is\nthe begin of the loop?\" logic. If I understood correctly, the `begin`\nvariable is to check whether is the beginning of the word processing\nand is changed to 1 (aka: to true) on the first loop interaction after\nthe loop is executed for the first time.\n\nHowever, I believe we can check this information in different way that\nwill simplify the logic by removing the \"begin\" and the\n\"if (!begin)...\" altogether. The for-loop is initialize with \"i\" set to\n\"pos\" which means that on the first execution the expression \"i == pos\"\nis going to be true, and \"false\" for the next interactions. Thus, checking\nif \"i\" is different, or better checking if \"i\" is greater should bring\nthe same result.\n\nthat said, the implementation might look like this:\n\n \tstrbuf_reset(word);\n\tfor (i = pos; i < len; i++) {\n\t\tif (line[i] == ' ' && i > pos)\n\t\t\treturn i + 1;\n\n\t\tstrbuf_addch(word, line[i]);\n\t}\n\nWith additionally, removing the \"begin\" from the beginning of the function.\n\nThe above was copied into the email without major tests, although\nI have this implementation locally just to ensure its compiles\nsuccessfully.\n\nPlease take this suggestion as you wish, I do not have any strong\nopinion on the current implementation. Also, I'm a recent contributor to\nthe project and should not be much trusted when proposing changes as when\nproposal comes from project maintainer and/or Senior contributors ;).\n\n> +static int process_line(struct bisect_terms *terms, struct strbuf *line, struct strbuf *word)\n> +{\n> +\tint res = 0;\n> +\tint pos = 0;\n> +\n> +\twhile (pos < line->len) {\n> +\t\tpos = get_next_word(line->buf, pos, word);\n> +\n> +\t\tif (!strcmp(word->buf, \"git\"))\n> +\t\t\tcontinue;\n> +\t\telse if (!strcmp(word->buf, \"git-bisect\"))\n> +\t\t\tcontinue;\n> +\t\telse if (!strcmp(word->buf, \"bisect\"))\n> +\t\t\tcontinue;\n> +\t\telse if (starts_with(word->buf, \"#\"))\n> +\t\t\tbreak;\n> +\n> +\t\tget_terms(terms);\n> +\t\tif (check_and_set_terms(terms, word->buf))\n> +\t\t\treturn -1;\n> +\n> +\t\tif (!strcmp(word->buf, \"start\")) {\n> +\t\t\tstruct strvec argv = STRVEC_INIT;\n> +\t\t\tint res;\n\nI believe this variable is already defined and initialize on the\nbeginning of the function, right? If that is the case then the\ndeclaration seems duplicated here and can be avoided.\n\n> +\t\t\tsq_dequote_to_strvec(line->buf+pos, &argv);\n> +\t\t\tres = bisect_start(terms, argv.v, argv.nr);\n> +\t\t\tstrvec_clear(&argv);\n> +\t\t\tif (res)\n> +\t\t\t\treturn -1;\n\nAlso, (see bellow)\n\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tif (one_of(word->buf, terms->term_good,\n> +\t\t\t terms->term_bad, \"skip\", NULL)) {\n> +\t\t\tif (bisect_write(word->buf, line->buf+pos, terms, 0))\n> +\t\t\t\treturn -1;\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tif (!strcmp(word->buf, \"terms\")) {\n> +\t\t\tstruct strvec argv = STRVEC_INIT;\n> +\t\t\tint res;\n\nIn case this supposed to be the same from the beginning of the function,\nthe declaration seems duplicated here as well. \n\n> +\t\t\tsq_dequote_to_strvec(line->buf+pos, &argv);\n> +\t\t\tres = bisect_terms(terms, argv.nr == 1 ? argv.v[0] : NULL);\n> +\t\t\tstrvec_clear(&argv);\n> +\t\t\tif (res)\n> +\t\t\t\treturn -1;\n> +\t\t\tbreak;\n> +\t\t}\n\nAnother suggestion, again take as you wish, you can place the \"if\"\ndirectly on the call of the \"bisect_start()\" and set the \"res = -1\"\nas the value of \"res\" will be used for the function return anyways.\nAgain with the intent of simplify the implementation.\n\nThe code will look something like the similar:\n\n if (bisect_terms(terms, argv.nr == 1 ? argv.v[0] : NULL))\n res = -1;\n strvec_clear(&argv);\n break;\n\nI did not test the above code thoroughly though.\n\n\n> +\t\terror(_(\"Replay file contains rubbish (\\\"%s\\\")\"),\n> +\t\t word->buf);\n\nI think this will be nicer on the same line ;). Not worth a re-roll\n\n> +\t\tres = -1;\n> +\t}\n> +\treturn res;\n> +}\n> +\n> +static int process_replay_file(FILE *fp, struct bisect_terms *terms)\n> +{\n> +\tstruct strbuf line = STRBUF_INIT;\n> +\tstruct strbuf word = STRBUF_INIT;\n> +\tint res = 0;\n> +\n> +\twhile (strbuf_getline(&line, fp) != EOF) {\n> +\t\tres = process_line(terms, &line, &word);\n> +\t\tif (res)\n> +\t\t\tbreak;\n> +\t}\n> +\n> +\tstrbuf_release(&line);\n> +\tstrbuf_release(&word);\n> +\treturn res;\n> +}\n> +\n\n\n-- \nThanks\nRafael\n"},{"id":"412875","messageId":"gohp6klfdpbca4.fsf@cpm12071.fritz.box","threadId":"54864","inReplyTo":"20201221162743.96056-3-mirucam@gmail.com","subject":"Re: [PATCH v2 2/7] bisect--helper: reimplement `bisect_replay` shell function in C","fromName":"Rafael Silva","fromEmail":"rafaeloliveira.cs@gmail.com","sentAt":"2020-12-23T00:23:49Z","receivedAt":"2020-12-23T00:24:42Z","isPatch":true,"sender":{"key":"rafaeloliveira.cs@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5935135?v=4"},"body":"\nHi Miriam,\n\nMiriam Rubio writes:\n\n> +\n> +static int process_replay_file(FILE *fp, struct bisect_terms *terms)\n> +{\n> +\tstruct strbuf line = STRBUF_INIT;\n> +\tstruct strbuf word = STRBUF_INIT;\n> +\tint res = 0;\n> +\n> +\twhile (strbuf_getline(&line, fp) != EOF) {\n> +\t\tres = process_line(terms, &line, &word);\n> +\t\tif (res)\n> +\t\t\tbreak;\n> +\t}\n> +\n\nI spotted another place where an optimization can be performed.\n\nThe \"if (res) .. break\" conditional is only used to break the loop,\nwhich is the same job of the expression from the while-loop\nitself. Hence, as the purpose is to control the loop execution itself,\nchecking the response of \"process_line()\" via the \"res\" value can be\nmove to the loop expression itself and simplifying the code further,\nas shown on the following patch:\n\n-- >8 --\n\ndiff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\nindex b887413d8d..fb15587af8 100644\n--- a/builtin/bisect--helper.c\n+++ b/builtin/bisect--helper.c\n@@ -988,11 +988,8 @@ static int process_replay_file(FILE *fp, struct bisect_terms *terms)\n struct strbuf word = STRBUF_INIT;\n int res = 0;\n\n- while (strbuf_getline(&line, fp) != EOF) {\n+ while (!res && strbuf_getline(&line, fp) != EOF)\n res = process_line(terms, &line, &word);\n- if (res)\n- break;\n- }\n\n strbuf_release(&line);\n strbuf_release(&word);\n\n-- >8 --\n\nThis also seems to be similar approach from \"one_of()\" introduced by\n4ba1e5c414 (bisect--helper: rewrite `check_term_format` shell function\nin C, 2017-09-29).\n\nOnce more, the intention is to simplify the code and improve the code\nmaintainability.\n\n> +\tstrbuf_release(&line);\n> +\tstrbuf_release(&word);\n> +\treturn res;\n> +}\n> +\n\nI hope this helps increase the code quality.\nHappy Holidays\n\n-- \nThanks\nRafael\n"},{"id":"412876","messageId":"xmqq4kkdfgln.fsf@gitster.c.googlers.com","threadId":"54864","inReplyTo":"gohp6klfdpbca4.fsf@cpm12071.fritz.box","subject":"Re: [PATCH v2 2/7] bisect--helper: reimplement `bisect_replay` shell function in C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-23T01:36:52Z","receivedAt":"2020-12-23T01:37:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rafael Silva writes:\n\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index b887413d8d..fb15587af8 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -988,11 +988,8 @@ static int process_replay_file(FILE *fp, struct bisect_terms *terms)\n> struct strbuf word = STRBUF_INIT;\n> int res = 0;\n>\n> - while (strbuf_getline(&line, fp) != EOF) {\n> + while (!res && strbuf_getline(&line, fp) != EOF)\n> res = process_line(terms, &line, &word);\n> - if (res)\n> - break;\n> - }\n\nI do not mind shorter and crisper code, but I somehow find that the\noriginal more cleanly expresses the intent.\n\n\"We'll grab input lines one by one until the input runs out\" and \"we\nstop when we see a line that process_line() likes\" are conditions\nthat the loop may stop at two logically distinct levels. You can\nconflate them into a single boolean, making it \"unless we found a\nline the process_line() liked in the previous round, grab the next\nline but stop when we ran out the input\", and it may make the result\nshorter, but it may be easier to follow by normal readers if we kept\nthem separate, like the original does.\n"},{"id":"412914","messageId":"gohp6k35zw6drc.fsf@cpm12071.fritz.box","threadId":"54864","inReplyTo":"xmqq4kkdfgln.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 2/7] bisect--helper: reimplement `bisect_replay` shell function in C","fromName":"Rafael Silva","fromEmail":"rafaeloliveira.cs@gmail.com","sentAt":"2020-12-23T10:03:04Z","receivedAt":"2020-12-23T10:04:07Z","isPatch":true,"sender":{"key":"rafaeloliveira.cs@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5935135?v=4"},"body":"\nJunio C Hamano writes:\n\n> Rafael Silva writes:\n>\n>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n>> index b887413d8d..fb15587af8 100644\n>> --- a/builtin/bisect--helper.c\n>> +++ b/builtin/bisect--helper.c\n>> @@ -988,11 +988,8 @@ static int process_replay_file(FILE *fp, struct bisect_terms *terms)\n>> struct strbuf word = STRBUF_INIT;\n>> int res = 0;\n>>\n>> - while (strbuf_getline(&line, fp) != EOF) {\n>> + while (!res && strbuf_getline(&line, fp) != EOF)\n>> res = process_line(terms, &line, &word);\n>> - if (res)\n>> - break;\n>> - }\n>\n> I do not mind shorter and crisper code, but I somehow find that the\n> original more cleanly expresses the intent.\n>\n> \"We'll grab input lines one by one until the input runs out\" and \"we\n> stop when we see a line that process_line() likes\" are conditions\n> that the loop may stop at two logically distinct levels. You can\n> conflate them into a single boolean, making it \"unless we found a\n> line the process_line() liked in the previous round, grab the next\n> line but stop when we ran out the input\", and it may make the result\n> shorter, but it may be easier to follow by normal readers if we kept\n> them separate, like the original does.\n\nThat's a good point (and nice explanation, by the way). Before I was\nthinking more on the line \"while we do not found a good line from\nprocess_line() and we do not finish processing the file, let's go to\nthe next line\" which lead me to proposed changes for shorten the code.\n\nHowever, after your explanation, I can see now and agree the original\ndoes seems easier to follow and we can as it is.\n\n-- \nThanks\nRafael\n"},{"id":"412943","messageId":"xmqqft3w8esq.fsf@gitster.c.googlers.com","threadId":"54864","inReplyTo":"gohp6k35zw6drc.fsf@cpm12071.fritz.box","subject":"Re: [PATCH v2 2/7] bisect--helper: reimplement `bisect_replay` shell function in C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-23T20:09:57Z","receivedAt":"2020-12-23T20:10:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rafael Silva writes:\n\n> That's a good point (and nice explanation, by the way). Before I was\n> thinking more on the line \"while we do not found a good line from\n> process_line() and we do not finish processing the file, let's go to\n> the next line\" which lead me to proposed changes for shorten the code.\n>\n> However, after your explanation, I can see now and agree the original\n> does seems easier to follow and we can as it is.\n\nWell, it is very possible if you come with your version of a similar\nloop in three months in a *new* codepath, I may say that it is a\ngood way to write it. As I said, I do not mind shorter and crisper\ncode.\n\nWhat I am saying is that I may have preference but it is not so\nstrong one in this case, and certainly it is not strong enough to\nsuggest rewriting one way to the other when the initial variant in\nthe patch (which may be either one) is good enough.\n\nThanks.\n"},{"id":"414595","messageId":"nycvar.QRO.7.76.6.2101181555200.52@tvgsbejvaqbjf.bet","threadId":"54864","inReplyTo":"20201221162743.96056-2-mirucam@gmail.com","subject":"Re: [PATCH v2 1/7] bisect--helper: reimplement `bisect_log` shell function in C","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-18T15:02:17Z","receivedAt":"2021-01-18T15:04:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Miriam,\n\nOn Mon, 21 Dec 2020, Miriam Rubio wrote:\n\n> From: Pranit Bauva \n>\n> Reimplement the `bisect_log()` shell function in C and also add\n> `--bisect-log` subcommand to `git bisect--helper` to call it from\n> git-bisect.sh .\n>\n> Using `--bisect-log` subcommand is a temporary measure to port shell\n> function to C so as to use the existing test suite.\n>\n> Mentored-by: Lars Schneider \n> Mentored-by: Christian Couder \n> Mentored-by: Johannes Schindelin \n> Signed-off-by: Pranit Bauva \n> Signed-off-by: Tanushree Tumane \n> Signed-off-by: Miriam Rubio \n\nGood. I see this was originally sent as [PATCH 20/29] in\nhttps://lore.kernel.org/git/20200120143800.900-21-mirucam@gmail.com/, but\nthis version contains improvements:\n\n- It returns `BISECT_FAILED` in `bisect_log()` instead of -1 (and\n `BISECT_OK` instead of 0)\n\n- Instead of checking for `argc > 1` (which was wrong), it now verifies\n that no arguments were passed via `if (argc)`\n\n- It exits from the shell script appropriately when the helper failed\n\nJust one nit:\n\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index 709eb713a3..1854377fa6 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -938,6 +951,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\t\t N_(\"verify the next bisection state then checkout the next bisection commit\"), BISECT_AUTO_NEXT),\n> \t\tOPT_CMDMODE(0, \"bisect-state\", &cmdmode,\n> \t\t\t N_(\"mark the state of ref (or refs)\"), BISECT_STATE),\n> +\t\tOPT_CMDMODE(0, \"bisect-log\", &cmdmode,\n> +\t\t\t N_(\"output the contents of BISECT_LOG\"), BISECT_LOG),\n\nIf this is supposed to be a more permanent subcommand (and\nhttps://git-scm.com/docs/git-bisect#_bisect_log_and_bisect_replay suggests\nit might be), it would probably make more sense to describe the option in\nless implementation-specific detail. Maybe something like:\n\n\tlist the bisection steps so far\n\nCiao,\nDscho\n"},{"id":"414598","messageId":"nycvar.QRO.7.76.6.2101181609480.52@tvgsbejvaqbjf.bet","threadId":"54864","inReplyTo":"20201221162743.96056-3-mirucam@gmail.com","subject":"Re: [PATCH v2 2/7] bisect--helper: reimplement `bisect_replay` shell function in C","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-18T15:59:00Z","receivedAt":"2021-01-18T16:13:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Miriam,\n\nthis patch looks pretty good to me. I just have a couple\ncomments/suggestions:\n\nOn Mon, 21 Dec 2020, Miriam Rubio wrote:\n\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index 1854377fa6..92c783237d 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -916,6 +917,121 @@ static enum bisect_error bisect_log(void)\n> \treturn status ? BISECT_FAILED : BISECT_OK;\n> }\n>\n> +static int get_next_word(const char *line, int pos, struct strbuf *word)\n> +{\n> +\tint i, len = strlen(line), begin = 0;\n> +\n> +\tstrbuf_reset(word);\n> +\tfor (i = pos; i < len; i++) {\n> +\t\tif (line[i] == ' ' && begin)\n> +\t\t\treturn i + 1;\n> +\n> +\t\tif (!begin)\n> +\t\t\tbegin = 1;\n> +\t\tstrbuf_addch(word, line[i]);\n> +\t}\n> +\n> +\treturn i;\n> +}\n\nWhile I have different objections than Rafael (the function seems to want\nto left-trim, but does an inadequate job at it), I do not even think that\nwe need this function. See below.\n\n> +\n> +static int process_line(struct bisect_terms *terms, struct strbuf *line, struct strbuf *word)\n> +{\n> +\tint res = 0;\n> +\tint pos = 0;\n> +\n> +\twhile (pos < line->len) {\n> +\t\tpos = get_next_word(line->buf, pos, word);\n> +\n> +\t\tif (!strcmp(word->buf, \"git\"))\n> +\t\t\tcontinue;\n> +\t\telse if (!strcmp(word->buf, \"git-bisect\"))\n> +\t\t\tcontinue;\n> +\t\telse if (!strcmp(word->buf, \"bisect\"))\n> +\t\t\tcontinue;\n\nThis is not quite correct, as it would skip arbitrary amounts of \"git\" and\n\"git-bisect\" and \"bisect\", even in the middle of the line.\n\nBesides, this `while()` loop iterates over the characters of the current\nline, while the original loop iterated over the _lines_:\n\n\twhile read git bisect command rev\n\tdo\n\t\ttest \"$git $bisect\" = \"git bisect\" || test \"$git\" = \"git-bisect\" || continue\n\t[...]\n\nAs you can see, lines that do not start with \"git bisect\" or \"git-bisect\"\nare simply ignored by `continue`ing to the next line. In the C code,\n`continue` would continue to the next _word_.\n\nI'd like to strongly suggest a very different approach, where no `while()`\nloop is used in `process_line()` (BTW can we please rename this to\n`process_replay_line()` to 1) make it less generic a name and 2) convey\nvia the name what this function is about?):\n\n\tconst char *p;\n\n\tif (!skip_prefix(line->buf, \"git bisect\", &p) &&\n\t !skip_prefix(line->buf, \"git bisect\", p))\n\t\treturn 0;\n\nAs the original code used `read git bisect command rev` to parse the line,\nwhich automatically trims white-space, we might want to do something\nsimilar to that:\n\n\tconst char *p = line->buf + strspn(line->buf, \" \\t\");\n\n\tif ((!skip_prefix(p, \"git bisect\", &p) &&\n\t !skip_prefix(p, \"git-bisect\", &p)) ||\n\t !isspace(*p))\n\t\treturn 0;\n\tp += strspn(p, \" \\t\");\n\n> +\t\telse if (starts_with(word->buf, \"#\"))\n> +\t\t\tbreak;\n\nPlease note that the original shell code is _a lot_ more forgiving: it\nignores _anything_ but \"git bisect\" and \"git-bisect\" at the beginning of\nthe (white-space-trimmed) line. Not just comments starting with `#`. I'd\nlike to return to the shell script's behavior.\n\n> +\n> +\t\tget_terms(terms);\n\nDo we really have to read the terms for every line we replay? I guess we\ndo, as every time we use new/old after good/bad (or vice versa), the terms\nfile gets rewritten.\n\nWe might want to address this awkwardness in the future: in C, we do not\nhave to read and write the terms file _all_ the time.\n\n> +\t\tif (check_and_set_terms(terms, word->buf))\n> +\t\t\treturn -1;\n> +\n> +\t\tif (!strcmp(word->buf, \"start\")) {\n\nLet's use `skip_prefix()` here, too:\n\n\t\tchar *word_end = p + strcspn(p, \" \\t\");\n\t\tconst char *rev = word_end + strspn(word_end, \" \\t\");\n\n\t\t*word_end = '\\0'; /* NUL-terminate the word */\n\n\t\tif (!strcmp(\"start\", p)) {\n\t\t\tstruct strvec argv = STRVEC_INIT;\n\t\t\tint res;\n\n\t\t\tsq_dequote_to_strvec(rev, &argv);\n\t\t\tres = bisect_start(terms, argv.v, argv.nr);\n\t\t\tstrvec_clear(&argv);\n\t\t\treturn res;\n\t\t}\n\n> +\t\t\tstruct strvec argv = STRVEC_INIT;\n> +\t\t\tint res;\n> +\t\t\tsq_dequote_to_strvec(line->buf+pos, &argv);\n> +\t\t\tres = bisect_start(terms, argv.v, argv.nr);\n> +\t\t\tstrvec_clear(&argv);\n> +\t\t\tif (res)\n> +\t\t\t\treturn -1;\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tif (one_of(word->buf, terms->term_good,\n> +\t\t\t terms->term_bad, \"skip\", NULL)) {\n> +\t\t\tif (bisect_write(word->buf, line->buf+pos, terms, 0))\n> +\t\t\t\treturn -1;\n\nApart from using `p` as above instead of `word->buf`, and `rev` instead of\n`line->buf+pos`, why not returning directly what `bisect_write()`\nreturned?\n\n\t\tif (one_of(p, terms->term_good, terms->term_bad, \"skip\", NULL))\n\t\t\treturn bisect_write(p, rev, terms, 0);\n\n> +\t\t\tbreak;\n> +\t\t}\n\n> +\n> +\t\tif (!strcmp(word->buf, \"terms\")) {\n> +\t\t\tstruct strvec argv = STRVEC_INIT;\n> +\t\t\tint res;\n> +\t\t\tsq_dequote_to_strvec(line->buf+pos, &argv);\n> +\t\t\tres = bisect_terms(terms, argv.nr == 1 ? argv.v[0] : NULL);\n\nWe should probably error out if `argv.nr > 1`.\n\n> +\t\t\tstrvec_clear(&argv);\n> +\t\t\tif (res)\n> +\t\t\t\treturn -1;\n> +\t\t\tbreak;\n\nLet's `return res` directly.\n\n> +\t\t}\n> +\n> +\t\terror(_(\"Replay file contains rubbish (\\\"%s\\\")\"),\n> +\t\t word->buf);\n\nThe shell script version does this instead:\n\n\t\tdie \"$(gettext \"?? what are you talking about?\")\" ;;\n\nWe should do the same, if only to make life easier on the translators.\n\n> +\t\tres = -1;\n> +\t}\n> +\treturn res;\n> +}\n> +\n> +static int process_replay_file(FILE *fp, struct bisect_terms *terms)\n> +{\n> +\tstruct strbuf line = STRBUF_INIT;\n> +\tstruct strbuf word = STRBUF_INIT;\n> +\tint res = 0;\n> +\n> +\twhile (strbuf_getline(&line, fp) != EOF) {\n> +\t\tres = process_line(terms, &line, &word);\n> +\t\tif (res)\n> +\t\t\tbreak;\n> +\t}\n> +\n> +\tstrbuf_release(&line);\n> +\tstrbuf_release(&word);\n> +\treturn res;\n> +}\n\nA lot of this function is just boiler plate. I'd prefer to merge the code\ninto `bisect_replay()` instead.\n\n> +\n> +static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *filename)\n> +{\n> +\tFILE *fp = NULL;\n> +\tenum bisect_error res = BISECT_OK;\n> +\n> +\tif (is_empty_or_missing_file(filename))\n> +\t\treturn error(_(\"cannot read file '%s' for replaying\"), filename);\n> +\n> +\tif (bisect_reset(NULL))\n> +\t\treturn BISECT_FAILED;\n> +\n> +\tfp = fopen(filename, \"r\");\n> +\tif (!fp)\n> +\t\treturn BISECT_FAILED;\n> +\n> +\tres = process_replay_file(fp, terms);\n> +\tfclose(fp);\n> +\n> +\tif (res)\n> +\t\treturn BISECT_FAILED;\n> +\n> +\treturn bisect_auto_next(terms, NULL);\n> +}\n\nThe rest looks good to me!\n\nCiao,\nDscho\n"},{"id":"414599","messageId":"nycvar.QRO.7.76.6.2101181707580.52@tvgsbejvaqbjf.bet","threadId":"54864","inReplyTo":"20201221162743.96056-7-mirucam@gmail.com","subject":"Re: [PATCH v2 6/7] bisect--helper: reimplement `bisect_skip` shell function in C","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-18T16:14:23Z","receivedAt":"2021-01-18T16:16:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Miriam,\n\nOn Mon, 21 Dec 2020, Miriam Rubio wrote:\n\n> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c\n> index d570a165de..1a6c75183a 100644\n> --- a/builtin/bisect--helper.c\n> +++ b/builtin/bisect--helper.c\n> @@ -1030,6 +1031,43 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f\n> \treturn bisect_auto_next(terms, NULL);\n> }\n>\n> +static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **argv, int argc)\n> +{\n> +\tint i;\n> +\tenum bisect_error res;\n> +\tconst char *pattern = \"*..*\";\n> +\tstruct strvec argv_state = STRVEC_INIT;\n> +\n> +\tstrvec_push(&argv_state, \"skip\");\n> +\n> +\tfor (i = 0; i < argc; i++) {\n> +\t\tif (!wildmatch(pattern, argv[i], 0)) {\n\nBetter use `strstr()`:\n\n\t\tconst char *dotdot = strstr(argv[i], \"..\");\n\n\t\tif (dotdot) {\n\t\t\t[...]\n\t\t}\n\n> +\t\t\tstruct rev_info revs;\n> +\t\t\tstruct commit *commit;\n> +\t\t\tstruct strvec rev_argv = STRVEC_INIT;\n> +\n> +\t\t\tstrvec_pushl(&rev_argv, \"skipped_commits\", argv[i], NULL);\n> +\t\t\tinit_revisions(&revs, NULL);\n> +\t\t\tsetup_revisions(rev_argv.nr, rev_argv.v, &revs, NULL);\n> +\t\t\tstrvec_clear(&rev_argv);\n\nSince the first argument passed to `setup_revisions()` is always ignored,\nyou can avoid all that `strvec` business altogether thusly:\n\n\t\t\tsetup_revisions(2, argv + i - 1, &revs, NULL);\n\nOther than that, this patch looks good to me.\n\nCiao,\nDscho\n\n> +\n> +\t\t\tif (prepare_revision_walk(&revs))\n> +\t\t\t\tdie(_(\"revision walk setup failed\\n\"));\n> +\t\t\twhile ((commit = get_revision(&revs)) != NULL)\n> +\t\t\t\tstrvec_push(&argv_state,\n> +\t\t\t\t\t\toid_to_hex(&commit->object.oid));\n> +\n> +\t\t\treset_revision_walk();\n> +\t\t} else {\n> +\t\t\tstrvec_push(&argv_state, argv[i]);\n> +\t\t}\n> +\t}\n> +\tres = bisect_state(terms, argv_state.v, argv_state.nr);\n> +\n> +\tstrvec_clear(&argv_state);\n> +\treturn res;\n> +}\n> +\n> int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> {\n> \tenum {\n> @@ -1042,7 +1080,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\tBISECT_NEXT,\n> \t\tBISECT_STATE,\n> \t\tBISECT_LOG,\n> -\t\tBISECT_REPLAY\n> +\t\tBISECT_REPLAY,\n> +\t\tBISECT_SKIP\n> \t} cmdmode = 0;\n> \tint res = 0, nolog = 0;\n> \tstruct option options[] = {\n> @@ -1064,6 +1103,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\t\t N_(\"output the contents of BISECT_LOG\"), BISECT_LOG),\n> \t\tOPT_CMDMODE(0, \"bisect-replay\", &cmdmode,\n> \t\t\t N_(\"replay the bisection process from the given file\"), BISECT_REPLAY),\n> +\t\tOPT_CMDMODE(0, \"bisect-skip\", &cmdmode,\n> +\t\t\t N_(\"skip some commits for checkout\"), BISECT_SKIP),\n> \t\tOPT_BOOL(0, \"no-log\", &nolog,\n> \t\t\t N_(\"no log for BISECT_WRITE\")),\n> \t\tOPT_END()\n> @@ -1126,6 +1167,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n> \t\tset_terms(&terms, \"bad\", \"good\");\n> \t\tres = bisect_replay(&terms, argv[0]);\n> \t\tbreak;\n> +\tcase BISECT_SKIP:\n> +\t\tset_terms(&terms, \"bad\", \"good\");\n> +\t\tres = bisect_skip(&terms, argv, argc);\n> +\t\tbreak;\n> \tdefault:\n> \t\tBUG(\"unknown subcommand %d\", cmdmode);\n> \t}\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index 79bcd31bd7..016cc34e03 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -39,21 +39,6 @@ _x40=\"$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40\"\n> TERM_BAD=bad\n> TERM_GOOD=good\n>\n> -bisect_skip() {\n> -\tall=''\n> -\tfor arg in \"$@\"\n> -\tdo\n> -\t\tcase \"$arg\" in\n> -\t\t*..*)\n> -\t\t\trevs=$(git rev-list \"$arg\") || die \"$(eval_gettext \"Bad rev input: \\$arg\")\" ;;\n> -\t\t*)\n> -\t\t\trevs=$(git rev-parse --sq-quote \"$arg\") ;;\n> -\t\tesac\n> -\t\tall=\"$all $revs\"\n> -\tdone\n> -\teval git bisect--helper --bisect-state 'skip' $all\n> -}\n> -\n> bisect_visualize() {\n> \tgit bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit\n>\n> @@ -162,7 +147,7 @@ case \"$#\" in\n> \tbad|good|new|old|\"$TERM_BAD\"|\"$TERM_GOOD\")\n> \t\tgit bisect--helper --bisect-state \"$cmd\" \"$@\" ;;\n> \tskip)\n> -\t\tbisect_skip \"$@\" ;;\n> +\t\tgit bisect--helper --bisect-skip \"$@\" || exit;;\n> \tnext)\n> \t\t# Not sure we want \"next\" at the UI level anymore.\n> \t\tgit bisect--helper --bisect-next \"$@\" || exit ;;\n> --\n> 2.29.2\n>\n>\n"},{"id":"414600","messageId":"nycvar.QRO.7.76.6.2101181714370.52@tvgsbejvaqbjf.bet","threadId":"54864","inReplyTo":"20201221162743.96056-1-mirucam@gmail.com","subject":"Re: [PATCH v2 0/7] Finish converting git bisect to C part 3","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-01-18T16:15:16Z","receivedAt":"2021-01-18T16:17:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Miriam,\n\nOn Mon, 21 Dec 2020, Miriam Rubio wrote:\n\n> These patches correspond to a third part of patch series\n> of Outreachy project \"Finish converting `git bisect` from shell to C\"\n> started by Pranit Bauva and Tanushree Tumane\n> (https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and\n> continued by me.\n>\n> This third part is formed by reimplementations of some `git bisect`\n> subcommands and removal of some temporary subcommands.\n>\n> These patch series emails were generated from:\n> https://gitlab.com/mirucam/git/commits/git-bisect-work-part3.\n\nNice, thank you very much!\n\nI offered a couple suggestions how I think this patch series could be\nimproved even further.\n\nLooking forward to the next iteration,\nDscho\n\n>\n> General changes\n> ---------------\n> * Rebase on master branch: 6d3ef5b467 (Git 2.30-rc1, 2020-12-18).\n> * Change argv_array structs to strvec.\n>\n> Specific changes\n> ----------------\n>\n> [1/7] bisect--helper: reimplement `bisect_log` shell function in C\n> * Add `|| exit` to bisect_log call in shell script.\n> ---\n>\n> [2/7] bisect--helper: reimplement `bisect_replay` shell function in C\n> * Add `|| exit` to bisect_replay call in shell script.\n> ---\n>\n> [6/7] bisect--helper: reimplement `bisect_skip` shell function in C\n> * Add `|| exit` to bisect_skip call in shell script.\n> ---\n>\n> Pranit Bauva (7):\n> bisect--helper: reimplement `bisect_log` shell function in C\n> bisect--helper: reimplement `bisect_replay` shell function in C\n> bisect--helper: retire `--bisect-write` subcommand\n> bisect--helper: use `res` instead of return in BISECT_RESET case\n> option\n> bisect--helper: retire `--bisect-auto-next` subcommand\n> bisect--helper: reimplement `bisect_skip` shell function in C\n> bisect--helper: retire `--check-and-set-terms` subcommand\n>\n> builtin/bisect--helper.c | 223 +++++++++++++++++++++++++++++++++------\n> git-bisect.sh | 58 +---------\n> 2 files changed, 195 insertions(+), 86 deletions(-)\n>\n> --\n> 2.29.2\n>\n>\n"},{"id":"414645","messageId":"xmqqr1mhiyy9.fsf@gitster.c.googlers.com","threadId":"54864","inReplyTo":"nycvar.QRO.7.76.6.2101181714370.52@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 0/7] Finish converting git bisect to C part 3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-18T23:53:50Z","receivedAt":"2021-01-18T23:54:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi Miriam,\n>\n> On Mon, 21 Dec 2020, Miriam Rubio wrote:\n>\n>> These patches correspond to a third part of patch series\n>> of Outreachy project \"Finish converting `git bisect` from shell to C\"\n>> started by Pranit Bauva and Tanushree Tumane\n>> (https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and\n>> continued by me.\n>>\n>> This third part is formed by reimplementations of some `git bisect`\n>> subcommands and removal of some temporary subcommands.\n>>\n>> These patch series emails were generated from:\n>> https://gitlab.com/mirucam/git/commits/git-bisect-work-part3.\n>\n> Nice, thank you very much!\n>\n> I offered a couple suggestions how I think this patch series could be\n> improved even further.\n>\n> Looking forward to the next iteration,\n> Dscho\n\nThanks for taking time to review a topic. It would be really nice\nto see this topic thru to its end.\n\nAnd of course, thanks Miriam for your continued effort on the\n\"bisect\" topic.\n"}]}