{"thread":{"id":"39590","subject":"[PATCH v2 5/7] bisect: change read_bisect_terms parameters","startedAt":"2015-06-10T19:01:56Z","lastAt":"2015-06-17T08:18:20Z","messageCount":23,"participants":["Antoine Delaite","Junio C Hamano","Matthieu Moy","Louis-Alexandre Stuber"],"isPatch":true,"patchVersion":2,"patchTotal":7},"messages":[{"id":"263520","messageId":"1433962918-6536-1-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","threadId":"39590","inReplyTo":null,"subject":"[PATCH v2 5/7] bisect: change read_bisect_terms parameters","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-10T19:01:56Z","receivedAt":"2015-06-10T19:01:56Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"From: Louis Stuber \n\nThe function reads BISECT_TERMS and stores it at the adress given in\nparameters (instead of global variables name_bad and name_good).\n\nThis allows to use the function outside bisect.c.\n\nSigned-off-by: Antoine Delaite \nSigned-off-by: Louis Stuber \n---\n bisect.c | 15 +++++++--------\n 1 files changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex eaa85b6..7afd335 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -908,12 +908,11 @@ static void show_diff_tree(const char *prefix, struct commit *commit)\n }\n \n /*\n- * The terms used for this bisect session are stored in\n- * BISECT_TERMS: it can be bad/good or new/old.\n- * We read them and store them to adapt the messages\n- * accordingly. Default is bad/good.\n+ * The terms used for this bisect session are stored in BISECT_TERMS.\n+ * We read them and store them to adapt the messages accordingly.\n+ * Default is bad/good.\n */\n-void read_bisect_terms(void)\n+void read_bisect_terms(const char **read_bad, const char **read_good)\n {\n \tstruct strbuf str = STRBUF_INIT;\n \tconst char *filename = git_path(\"BISECT_TERMS\");\n@@ -924,9 +923,9 @@ void read_bisect_terms(void)\n \t\t\tstrerror(errno));\n \t} else {\n \t\tstrbuf_getline(&str, fp, '\\n');\n-\t\tname_bad = strbuf_detach(&str, NULL);\n+\t\t*read_bad = strbuf_detach(&str, NULL);\n \t\tstrbuf_getline(&str, fp, '\\n');\n-\t\tname_good = strbuf_detach(&str, NULL);\n+\t\t*read_good = strbuf_detach(&str, NULL);\n \t}\n \tstrbuf_release(&str);\n \tfclose(fp);\n@@ -948,7 +947,7 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \tconst unsigned char *bisect_rev;\n \tchar bisect_rev_hex[GIT_SHA1_HEXSZ + 1];\n \n-\tread_bisect_terms();\n+\tread_bisect_terms(&name_bad, &name_good);\n \tif (read_bisect_refs())\n \t\tdie(\"reading bisect refs failed\");\n \n-- \n1.7.1\n"},{"id":"263521","messageId":"1433962918-6536-2-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","threadId":"39590","inReplyTo":"1433962918-6536-1-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","subject":"[PATCH v2 6/7] revision: fix rev-list --bisect in old/new mode","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-10T19:01:57Z","receivedAt":"2015-06-10T19:01:57Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"From: Louis Stuber \n\nCalling git rev-list --bisect when an old/new mode bisection was started\nshows the help notice. This has been fixed by reading BISECT_TERMS in\nrevision.c to find the correct bisect refs path (which was always\nrefs/bisect/bad (or good) before and can be refs/bisect/new (old) now).\n\nSigned-off-by: Louis Stuber \nSigned-off-by: Antoine Delaite \n---\n revision.c | 16 ++++++++++++++--\n 1 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 7ddbaa0..a332270 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -21,6 +21,9 @@\n \n volatile show_early_output_fn_t show_early_output;\n \n+static const char *name_bad;\n+static const char *name_good;\n+\n char *path_name(const struct name_path *path, const char *name)\n {\n \tconst struct name_path *p;\n@@ -2073,14 +2076,22 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n \tctx->argc -= n;\n }\n \n+extern void read_bisect_terms(const char **bad, const char **good);\n+\n static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n {\n-\treturn for_each_ref_in_submodule(submodule, \"refs/bisect/bad\", fn, cb_data);\n+\tchar bisect_refs_path[256];\n+\tstrcpy(bisect_refs_path, \"refs/bisect/\");\n+\tstrcat(bisect_refs_path, name_bad);\n+\treturn for_each_ref_in_submodule(submodule, bisect_refs_path, fn, cb_data);\n }\n \n static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n {\n-\treturn for_each_ref_in_submodule(submodule, \"refs/bisect/good\", fn, cb_data);\n+\tchar bisect_refs_path[256];\n+\tstrcpy(bisect_refs_path, \"refs/bisect/\");\n+\tstrcat(bisect_refs_path, name_good);\n+\treturn for_each_ref_in_submodule(submodule, bisect_refs_path, fn, cb_data);\n }\n \n static int handle_revision_pseudo_opt(const char *submodule,\n@@ -2109,6 +2120,7 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \t\thandle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);\n \t\tclear_ref_exclusion(&revs->ref_excludes);\n \t} else if (!strcmp(arg, \"--bisect\")) {\n+\t\tread_bisect_terms(&name_bad, &name_good);\n \t\thandle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);\n \t\thandle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);\n \t\trevs->bisect = 1;\n-- \n1.7.1\n"},{"id":"263522","messageId":"1433962918-6536-3-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","threadId":"39590","inReplyTo":"1433962918-6536-1-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","subject":"[PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-10T19:01:58Z","receivedAt":"2015-06-10T19:01:58Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"Introduction of the git bisect terms function.\nThe user can set its own terms.\n\nList of known commands not available :\n`git bisect replay`\n`git bisect terms term1 term2\nthen\ngit bisect start bad_rev good_rev`\n\nSigned-off-by: Antoine Delaite \nSigned-off-by: Louis Stuber \n---\n Documentation/git-bisect.txt | 19 ++++++++++++++++++\n git-bisect.sh | 44 ++++++++++++++++++++++++++++++++++-------\n 2 files changed, 55 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt\nindex 3c3021a..ef0c03c 100644\n--- a/Documentation/git-bisect.txt\n+++ b/Documentation/git-bisect.txt\n@@ -133,6 +133,25 @@ You must run `git bisect start` without commits as argument and run\n `git bisect new `/`git bisect old ...` after to add the\n commits.\n \n+Alternative terms: use your own terms\n+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n+\n+If the builtins terms bad/good and new/old do not satisfy you, you can\n+set your own terms.\n+\n+------------------------------------------------\n+git bisect terms term1 term2\n+------------------------------------------------\n+\n+This command has to be used before a bisection has started.\n+The term1 must be associated with the latest revisions and term2 with the\n+ancestors of term1.\n+\n+Only the first bisection following the 'git bisect terms' will use the terms.\n+If you mistyped one of the terms you can do again 'git bisect terms term1\n+term2'.\n+\n+\n Bisect visualize\n ~~~~~~~~~~~~~~~~\n \ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex c012f5d..22d65b1 100644\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'\n+USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'\n LONG_USAGE='git bisect help\n \tprint this long help message.\n git bisect start [--no-checkout] [ [...]] [--] [...]\n@@ -11,6 +11,8 @@ git bisect (bad|new) []\n git bisect (good|old) [...]\n \tmark ... known-good revisions/\n \t\trevisions before change in a given property.\n+git bisect terms term1 term2\n+\tset up term1 and term2 as bisection terms.\n git bisect skip [(|)...]\n \tmark ... untestable revisions.\n git bisect next\n@@ -79,9 +81,16 @@ bisect_start() {\n \torig_args=$(git rev-parse --sq-quote \"$@\")\n \tbad_seen=0\n \teval=''\n-\t# start_bad_good is used to detect if we did a \n-\t# 'git bisect start bad_rev good_rev'\n-\tstart_bad_good=0\n+\t# terms_defined is used to detect if we did a\n+\t# 'git bisect start bad_rev good_rev' or if the user\n+\t# defined his own terms with git bisect terms\n+\tterms_defined=0\n+\tif test -s \"$GIT_DIR/TERMS_DEFINED\"\n+\tthen\n+\t\tterms_defined=1\n+\t\tget_terms\n+\t\trm -rf \"$GIT_DIR/TERMS_DEFINED\"\n+\tfi\n \tif test \"z$(git rev-parse --is-bare-repository)\" != zfalse\n \tthen\n \t\tmode=--no-checkout\n@@ -107,7 +116,7 @@ bisect_start() {\n \t\t\t\tbreak\n \t\t\t}\n \n-\t\t\tstart_bad_good=1\n+\t\t\tterms_defined=1\n \n \t\t\tcase $bad_seen in\n \t\t\t0) state=$NAME_BAD ; bad_seen=1 ;;\n@@ -180,7 +189,7 @@ bisect_start() {\n \t} &&\n \tgit rev-parse --sq-quote \"$@\" >\"$GIT_DIR/BISECT_NAMES\" &&\n \teval \"$eval true\" &&\n-\tif test $start_bad_good -eq 1 && test ! -s \"$GIT_DIR/BISECT_TERMS\"\n+\tif test $terms_defined -eq 1 && test ! -s \"$GIT_DIR/BISECT_TERMS\"\n \tthen\n \t\techo \"$NAME_BAD\" >\"$GIT_DIR/BISECT_TERMS\" &&\n \t\techo \"$NAME_GOOD\" >>\"$GIT_DIR/BISECT_TERMS\"\n@@ -419,6 +428,7 @@ bisect_clean_state() {\n \trm -f \"$GIT_DIR/BISECT_NAMES\" &&\n \trm -f \"$GIT_DIR/BISECT_RUN\" &&\n \trm -f \"$GIT_DIR/BISECT_TERMS\" &&\n+\trm -f \"$GIT_DIR/TERMS_DEFINED\" &&\n \t# Cleanup head-name if it got left by an old version of git-bisect\n \trm -f \"$GIT_DIR/head-name\" &&\n \tgit update-ref -d --no-deref BISECT_HEAD &&\n@@ -529,7 +539,8 @@ get_terms () {\n check_and_set_terms () {\n \tcmd=\"$1\"\n \tcase \"$cmd\" in\n-\tbad|good|new|old)\n+\tskip) ;;\n+\t*)\n \t\tif test -s \"$GIT_DIR/BISECT_TERMS\" && test \"$cmd\" != \"$NAME_BAD\" && test \"$cmd\" != \"$NAME_GOOD\"\n \t\tthen\n \t\t\tdie \"$(eval_gettext \"Invalid command: you're currently in a \\$NAME_BAD/\\$NAME_GOOD bisect.\")\"\n@@ -562,6 +573,21 @@ bisect_voc () {\n \tesac\n }\n \n+bisect_terms () {\n+\ttest $# -eq 2 ||\n+\tdie \"You need to give me at least two arguments\"\n+\n+\tif ! test -s \"$GIT_DIR/BISECT_START\"\n+\tthen\n+\t\techo $1 >\"$GIT_DIR/BISECT_TERMS\" &&\n+\t\techo $2 >>\"$GIT_DIR/BISECT_TERMS\" &&\n+\t\techo \"1\" > \"$GIT_DIR/TERMS_DEFINED\"\n+\telse\n+\t\tdie \"A bisection has already started, please use \"\\\n+\t\t\"'git bisect reset' to restart and change the terms\"\n+\tfi\n+}\n+\n case \"$#\" in\n 0)\n \tusage ;;\n@@ -574,7 +600,7 @@ case \"$#\" in\n \t\tgit bisect -h ;;\n \tstart)\n \t\tbisect_start \"$@\" ;;\n-\tbad|good|new|old)\n+\tbad|good|new|old|$NAME_BAD|$NAME_GOOD)\n \t\tbisect_state \"$cmd\" \"$@\" ;;\n \tskip)\n \t\tbisect_skip \"$@\" ;;\n@@ -591,6 +617,8 @@ case \"$#\" in\n \t\tbisect_log ;;\n \trun)\n \t\tbisect_run \"$@\" ;;\n+\tterms)\n+\t\tbisect_terms \"$@\" ;;\n \t*)\n \t\tusage ;;\n \tesac\n-- \n1.7.1\n"},{"id":"263534","messageId":"xmqqk2vbi7rf.fsf@gitster.dls.corp.google.com","threadId":"39590","inReplyTo":"1433962918-6536-3-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T21:16:36Z","receivedAt":"2015-06-10T21:16:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Delaite writes:\n\n> -USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'\n> +USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'\n\nI think this patch makes the whole series go in the right direction.\n\nI wonder if you can skip the \"we only support new/old if you are not\ndoing bog-standard bad/good\" step and start from this \"bisect terms\"\none, though.\n\nThen you do not even have to treat new/old any specially, and do not\neven have to list them in the above list.\n\n> @@ -79,9 +81,16 @@ bisect_start() {\n> \torig_args=$(git rev-parse --sq-quote \"$@\")\n> \tbad_seen=0\n> \teval=''\n> -\t# start_bad_good is used to detect if we did a \n> -\t# 'git bisect start bad_rev good_rev'\n> -\tstart_bad_good=0\n> +\t# terms_defined is used to detect if we did a\n> +\t# 'git bisect start bad_rev good_rev' or if the user\n> +\t# defined his own terms with git bisect terms\n> +\tterms_defined=0\n\nI like this change very much; it removes the mysteriously misnamed\nstart-bad-good variable (because you do not really _care_ that\n'start' was what implicitly decided that good/bad pair is the term\nwe use in this session; what you care is that the terms are already\nknown or not).\n\nThat is another reason why I think it would be a better organization\nfor the patch series to do without the intermediate \"we now add new/old\nas another hardcoded values on top of the traditional bad/good\".\n\nThat is, I would think a reasonable progression of the series would\nlook more like these three steps:\n\n - preliminary clean-up steps (e.g. \"correct 'mistook'\");\n\n - use $name_new and $name_old throughout the code, giving them\n 'bad' and 'good' as hardcoded values; finally\n\n - add 'bisect terms' support.\n\nThanks.\n"},{"id":"263539","messageId":"xmqq7frbi4uv.fsf@gitster.dls.corp.google.com","threadId":"39590","inReplyTo":"xmqqk2vbi7rf.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-10T22:19:20Z","receivedAt":"2015-06-10T22:19:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I like this change very much; it removes the mysteriously misnamed\n> start-bad-good variable (because you do not really _care_ that\n> 'start' was what implicitly decided that good/bad pair is the term\n> we use in this session; what you care is that the terms are already\n> known or not).\n>\n> That is another reason why I think it would be a better organization\n> for the patch series to do without the intermediate \"we now add new/old\n> as another hardcoded values on top of the traditional bad/good\".\n>\n> That is, I would think a reasonable progression of the series would\n> look more like these three steps:\n>\n> - preliminary clean-up steps (e.g. \"correct 'mistook'\");\n>\n> - use $name_new and $name_old throughout the code, giving them\n> 'bad' and 'good' as hardcoded values; finally\n>\n> - add 'bisect terms' support.\n\nJust in case I confused readers with a message that apparently\nconflicts with what I said in the ancient thread:\n\n http://thread.gmane.org/gmane.comp.version-control.git/199758/focus=200025\n\nI am admitting that I was wrong. Or perhaps I was right back then,\nbut the world has changed ;-).\n\nWe have been hearing \"bisect bad/good\" is hard to use for the last 3\nyears since that thread discussed this topic, and that made me\nrealize that addition of single new/old may not be good enough, and\nwe should bite the bullet to support 'bisect terms' properly, making\nbad/good and new/old even less special (contrary to what I said back\nthen in that thread \"we only need these two pairs\"), following the\nsuggestion by Phil Hord in that thread.\n\nAnd the suggested three-step approach above reflects that new world\norder in my mind. We admit that the machinery should have been\nbuilt around a value-agnostic \"old vs new\" in the first place, but\nunfortunately it wasn't. So we belatedly update the system to use\nthese two terms internally to express the logic by naming the\nvariables name_old and name_new after these two value-agnostic\nconcepts, and then support the traditional \"good vs bad\" as a mere\ndefault values for the names of old and new states.\n\nOne thing I forgot to say in the review of this patch:\n\n> +bisect_terms () {\n> +\ttest $# -eq 2 ||\n> +\tdie \"You need to give me at least two arguments\"\n> +\n> +\tif ! test -s \"$GIT_DIR/BISECT_START\"\n> +\tthen\n> +\t\techo $1 >\"$GIT_DIR/BISECT_TERMS\" &&\n> +\t\techo $2 >>\"$GIT_DIR/BISECT_TERMS\" &&\n> +\t\techo \"1\" > \"$GIT_DIR/TERMS_DEFINED\"\n> +\telse\n> +\t\tdie \"A bisection has already started, please use \"\\\n> +\t\t\"'git bisect reset' to restart and change the terms\"\n> +\tfi\n> +}\n> +\n\nI think \"git bisect terms\" is a good way to help a user to recall\nwhat two names s/he decided to use for the current session. So\ndying 'already started' with suggestion for 'reset' is OK, but at\nthe same time, helping the user to continue the current bisection by\ngiving a message along the lines of \"You are hunting for a commit\nthat is at the boundary of the old state (you are calling it\n'$NAME_OLD') and the new state ('$NAME_NEW')\" would be a good idea.\n\nThanks.\n"},{"id":"263555","messageId":"vpqoakmoaz7.fsf@anie.imag.fr","threadId":"39590","inReplyTo":"xmqqk2vbi7rf.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-11T09:22:52Z","receivedAt":"2015-06-11T09:22:52Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Antoine Delaite writes:\n>\n>> -USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'\n>> +USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'\n>\n> I think this patch makes the whole series go in the right direction.\n>\n> I wonder if you can skip the \"we only support new/old if you are not\n> doing bog-standard bad/good\" step and start from this \"bisect terms\"\n> one, though.\n\nWhile I think we should not hardcode too much in the code, I also think\nit makes sense to have hardcoded old/new in the user-interface. If you\ngive me just\n\n git bisect terms \n\nthen half of the times, I'll use old/new in the wrong order. And if I\nhadn't looked to bisect close enough, I'd even complain that the terms\nare not usable interchangeably.\n\nAt least, a in a flow like\n\n git bisect start\n git bisect new\n git checkout \n git bisect old\n\nthere's no ambiguity.\n\nSo, to me, having both hardcoded old/new (unambiguous) and \"git bisect\nterms\" (for flexibility) makes sense.\n\nAnother important parameter: the students are finishing their project\ntomorrow. They may continue on their personnal free time, but at best\ntheir time budget is considerably reduced, and from my past experience,\nthe time budget usually reaches 0. That's not a reason to merge bad code\nin git.git, but an incentive to close the series with something going in\nthe right direction. Typically:\n\n> - preliminary clean-up steps (e.g. \"correct 'mistook'\");\n\nThis is straightforward\n\n> - use $name_new and $name_old throughout the code, giving them\n> 'bad' and 'good' as hardcoded values; finally\n\nThis is relatively uncontroversial, and should be easy to finish. I\nwould consider it a good thing anyway.\n\n> - add 'bisect terms' support.\n\nI'd put this on the low-priority whishlist. If the codebase is ready and\na preliminary patch exists, it will be relatively easy for someone else\nto finish the job.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"263556","messageId":"vpqk2vaoa2t.fsf@anie.imag.fr","threadId":"39590","inReplyTo":"xmqq7frbi4uv.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-11T09:42:18Z","receivedAt":"2015-06-11T09:42:18Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>> +bisect_terms () {\n>> +\ttest $# -eq 2 ||\n>> +\tdie \"You need to give me at least two arguments\"\n>> +\n>> +\tif ! test -s \"$GIT_DIR/BISECT_START\"\n>> +\tthen\n>> +\t\techo $1 >\"$GIT_DIR/BISECT_TERMS\" &&\n>> +\t\techo $2 >>\"$GIT_DIR/BISECT_TERMS\" &&\n>> +\t\techo \"1\" > \"$GIT_DIR/TERMS_DEFINED\"\n>> +\telse\n>> +\t\tdie \"A bisection has already started, please use \"\\\n>> +\t\t\"'git bisect reset' to restart and change the terms\"\n>> +\tfi\n>> +}\n>> +\n>\n> I think \"git bisect terms\" is a good way to help a user to recall\n> what two names s/he decided to use for the current session. So\n> dying 'already started' with suggestion for 'reset' is OK, but at\n> the same time, helping the user to continue the current bisection by\n> giving a message along the lines of \"You are hunting for a commit\n> that is at the boundary of the old state (you are calling it\n> '$NAME_OLD') and the new state ('$NAME_NEW')\" would be a good idea.\n\nI'd put a very verbose message explaining the situation and the way out\n(use 'git bisect') for the second \"die\", and I would consider \"git\nbisect terms\" without arguments as a valid command to ask \"please tell\nme what the terms are\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"263590","messageId":"xmqqa8w6gumu.fsf@gitster.dls.corp.google.com","threadId":"39590","inReplyTo":"vpqk2vaoa2t.fsf@anie.imag.fr","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-11T14:57:45Z","receivedAt":"2015-06-11T14:57:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> +bisect_terms () {\n>>> +\ttest $# -eq 2 ||\n>>> +\tdie \"You need to give me at least two arguments\"\n>>> +\n>>> +\tif ! test -s \"$GIT_DIR/BISECT_START\"\n>>> +\tthen\n>>> +\t\techo $1 >\"$GIT_DIR/BISECT_TERMS\" &&\n>>> +\t\techo $2 >>\"$GIT_DIR/BISECT_TERMS\" &&\n>>> +\t\techo \"1\" > \"$GIT_DIR/TERMS_DEFINED\"\n>>> +\telse\n>>> +\t\tdie \"A bisection has already started, please use \"\\\n>>> +\t\t\"'git bisect reset' to restart and change the terms\"\n>>> +\tfi\n>>> +}\n>>> +\n>>\n>> I think \"git bisect terms\" is a good way to help a user to recall\n>> what two names s/he decided to use for the current session. So\n>> dying 'already started' with suggestion for 'reset' is OK, but at\n>> the same time, helping the user to continue the current bisection by\n>> giving a message along the lines of \"You are hunting for a commit\n>> that is at the boundary of the old state (you are calling it\n>> '$NAME_OLD') and the new state ('$NAME_NEW')\" would be a good idea.\n>\n> I'd put a very verbose message explaining the situation and the way out\n> (use 'git bisect') for the second \"die\", and I would consider \"git\n> bisect terms\" without arguments as a valid command to ask \"please tell\n> me what the terms are\".\n\nOf course you are right. The \"remind me what I was doing\" help\nshould be given when the user asks \"git bisect terms\" without any\nparameters, not in \"else die\" part.\n\nThanks for a correction.\n"},{"id":"263591","messageId":"xmqq616ugudn.fsf@gitster.dls.corp.google.com","threadId":"39590","inReplyTo":"vpqoakmoaz7.fsf@anie.imag.fr","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-11T15:03:16Z","receivedAt":"2015-06-11T15:03:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> Antoine Delaite writes:\n>>\n>>> -USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'\n>>> +USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'\n>>\n>> I think this patch makes the whole series go in the right direction.\n>>\n>> I wonder if you can skip the \"we only support new/old if you are not\n>> doing bog-standard bad/good\" step and start from this \"bisect terms\"\n>> one, though.\n>\n> While I think we should not hardcode too much in the code, I also think\n> it makes sense to have hardcoded old/new in the user-interface. \n\nOK.\n"},{"id":"263595","messageId":"vpqy4jq2rjr.fsf@anie.imag.fr","threadId":"39590","inReplyTo":"1433962918-6536-3-git-send-email-antoine.delaite@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-11T15:28:08Z","receivedAt":"2015-06-11T15:28:08Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Antoine Delaite writes:\n\n> -\t# start_bad_good is used to detect if we did a \n> -\t# 'git bisect start bad_rev good_rev'\n> -\tstart_bad_good=0\n> +\t# terms_defined is used to detect if we did a\n> +\t# 'git bisect start bad_rev good_rev' or if the user\n> +\t# defined his own terms with git bisect terms\n> +\tterms_defined=0\n\nModifying in PATCH 7 some code that you introduced in PATCH 3 is\nsuspicious. Is there any reason you did not name the variable\n\"terms_defined\" in the first place? (i.e. squash this hunk and the other\ninstance of start_bad_good into PATCH 3)\n\n(Whether this is a rethorical question is up to you ;-) )\n\n> +\tif test -s \"$GIT_DIR/TERMS_DEFINED\"\n> +\tthen\n> +\t\tterms_defined=1\n> +\t\tget_terms\n> +\t\trm -rf \"$GIT_DIR/TERMS_DEFINED\"\n\nI don't understand why you need to delete this file. I did not review\nthoroughly so there may be a reason, but you can help the reader with a\ncomment here.\n\n> +bisect_terms () {\n> +\ttest $# -eq 2 ||\n> +\tdie \"You need to give me at least two arguments\"\n> +\n> +\tif ! test -s \"$GIT_DIR/BISECT_START\"\n> +\tthen\n> +\t\techo $1 >\"$GIT_DIR/BISECT_TERMS\" &&\n> +\t\techo $2 >>\"$GIT_DIR/BISECT_TERMS\" &&\n> +\t\techo \"1\" > \"$GIT_DIR/TERMS_DEFINED\"\n\nSpace after >.\n\n> @@ -574,7 +600,7 @@ case \"$#\" in\n> \t\tgit bisect -h ;;\n> \tstart)\n> \t\tbisect_start \"$@\" ;;\n> -\tbad|good|new|old)\n> +\tbad|good|new|old|$NAME_BAD|$NAME_GOOD)\n\nSee my other message about quoting.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"263813","messageId":"1155395826.552092.1434285546884.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39590","inReplyTo":"vpqy4jq2rjr.fsf@anie.imag.fr","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Louis-Alexandre Stuber","fromEmail":"stuberl@ensimag.grenoble-inp.fr","sentAt":"2015-06-14T12:39:06Z","receivedAt":"2015-06-14T12:39:06Z","isPatch":true,"sender":{"key":"stuberl@ensimag.grenoble-inp.fr","avatar":null},"body":"Matthieu Moy writes:\n\n> Modifying in PATCH 7 some code that you introduced in PATCH 3 is\n> suspicious. Is there any reason you did not name the variable\n> \"terms_defined\" in the first place? (i.e. squash this hunk and the other\n> instance of start_bad_good into PATCH 3)\n>\n> (Whether this is a rethorical question is up to you ;-) )\n\nIn the previouses versions where we only want to introduce old/new,\nthe terms can only be defined in bisect_start if the user typed\nstart . The name \"start_bad_good\" is not very explicit\nindeed, but isn't it more appropriate in this case than terms_defined ?\n\n> I don't understand why you need to delete this file. I did not review\n> thoroughly so there may be a reason, but you can help the reader with a\n> comment here.\n\nI think it's a mistake. I'd say we should put this test just before the\n\"bisect_clean_state || exit\" line, but that would deserve more attention\nindeed. The idea is to delete the file at the right moment because we \ndon't want it to exist again when the user starts an other bisection,\nbut also have an intelligent behaviour if the start command fails.\n"},{"id":"263823","messageId":"835646176.485512.1434310200681.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39590","inReplyTo":"1155395826.552092.1434285546884.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"[PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-14T19:30:00Z","receivedAt":"2015-06-14T19:30:00Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"Louis Stuber writes: \n\n>Matthieu Moy writes: \n> \n>> Modifying in PATCH 7 some code that you introduced in PATCH 3 is \n>> suspicious. Is there any reason you did not name the variable \n>> \"terms_defined\" in the first place? (i.e. squash this hunk and the other \n>> instance of start_bad_good into PATCH 3) \n>> \n>> (Whether this is a rethorical question is up to you ;-) ) \n> \n>In the previouses versions where we only want to introduce old/new, \n>the terms can only be defined in bisect_start if the user typed \n>start . The name \"start_bad_good\" is not very explicit \n>indeed, but isn't it more appropriate in this case than terms_defined ? \n\nI agree with Louis, but maybe a consistant commit history is more \nimportant. But if only the first patches (which implement old/new ) \nwould come to be accepted the name of the variable would sounds strange. \n\n>> I don't understand why you need to delete this file. I did not review \n>> thoroughly so there may be a reason, but you can help the reader with a \n>> comment here. \n> \n>I think it's a mistake. I'd say we should put this test just before the \n>\"bisect_clean_state || exit\" line, but that would deserve more attention \n>indeed. The idea is to delete the file at the right moment because we \n>don't want it to exist again when the user starts an other bisection, \n>but also have an intelligent behaviour if the start command fails. \n\nYes if the start commands fails, like if you gave wrong sha1, it would be \nnice not to have to type again 'git bisect terms ...' . There is no use to \nput it before clean_state because clean_state because clean_state will \nactually delete the file. So we can just let clean_state do this. \n"},{"id":"263824","messageId":"323480549.485629.1434310820944.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39590","inReplyTo":"vpqy4jq2rjr.fsf@anie.imag.fr","subject":"[PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-14T19:40:20Z","receivedAt":"2015-06-14T19:40:20Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"Matthieu Moy writes:\n\n>> + if test -s \"$GIT_DIR/TERMS_DEFINED\"\n>> + then\n>> + terms_defined=1\n>> + get_terms\n>> + rm -rf \"$GIT_DIR/TERMS_DEFINED\"\n>\n>I don't understand why you need to delete this file. I did not review\n>thoroughly so there may be a reason, but you can help the reader with a\n>comment here.\n\nI will just complete Louis' answer. We delete it with backward\ncompatibility with old/new in mind (even if old/new is not merged yet).\nFor instance, after a old/new mode, if you do a 'bisect start rev1 rev2'\nthe mode would be bad/good ie the default mode. So if you defined your\nterms, we decided it would only be for the following bisection. The next \n'bisect start rev1 rev2' would be in bad/good mode.\nBut this have to be discuted, do the user have to type 'git bisect terms'\neach bisection if he wants to use special terms ?\n"},{"id":"263825","messageId":"771291489.485827.1434312326536.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39590","inReplyTo":"323480549.485629.1434310820944.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"[PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-14T20:05:26Z","receivedAt":"2015-06-14T20:05:26Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"This message's goal is to explained where am I on the bisect \nfunctions. \n\n-git bisect replay now work with any terms. \n-I took account of most of the last reviews (coding style, \ndouble sed ...)\n-'git bisect terms' without arguments now display the current \nterms \n-bisect_terms : if a bisection has already started, a more \nverbose message is displayed \n-I solved a merge conflict in bisect.c due to the update of \nmaster branch. \n\nTo submit a v3 I would need answer about how we rebase the commit \nhistory and what do we do to simplify the life of the user with the \nterms (see my last mails). \nI was thinking of: \n-a config variable that would say :\"as long as I don't reset keep \nthe terms of the previous bisection\" \nor we could decided that this is the default behaviour of bisect. \n\n-two config variables to choose default terms \n\nI may have less time in the next days but I would like to submit a v3. \n"},{"id":"263838","messageId":"vpqoakh74fw.fsf@anie.imag.fr","threadId":"39590","inReplyTo":"835646176.485512.1434310200681.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-15T08:37:23Z","receivedAt":"2015-06-15T08:37:23Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Antoine Delaite writes:\n\n> Louis Stuber writes: \n>\n>>Matthieu Moy writes: \n>> \n>>> Modifying in PATCH 7 some code that you introduced in PATCH 3 is \n>>> suspicious. Is there any reason you did not name the variable \n>>> \"terms_defined\" in the first place? (i.e. squash this hunk and the other \n>>> instance of start_bad_good into PATCH 3) \n>>> \n>>> (Whether this is a rethorical question is up to you ;-) ) \n>> \n>>In the previouses versions where we only want to introduce old/new, \n>>the terms can only be defined in bisect_start if the user typed \n>>start . The name \"start_bad_good\" is not very explicit \n>>indeed, but isn't it more appropriate in this case than terms_defined ? \n>\n> I agree with Louis, but maybe a consistant commit history is more \n> important. But if only the first patches (which implement old/new ) \n> would come to be accepted the name of the variable would sounds strange. \n\nI would say terms_defined is OK even if only the first patches get\nmerged. The reason why you need this variable is because you need to\nknow whether the terms have been defined or not, and to me that's the\nmost important.\n\nI'd suggest something like this:\n\n# terms_defined is 0 when the user did not define the terms explicitely\n# yet. This is the case when running 'git bisect start bad_rev good_rev'\n# before we see an explicit reference to a term.\nterms_defined=0\n\nThen PATCH 7/7 can add a mention of 'git bisect terms' just in the\ncomment.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"263841","messageId":"vpq8ubl73py.fsf@anie.imag.fr","threadId":"39590","inReplyTo":"323480549.485629.1434310820944.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-15T08:52:57Z","receivedAt":"2015-06-15T08:52:57Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Antoine Delaite writes:\n\n> Matthieu Moy writes:\n>\n>>> + if test -s \"$GIT_DIR/TERMS_DEFINED\"\n>>> + then\n>>> + terms_defined=1\n>>> + get_terms\n>>> + rm -rf \"$GIT_DIR/TERMS_DEFINED\"\n>>\n>>I don't understand why you need to delete this file. I did not review\n>>thoroughly so there may be a reason, but you can help the reader with a\n>>comment here.\n>\n> I will just complete Louis' answer. We delete it with backward\n> compatibility with old/new in mind (even if old/new is not merged yet).\n> For instance, after a old/new mode, if you do a 'bisect start rev1 rev2'\n> the mode would be bad/good ie the default mode. So if you defined your\n> terms, we decided it would only be for the following bisection.\n\nI would say \"for the current bisection\", i.e.\n\n$ git bisect start\n$ git bisect terms foo bar # Scope starts here\n...\nThe first 'bar' commit is deadbeef. # Scope ends here\n\n$ git bisect terms foo bar\nYou need to start by \"git bisect start\"\nDo you want me to do it for you [Y/n]?\n\n> The next 'bisect start rev1 rev2' would be in bad/good mode. But this\n> have to be discuted, do the user have to type 'git bisect terms' each\n> bisection if he wants to use special terms ?\n\nTo me, yes. If we allow arbitrary terms, then they will most likely be\nspecific to one bisect session (e.g. if I bisect \"present/absent\" once,\nit tells me nothing about what I'll want for my next bisection).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"263842","messageId":"vpq381t73kg.fsf@anie.imag.fr","threadId":"39590","inReplyTo":"771291489.485827.1434312326536.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-15T08:56:15Z","receivedAt":"2015-06-15T08:56:15Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Antoine Delaite writes:\n\n> To submit a v3 I would need answer about how we rebase the commit \n> history and what do we do to simplify the life of the user with the \n> terms (see my last mails). \n> I was thinking of: \n> -a config variable that would say :\"as long as I don't reset keep \n> the terms of the previous bisection\" \n> or we could decided that this is the default behaviour of bisect. \n>\n> -two config variables to choose default terms \n\nI'd say: keep it simple, try to close this series ASAP. It'll still be\ntime to send another series once this one has settled.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"263863","messageId":"xmqq1thdar9u.fsf@gitster.dls.corp.google.com","threadId":"39590","inReplyTo":"vpqoakh74fw.fsf@anie.imag.fr","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-15T16:08:13Z","receivedAt":"2015-06-15T16:08:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> I would say terms_defined is OK even if only the first patches get\n> merged. The reason why you need this variable is because you need to\n> know whether the terms have been defined or not, and to me that's the\n> most important.\n\nWell said. Thanks.\n\n>\n> I'd suggest something like this:\n>\n> # terms_defined is 0 when the user did not define the terms explicitely\n\ns/citely/citly/;\n\n> # yet. This is the case when running 'git bisect start bad_rev good_rev'\n> # before we see an explicit reference to a term.\n> terms_defined=0\n>\n> Then PATCH 7/7 can add a mention of 'git bisect terms' just in the\n> comment.\n"},{"id":"263973","messageId":"1482001285.562840.1434488820667.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39590","inReplyTo":"vpq8ubl73py.fsf@anie.imag.fr","subject":"[PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-16T21:07:00Z","receivedAt":"2015-06-16T21:07:00Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"Matthieu Moy writes: \n\n>I would say \"for the current bisection\", i.e. \n> \n>$ git bisect start \n>$ git bisect terms foo bar # Scope starts here \n>... \n>The first 'bar' commit is deadbeef. # Scope ends here \n> \n>$ git bisect terms foo bar \n>You need to start by \"git bisect start\" \n>Do you want me to do it for you [Y/n]? \n\nI personnaly don't like this autostart. In the first version of this patch \n7/7 'git bisect terms' has to be called before the start. I think it is \nbetter because we can then use 'git bisect start rev1 rev2 ...' \n\n>> The next 'bisect start rev1 rev2' would be in bad/good mode. But this \n>> have to be discuted, do the user have to type 'git bisect terms' each \n>> bisection if he wants to use special terms ? \n> \n>To me, yes. If we allow arbitrary terms, then they will most likely be \n>specific to one bisect session (e.g. if I bisect \"present/absent\" once, \n>it tells me nothing about what I'll want for my next bisection). \n\nOk. \n\nThanks. \n"},{"id":"263974","messageId":"183774662.562924.1434489533064.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39590","inReplyTo":"vpqoakh74fw.fsf@anie.imag.fr","subject":"[PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-16T21:18:53Z","receivedAt":"2015-06-16T21:18:53Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"Matthieu Moy writes:\n\n># terms_defined is 0 when the user did not define the terms explicitely\n># yet. This is the case when running 'git bisect start bad_rev good_rev'\n># before we see an explicit reference to a term.\n>terms_defined=0\n\nThe thing is:\n'git bisect reset\ngit bisect new HEAD\n(autostart ?) y'\n\nis strictly equivalent to\n\n'git bisect reset\ngit bisect start\ngit bisect new HEAD'\n\nIn both case terms_defined value would be 0 while we kinda know that a\nterm has been used. It just that in the code it is not taken in account yet.\nI don't really mind doing the change but I'm just pointing out that it can be\nconfusing.\n\nThanks.\n"},{"id":"263986","messageId":"vpq1tha4xxq.fsf@anie.imag.fr","threadId":"39590","inReplyTo":"183774662.562924.1434489533064.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-17T07:05:21Z","receivedAt":"2015-06-17T07:05:21Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Antoine Delaite writes:\n\n> Matthieu Moy writes:\n>\n>># terms_defined is 0 when the user did not define the terms explicitely\n>># yet. This is the case when running 'git bisect start bad_rev good_rev'\n>># before we see an explicit reference to a term.\n>>terms_defined=0\n>\n> The thing is:\n> 'git bisect reset\n> git bisect new HEAD\n\n\"git bisect new\" does not exist. Did you mean \"git bisect start HEAD\"?\n\n> (autostart ?) y'\n>\n> is strictly equivalent to\n>\n> 'git bisect reset\n> git bisect start\n> git bisect new HEAD'\n>\n> In both case terms_defined value would be 0 while we kinda know that a\n> term has been used.\n\nI don't understand. The user didn't say either \"bad\" or \"good\", so in\nboth cases we haven't seen a term yet. Or I misunderstood what you meant\nby \"define a term\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"263988","messageId":"575469420.568311.1434528064283.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39590","inReplyTo":"vpq1tha4xxq.fsf@anie.imag.fr","subject":"[PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Antoine Delaite","fromEmail":"antoine.delaite@ensimag.grenoble-inp.fr","sentAt":"2015-06-17T08:01:04Z","receivedAt":"2015-06-17T08:01:04Z","isPatch":true,"sender":{"key":"antoine.delaite@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12510393?v=4"},"body":"Matthieu Moy writes: \n\n>> \n>>># terms_defined is 0 when the user did not define the terms explicitely \n>>># yet. This is the case when running 'git bisect start bad_rev good_rev' \n>>># before we see an explicit reference to a term. \n>>>terms_defined=0 \n>> \n>> The thing is: \n>> 'git bisect reset \n>> git bisect new HEAD \n\n>\"git bisect new\" does not exist. Did you mean \"git bisect start HEAD\"? \n\nNo I meant new but it can be 'git bisect bad' aswell\nSo \n'\ngit bisect reset\ngit bisect bad\nanswer yes to \"autostart ?\"\n'\n\n>I don't understand. The user didn't say either \"bad\" or \"good\", so in \n>both cases we haven't seen a term yet. Or I misunderstood what you meant \n>by \"define a term\". \n\nIn the case I rewrited, we saw a 'bad' but terms_defined value in bisect_start\n(called by the autostart) is 0. \n\n\nThanks.\n"},{"id":"263992","messageId":"vpqioam21f7.fsf@anie.imag.fr","threadId":"39590","inReplyTo":"575469420.568311.1434528064283.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v2 7/7] bisect: allows any terms set by user","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-06-17T08:18:20Z","receivedAt":"2015-06-17T08:18:20Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Antoine Delaite writes:\n\n> Matthieu Moy writes: \n>\n>>> \n>>>># terms_defined is 0 when the user did not define the terms explicitely \n>>>># yet. This is the case when running 'git bisect start bad_rev good_rev' \n>>>># before we see an explicit reference to a term. \n>>>>terms_defined=0 \n>>> \n>>> The thing is: \n>>> 'git bisect reset \n>>> git bisect new HEAD \n>\n>>\"git bisect new\" does not exist. Did you mean \"git bisect start HEAD\"? \n>\n> No I meant new but it can be 'git bisect bad' aswell\n\nOK, answering emails before coffee doesn't suit me well, I did not\nremember that the series was about \"new\" ;-).\n\n(Actually your use-case is not possible yet as of PATCH 3 which\nintroduces start_bad_good. It is possible after PATCH 4)\n\n> So \n> '\n> git bisect reset\n> git bisect bad\n> answer yes to \"autostart ?\"\n> '\n\n> In the case I rewrited, we saw a 'bad' but terms_defined value in bisect_start\n> (called by the autostart) is 0. \n\nAs you said, it is really equivalent to\n\ngit bisect start\ngit bisect bad\n\nthe autostart is just a convenience piece of code to run \"git bisect\nstart\" for the user, but does not change the logic of the code. Write\ngood code for the normal case (start, and then bad), and it will just\nwork without having to worry about in in the autostart case.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}