{"thread":{"id":"43855","subject":"[PATCH 1/2] advice: preset advice preferences to -1","startedAt":"2016-08-15T18:40:32Z","lastAt":"2016-08-15T22:38:16Z","messageCount":10,"participants":["Stefan Beller","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"299370","messageId":"20160815184021.12396-1-sbeller@google.com","threadId":"43855","inReplyTo":null,"subject":"[PATCH 1/2] advice: preset advice preferences to -1","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-15T18:40:20Z","receivedAt":"2016-08-15T18:40:32Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In the next patch we want to make a distinction if a the advice was\nrequested explicitly or is set implicit. To do that we need to have\ndifferent values for the default and a value that is set explicitly.\n\nSigned-off-by: Stefan Beller \n---\n advice.c | 30 +++++++++++++++---------------\n 1 file changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/advice.c b/advice.c\nindex b84ae49..56b70b6 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -1,20 +1,20 @@\n #include \"cache.h\"\n \n-int advice_push_update_rejected = 1;\n-int advice_push_non_ff_current = 1;\n-int advice_push_non_ff_matching = 1;\n-int advice_push_already_exists = 1;\n-int advice_push_fetch_first = 1;\n-int advice_push_needs_force = 1;\n-int advice_status_hints = 1;\n-int advice_status_u_option = 1;\n-int advice_commit_before_merge = 1;\n-int advice_resolve_conflict = 1;\n-int advice_implicit_identity = 1;\n-int advice_detached_head = 1;\n-int advice_set_upstream_failure = 1;\n-int advice_object_name_warning = 1;\n-int advice_rm_hints = 1;\n+int advice_push_update_rejected = -1;\n+int advice_push_non_ff_current = -1;\n+int advice_push_non_ff_matching = -1;\n+int advice_push_already_exists = -1;\n+int advice_push_fetch_first = -1;\n+int advice_push_needs_force = -1;\n+int advice_status_hints = -1;\n+int advice_status_u_option = -1;\n+int advice_commit_before_merge = -1;\n+int advice_resolve_conflict = -1;\n+int advice_implicit_identity = -1;\n+int advice_detached_head = -1;\n+int advice_set_upstream_failure = -1;\n+int advice_object_name_warning = -1;\n+int advice_rm_hints = -1;\n \n static struct {\n \tconst char *name;\n-- \n2.9.2.730.g525ad04.dirty\n\n"},{"id":"299371","messageId":"20160815184021.12396-2-sbeller@google.com","threadId":"43855","inReplyTo":"20160815184021.12396-1-sbeller@google.com","subject":"[PATCH 2/2] checkout: do not mention detach advice for explicit --detach option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-15T18:40:21Z","receivedAt":"2016-08-15T18:40:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When a user asked for a detached HEAD specifically with `--detach`,\nwe do not need to give advice on what a detached HEAD state entails as\nwe can assume they know what they're getting into as they asked for it.\n\nSigned-off-by: Stefan Beller \n---\n\n Junio writes:\n > It might be controversial how the second from the last case should\n > behave, though.\n \n I agree. I think if the advice is configured explicitly we can still give it.\n That makes the code a bit more complicated though.\n \n Also note I added stderr to stdout redirections as suggested by Peff.\n \n Thanks,\n Stefan\n \n builtin/checkout.c | 4 +++-\n t/t2020-checkout-detach.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 4866111..6196b40 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -658,7 +658,9 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n \t\tupdate_ref(msg.buf, \"HEAD\", new->commit->object.oid.hash, NULL,\n \t\t\t REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n \t\tif (!opts->quiet) {\n-\t\t\tif (old->path && advice_detached_head)\n+\t\t\tif (old->path &&\n+\t\t\t (advice_detached_head == 1 ||\n+\t\t\t (advice_detached_head == -1 && !opts->force_detach)))\n \t\t\t\tdetach_advice(new->name);\n \t\t\tdescribe_detached_head(_(\"HEAD is now at\"), new->commit);\n \t\t}\ndiff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh\nindex 5d68729..fe311a1 100755\n--- a/t/t2020-checkout-detach.sh\n+++ b/t/t2020-checkout-detach.sh\n@@ -163,4 +163,32 @@ test_expect_success 'tracking count is accurate after orphan check' '\n \ttest_i18ncmp expect stdout\n '\n \n+test_expect_success 'no advice given for explicit detached head state' '\n+\t# baseline\n+\ttest_config advice.detachedHead true &&\n+\tgit checkout child && git checkout HEAD^0 >expect.advice 2>&1 &&\n+\ttest_config advice.detachedHead false &&\n+\tgit checkout child && git checkout HEAD^0 >expect.no-advice 2>&1 &&\n+\ttest_unconfig advice.detachedHead &&\n+\t# without configuration, the advice.* variables default to true\n+\tgit checkout child && git checkout HEAD^0 >actual 2>&1 &&\n+\ttest_cmp expect.advice actual &&\n+\n+\t# with explicit --detach\n+\t# no configuration\n+\ttest_unconfig advice.detachedHead &&\n+\tgit checkout child && git checkout --detach HEAD^0 >actual 2>&1 &&\n+\ttest_cmp expect.no-advice actual &&\n+\n+\t# explicitly ask advice\n+\ttest_config advice.detachedHead true &&\n+\tgit checkout child && git checkout --detach HEAD^0 >actual 2>&1 &&\n+\ttest_cmp expect.advice actual &&\n+\n+\t# explicitly decline advice\n+\ttest_config advice.detachedHead false &&\n+\tgit checkout child && git checkout --detach HEAD^0 >actual 2>&1 &&\n+\ttest_cmp expect.no-advice actual\n+'\n+\n test_done\n-- \n2.9.2.730.g525ad04.dirty\n\n"},{"id":"299373","messageId":"20160815184730.eevqogqrxp2zp43q@sigill.intra.peff.net","threadId":"43855","inReplyTo":"20160815184021.12396-2-sbeller@google.com","subject":"Re: [PATCH 2/2] checkout: do not mention detach advice for explicit --detach option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-15T18:47:31Z","receivedAt":"2016-08-15T18:47:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 15, 2016 at 11:40:21AM -0700, Stefan Beller wrote:\n\n> When a user asked for a detached HEAD specifically with `--detach`,\n> we do not need to give advice on what a detached HEAD state entails as\n> we can assume they know what they're getting into as they asked for it.\n> \n> Signed-off-by: Stefan Beller \n> ---\n> \n> Junio writes:\n> > It might be controversial how the second from the last case should\n> > behave, though.\n> \n> I agree. I think if the advice is configured explicitly we can still give it.\n> That makes the code a bit more complicated though.\n\nSo....I guess. But has anybody in the history of git ever explicitly\nconfigured advice.* to true?\n\nIt has never produced any change of behavior, and the whole point of\n\"advice.*\" was that git would advise by default, and you would use\nadvice.* to shut it up once you were sufficiently educated.\n\nI don't think doing it this way is _wrong_. It just feels sort of\npointlessly over-engineered. It's also a little weird that all of the:\n\n if (advice_foo)\n\nwill trigger because \"advice_foo\" is set to -1. I think it does the\nright thing, but it feels like a bug (the value is now a tri-state, and\nwe silently collapse two states into one).\n\n-Peff\n"},{"id":"299375","messageId":"CAGZ79kZq=dPsngaqAVOj6UhaJE0=eR==XSTSSQCmcUTKugeM0Q@mail.gmail.com","threadId":"43855","inReplyTo":"20160815184730.eevqogqrxp2zp43q@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] checkout: do not mention detach advice for explicit --detach option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-15T18:54:53Z","receivedAt":"2016-08-15T18:54:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Aug 15, 2016 at 11:47 AM, Jeff King wrote:\n> On Mon, Aug 15, 2016 at 11:40:21AM -0700, Stefan Beller wrote:\n>\n>> When a user asked for a detached HEAD specifically with `--detach`,\n>> we do not need to give advice on what a detached HEAD state entails as\n>> we can assume they know what they're getting into as they asked for it.\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> Junio writes:\n>> > It might be controversial how the second from the last case should\n>> > behave, though.\n>>\n>> I agree. I think if the advice is configured explicitly we can still give it.\n>> That makes the code a bit more complicated though.\n>\n> So....I guess. But has anybody in the history of git ever explicitly\n> configured advice.* to true?\n\nAn admin/teacher of a university that wants to teach Git to students\nin a class? Better be safe than sorry and explicitly ask for advice because...\nwe cannot trust the default?\n\n>\n> It has never produced any change of behavior, and the whole point of\n> \"advice.*\" was that git would advise by default, and you would use\n> advice.* to shut it up once you were sufficiently educated.\n\nAnd now I am arguing that \"by default\" we should not give advice 100%\nof the time, but only when we think it is appropriate. You may disagree\n(as a teacher see above), so you can slightly change the setting to give\nout advice more often again?\n\n>\n> I don't think doing it this way is _wrong_. It just feels sort of\n> pointlessly over-engineered. It's also a little weird that all of the:\n>\n> if (advice_foo)\n>\n> will trigger because \"advice_foo\" is set to -1. I think it does the\n> right thing, but it feels like a bug (the value is now a tri-state, and\n> we silently collapse two states into one).\n\nI think this is what I did in some of the submodule code as well, which\nis why I assumed it's ok (or rather the projects groupthink on how to do\n\"default on but still different than explicit on\")\n\nIf you think this is wrong, what is the idiomatic way to solve this problem?\n\nThanks,\nStefan\n\n\n>\n> -Peff\n"},{"id":"299379","messageId":"20160815190856.27obqcpym4mhxgmo@sigill.intra.peff.net","threadId":"43855","inReplyTo":"CAGZ79kZq=dPsngaqAVOj6UhaJE0=eR==XSTSSQCmcUTKugeM0Q@mail.gmail.com","subject":"Re: [PATCH 2/2] checkout: do not mention detach advice for explicit --detach option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-15T19:08:57Z","receivedAt":"2016-08-15T19:15:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 15, 2016 at 11:54:53AM -0700, Stefan Beller wrote:\n\n> > So....I guess. But has anybody in the history of git ever explicitly\n> > configured advice.* to true?\n> \n> An admin/teacher of a university that wants to teach Git to students\n> in a class? Better be safe than sorry and explicitly ask for advice because...\n> we cannot trust the default?\n> \n> >\n> > It has never produced any change of behavior, and the whole point of\n> > \"advice.*\" was that git would advise by default, and you would use\n> > advice.* to shut it up once you were sufficiently educated.\n> \n> And now I am arguing that \"by default\" we should not give advice 100%\n> of the time, but only when we think it is appropriate. You may disagree\n> (as a teacher see above), so you can slightly change the setting to give\n> out advice more often again?\n\nI don't think it's quite the same thing. It is fine not to bother\nadvising because the advice is not really applicable, and that is what\nis happening here. We do not need to lecture the user on something they\nexplicitly asked for.\n\nBut that is different than \"by default, in situations where we have\nuseful advice to give, give it\".\n\nI guess you are indicating that somebody may disagree on \"applicable\"\nhere. Which I suppose is possible, but it seems like a bit of a made-up\nhypothetical.\n\n\nI had also thought at first you were arguing from the position of \"let's\nhandle advice.detachedHEAD=true in case somebody has set it\". That seems\neven more silly, because almost certainly nobody _has_ set it. But if\nyour position is \"let's make it do something useful in case somebody\nwants to set it\", then...I still think it's silly, but at least there is\nroom for debate. :)\n\n> > I don't think doing it this way is _wrong_. It just feels sort of\n> > pointlessly over-engineered. It's also a little weird that all of the:\n> >\n> > if (advice_foo)\n> >\n> > will trigger because \"advice_foo\" is set to -1. I think it does the\n> > right thing, but it feels like a bug (the value is now a tri-state, and\n> > we silently collapse two states into one).\n> \n> I think this is what I did in some of the submodule code as well, which\n> is why I assumed it's ok (or rather the projects groupthink on how to do\n> \"default on but still different than explicit on\")\n> \n> If you think this is wrong, what is the idiomatic way to solve this problem?\n\nI don't think it's wrong (didn't I say so :) ).\n\nIt's just that idiomatic use of a tri-state like this is generally\nsomething like:\n\n 1. Set the option to -1 for \"not specified\"\n\n 2. Fill in the values as 0/1 if the user asks for it.\n\n 3. Canonicalize any remaining unspecified value to 0/1, depending on\n the default. Usually this happens after we know all setup is done,\n but sometimes is done lazily by an accessor function.\n\n 3. Check the canonical value with \"if (option)\", or \"if (option())\" if\n using a lazy accessor.\n\nAnd we have fixed many bugs in the past where some non-canonical value\nslipped past step 3, and did the wrong thing in step 4.\n\nHere it's OK because \"if (option)\" means that \"unspecified\" collapses to\n\"true\", and that is the default for each of these options. It's just\nthat it's hard to distinguish from the buggy case.\n\nI suppose the more idiomatic way would be:\n\n static int advice_wanted(int x)\n {\n\tif (advice_values[x] < 0)\n\t\tadvice_values[x] = 1; /* all advice defaults to on */\n\treturn advice_values[x];\n }\n\n ...\n if (advice_wanted(ADVICE_DETACHED_HEAD))\n\t...\n\nbut perhaps it is not worth that amount of boilerplate (but maybe at\nleast a comment explaining the situation). You'd also need to check with\nyour patch that each user of the advice variables is checking just for a\nnon-zero value, and not explicitly for \"1\" (which is almost certainly\nthe case, but it needs to be considered).\n\n-Peff\n"},{"id":"299399","messageId":"xmqqh9alg2uh.fsf@gitster.mtv.corp.google.com","threadId":"43855","inReplyTo":"20160815184730.eevqogqrxp2zp43q@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] checkout: do not mention detach advice for explicit --detach option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-15T21:05:26Z","receivedAt":"2016-08-15T21:05:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I don't think doing it this way is _wrong_. It just feels sort of\n> pointlessly over-engineered. It's also a little weird that all of the:\n>\n> if (advice_foo)\n>\n> will trigger because \"advice_foo\" is set to -1. I think it does the\n> right thing, but it feels like a bug (the value is now a tri-state, and\n> we silently collapse two states into one).\n\nGuilty as charged. I do agree that this is over-engineered.\n"},{"id":"299400","messageId":"xmqqd1l9g2lv.fsf@gitster.mtv.corp.google.com","threadId":"43855","inReplyTo":"xmqqh9alg2uh.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] checkout: do not mention detach advice for explicit --detach option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-15T21:10:36Z","receivedAt":"2016-08-15T21:10:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> I don't think doing it this way is _wrong_. It just feels sort of\n>> pointlessly over-engineered. It's also a little weird that all of the:\n>>\n>> if (advice_foo)\n>>\n>> will trigger because \"advice_foo\" is set to -1. I think it does the\n>> right thing, but it feels like a bug (the value is now a tri-state, and\n>> we silently collapse two states into one).\n>\n> Guilty as charged. I do agree that this is over-engineered.\n\nLet's discard 1/2 and amend 2/2 with this incremental.\n\n\n\n builtin/checkout.c | 3 +--\n t/t2020-checkout-detach.sh | 5 -----\n 2 files changed, 1 insertion(+), 7 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 2a32b5f..337c35a 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -656,8 +656,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n \t\t\t REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n \t\tif (!opts->quiet) {\n \t\t\tif (old->path &&\n-\t\t\t (advice_detached_head == 1 ||\n-\t\t\t (advice_detached_head == -1 && !opts->force_detach)))\n+\t\t\t advice_detached_head == 1 && !opts->force_detach)\n \t\t\t\tdetach_advice(new->name);\n \t\t\tdescribe_detached_head(_(\"HEAD is now at\"), new->commit);\n \t\t}\ndiff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh\nindex fe311a1..fbb4ee9 100755\n--- a/t/t2020-checkout-detach.sh\n+++ b/t/t2020-checkout-detach.sh\n@@ -180,11 +180,6 @@ test_expect_success 'no advice given for explicit detached head state' '\n \tgit checkout child && git checkout --detach HEAD^0 >actual 2>&1 &&\n \ttest_cmp expect.no-advice actual &&\n \n-\t# explicitly ask advice\n-\ttest_config advice.detachedHead true &&\n-\tgit checkout child && git checkout --detach HEAD^0 >actual 2>&1 &&\n-\ttest_cmp expect.advice actual &&\n-\n \t# explicitly decline advice\n \ttest_config advice.detachedHead false &&\n \tgit checkout child && git checkout --detach HEAD^0 >actual 2>&1 &&\n"},{"id":"299401","messageId":"20160815211354.vsyg3lpira6pttom@sigill.intra.peff.net","threadId":"43855","inReplyTo":"xmqqd1l9g2lv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] checkout: do not mention detach advice for explicit --detach option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-15T21:13:54Z","receivedAt":"2016-08-15T21:14:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 15, 2016 at 02:10:36PM -0700, Junio C Hamano wrote:\n\n> > Guilty as charged. I do agree that this is over-engineered.\n> \n> Let's discard 1/2 and amend 2/2 with this incremental.\n\nLooks good (though I really am OK the other way if people feel\nstrongly).\n\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 2a32b5f..337c35a 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -656,8 +656,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n> \t\t\t REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n> \t\tif (!opts->quiet) {\n> \t\t\tif (old->path &&\n> -\t\t\t (advice_detached_head == 1 ||\n> -\t\t\t (advice_detached_head == -1 && !opts->force_detach)))\n> +\t\t\t advice_detached_head == 1 && !opts->force_detach)\n\nMaybe s/== 1//?\n\nThat's more idiomatic, though maybe you were future-proofing in case we\nturn it into a tri-state later. :)\n\n-Peff\n"},{"id":"299402","messageId":"CAGZ79kayr9p9qGrH563_-D-WpWvcgzmsQMrPxkxTptbLzp+4oA@mail.gmail.com","threadId":"43855","inReplyTo":"xmqqd1l9g2lv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] checkout: do not mention detach advice for explicit --detach option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-15T21:17:59Z","receivedAt":"2016-08-15T21:18:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Aug 15, 2016 at 2:10 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Jeff King writes:\n>>\n>>> I don't think doing it this way is _wrong_. It just feels sort of\n>>> pointlessly over-engineered. It's also a little weird that all of the:\n>>>\n>>> if (advice_foo)\n>>>\n>>> will trigger because \"advice_foo\" is set to -1. I think it does the\n>>> right thing, but it feels like a bug (the value is now a tri-state, and\n>>> we silently collapse two states into one).\n>>\n>> Guilty as charged. I do agree that this is over-engineered.\n>\n> Let's discard 1/2 and amend 2/2 with this incremental.\n\nThat is fine with me.\n\nThanks,\nStefan\n\n>\n>\n>\n> builtin/checkout.c | 3 +--\n> t/t2020-checkout-detach.sh | 5 -----\n> 2 files changed, 1 insertion(+), 7 deletions(-)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 2a32b5f..337c35a 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -656,8 +656,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n> REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n> if (!opts->quiet) {\n> if (old->path &&\n> - (advice_detached_head == 1 ||\n> - (advice_detached_head == -1 && !opts->force_detach)))\n> + advice_detached_head == 1 && !opts->force_detach)\n> detach_advice(new->name);\n> describe_detached_head(_(\"HEAD is now at\"), new->commit);\n> }\n> diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh\n> index fe311a1..fbb4ee9 100755\n> --- a/t/t2020-checkout-detach.sh\n> +++ b/t/t2020-checkout-detach.sh\n> @@ -180,11 +180,6 @@ test_expect_success 'no advice given for explicit detached head state' '\n> git checkout child && git checkout --detach HEAD^0 >actual 2>&1 &&\n> test_cmp expect.no-advice actual &&\n>\n> - # explicitly ask advice\n> - test_config advice.detachedHead true &&\n> - git checkout child && git checkout --detach HEAD^0 >actual 2>&1 &&\n> - test_cmp expect.advice actual &&\n> -\n> # explicitly decline advice\n> test_config advice.detachedHead false &&\n> git checkout child && git checkout --detach HEAD^0 >actual 2>&1 &&\n"},{"id":"299418","messageId":"xmqqvaz1ejzk.fsf@gitster.mtv.corp.google.com","threadId":"43855","inReplyTo":"20160815211354.vsyg3lpira6pttom@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] checkout: do not mention detach advice for explicit --detach option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-15T22:38:07Z","receivedAt":"2016-08-15T22:38:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> \t\t\t REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n>> \t\tif (!opts->quiet) {\n>> \t\t\tif (old->path &&\n>> -\t\t\t (advice_detached_head == 1 ||\n>> -\t\t\t (advice_detached_head == -1 && !opts->force_detach)))\n>> +\t\t\t advice_detached_head == 1 && !opts->force_detach)\n>\n> Maybe s/== 1//?\n\nSurely, and thanks.\n"}]}