{"thread":{"id":"49035","subject":"[PATCH 0/1] Support git pull --rebase=i ","startedAt":"2018-08-04T19:23:11Z","lastAt":"2018-08-08T22:19:09Z","messageCount":7,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"354501","messageId":"pull.14.git.gitgitgadget@gmail.com","threadId":"49035","inReplyTo":null,"subject":"[PATCH 0/1] Support git pull --rebase=i ","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-08-04T19:23:08Z","receivedAt":"2018-08-04T19:23:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The patch [https://github.com/git-for-windows/git/commit/4aa8b8c82] that\nintroduced support for pull --rebase= into the Git for Windows project\nstill allowed the very convenient abbreviation\n\ngit pull --rebase=i\n\nwhich was later lost when it was ported to the builtin git pull, and it was\nnot introduced before the patch eventually made it into Git as f5eb87b98dd\n(pull: allow interactive rebase with --rebase=interactive, 2016-01-13).\n\nHowever, it is really a useful short hand for the occasional rebasing pull\non branches that do not usually want to be rebased.\n\nSo let's reintroduce this convenience, at long last.\n\nJohannes Schindelin (1):\n pull --rebase=: allow single-letter abbreviations for the type\n\n builtin/pull.c | 6 +++---\n t/t5520-pull.sh | 12 ++++++++++++\n 2 files changed, 15 insertions(+), 3 deletions(-)\n\n\nbase-commit: 1d89318c48d233d52f1db230cf622935ac3c69fa\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-14%2Fdscho%2Fpull-rebase-i-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-14/dscho/pull-rebase-i-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/14\n-- \ngitgitgadget\n"},{"id":"354502","messageId":"d2ba1e6bef9d6d4eabc8eaa60435dcae1105fd07.1533410586.git.gitgitgadget@gmail.com","threadId":"49035","inReplyTo":"pull.14.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] pull --rebase=: allow single-letter abbreviations for the type","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-08-04T19:23:09Z","receivedAt":"2018-08-04T19:23:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nGit for Windows' original 4aa8b8c8283 (Teach 'git pull' to handle\n--rebase=interactive, 2011-10-21) had support for the very convenient\nabbreviation\n\n\tgit pull --rebase=i\n\nwhich was later lost when it was ported to the builtin `git pull`, and\nit was not introduced before the patch eventually made it into Git as\nf5eb87b98dd (pull: allow interactive rebase with --rebase=interactive,\n2016-01-13).\n\nHowever, it is *really* a useful short hand for the occasional rebasing\npull on branches that do not usually want to be rebased.\n\nSo let's reintroduce this convenience, at long last.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/pull.c | 6 +++---\n t/t5520-pull.sh | 12 ++++++++++++\n 2 files changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 4e7893539..53bc5facf 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -48,11 +48,11 @@ static enum rebase_type parse_config_rebase(const char *key, const char *value,\n \t\treturn REBASE_FALSE;\n \telse if (v > 0)\n \t\treturn REBASE_TRUE;\n-\telse if (!strcmp(value, \"preserve\"))\n+\telse if (!strcmp(value, \"preserve\") || !strcmp(value, \"p\"))\n \t\treturn REBASE_PRESERVE;\n-\telse if (!strcmp(value, \"merges\"))\n+\telse if (!strcmp(value, \"merges\") || !strcmp(value, \"m\"))\n \t\treturn REBASE_MERGES;\n-\telse if (!strcmp(value, \"interactive\"))\n+\telse if (!strcmp(value, \"interactive\") || !strcmp(value, \"i\"))\n \t\treturn REBASE_INTERACTIVE;\n \n \tif (fatal)\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 68aa5f034..5e501c8b0 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -475,10 +475,22 @@ test_expect_success 'pull.rebase=interactive' '\n \tfalse\n \tEOF\n \ttest_set_editor \"$TRASH_DIRECTORY/fake-editor\" &&\n+\ttest_when_finished \"test_might_fail git rebase --abort\" &&\n \ttest_must_fail git pull --rebase=interactive . copy &&\n \ttest \"I was here\" = \"$(cat fake.out)\"\n '\n \n+test_expect_success 'pull --rebase=i' '\n+\twrite_script \"$TRASH_DIRECTORY/fake-editor\" <<-\\EOF &&\n+\techo I was here, too >fake.out &&\n+\tfalse\n+\tEOF\n+\ttest_set_editor \"$TRASH_DIRECTORY/fake-editor\" &&\n+\ttest_when_finished \"test_might_fail git rebase --abort\" &&\n+\ttest_must_fail git pull --rebase=i . copy &&\n+\ttest \"I was here, too\" = \"$(cat fake.out)\"\n+'\n+\n test_expect_success 'pull.rebase=invalid fails' '\n \tgit reset --hard before-preserve-rebase &&\n \ttest_config pull.rebase invalid &&\n-- \ngitgitgadget\n"},{"id":"354679","messageId":"xmqqh8k7nu4l.fsf@gitster-ct.c.googlers.com","threadId":"49035","inReplyTo":"d2ba1e6bef9d6d4eabc8eaa60435dcae1105fd07.1533410586.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] pull --rebase=: allow single-letter abbreviations for the type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-06T19:59:54Z","receivedAt":"2018-08-06T20:00:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> Git for Windows' original 4aa8b8c8283 (Teach 'git pull' to handle\n> --rebase=interactive, 2011-10-21) had support for the very convenient\n> abbreviation\n>\n> \tgit pull --rebase=i\n>\n> which was later lost when it was ported to the builtin `git pull`, and\n> it was not introduced before the patch eventually made it into Git as\n> f5eb87b98dd (pull: allow interactive rebase with --rebase=interactive,\n> 2016-01-13).\n>\n> However, it is *really* a useful short hand for the occasional rebasing\n> pull on branches that do not usually want to be rebased.\n>\n> So let's reintroduce this convenience, at long last.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n\nMakes sense, whether this patch is adding back what in the past\nexisted only in the Windows port but lost, and whether the lossage\nwas long time ago or in just a few releases go, or it is adding a\ncomplete new feature for that matter. This looks like a good\nshort-hand.\n\nWill queue.\n"},{"id":"354727","messageId":"nycvar.QRO.7.76.6.1808071401540.71@tvgsbejvaqbjf.bet","threadId":"49035","inReplyTo":"xmqqh8k7nu4l.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] pull --rebase=: allow single-letter abbreviations for the type","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-07T12:02:03Z","receivedAt":"2018-08-07T12:02:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 6 Aug 2018, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > From: Johannes Schindelin \n> >\n> > Git for Windows' original 4aa8b8c8283 (Teach 'git pull' to handle\n> > --rebase=interactive, 2011-10-21) had support for the very convenient\n> > abbreviation\n> >\n> > \tgit pull --rebase=i\n> >\n> > which was later lost when it was ported to the builtin `git pull`, and\n> > it was not introduced before the patch eventually made it into Git as\n> > f5eb87b98dd (pull: allow interactive rebase with --rebase=interactive,\n> > 2016-01-13).\n> >\n> > However, it is *really* a useful short hand for the occasional rebasing\n> > pull on branches that do not usually want to be rebased.\n> >\n> > So let's reintroduce this convenience, at long last.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> \n> Makes sense, whether this patch is adding back what in the past\n> existed only in the Windows port but lost, and whether the lossage\n> was long time ago or in just a few releases go, or it is adding a\n> complete new feature for that matter. This looks like a good\n> short-hand.\n> \n> Will queue.\n\nThanks!\nDscho\n"},{"id":"354765","messageId":"87sh3qrn6d.fsf@evledraar.gmail.com","threadId":"49035","inReplyTo":"d2ba1e6bef9d6d4eabc8eaa60435dcae1105fd07.1533410586.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] pull --rebase=: allow single-letter abbreviations for the type","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-07T19:28:42Z","receivedAt":"2018-08-07T19:28:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Aug 04 2018, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin \n>\n> Git for Windows' original 4aa8b8c8283 (Teach 'git pull' to handle\n> --rebase=interactive, 2011-10-21) had support for the very convenient\n> abbreviation\n>\n> \tgit pull --rebase=i\n>\n> which was later lost when it was ported to the builtin `git pull`, and\n> it was not introduced before the patch eventually made it into Git as\n> f5eb87b98dd (pull: allow interactive rebase with --rebase=interactive,\n> 2016-01-13).\n>\n> However, it is *really* a useful short hand for the occasional rebasing\n> pull on branches that do not usually want to be rebased.\n>\n> So let's reintroduce this convenience, at long last.\n\nI agree with the shorthand, but I really think this should be\ndocumented. The amount of people who'll discover this by reading the\ncode is much smaller than those who might discover it via the docs.\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> builtin/pull.c | 6 +++---\n> t/t5520-pull.sh | 12 ++++++++++++\n> 2 files changed, 15 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/pull.c b/builtin/pull.c\n> index 4e7893539..53bc5facf 100644\n> --- a/builtin/pull.c\n> +++ b/builtin/pull.c\n> @@ -48,11 +48,11 @@ static enum rebase_type parse_config_rebase(const char *key, const char *value,\n> \t\treturn REBASE_FALSE;\n> \telse if (v > 0)\n> \t\treturn REBASE_TRUE;\n> -\telse if (!strcmp(value, \"preserve\"))\n> +\telse if (!strcmp(value, \"preserve\") || !strcmp(value, \"p\"))\n> \t\treturn REBASE_PRESERVE;\n> -\telse if (!strcmp(value, \"merges\"))\n> +\telse if (!strcmp(value, \"merges\") || !strcmp(value, \"m\"))\n> \t\treturn REBASE_MERGES;\n> -\telse if (!strcmp(value, \"interactive\"))\n> +\telse if (!strcmp(value, \"interactive\") || !strcmp(value, \"i\"))\n> \t\treturn REBASE_INTERACTIVE;\n\nHere 3 special cases are added...\n\n> \tif (fatal)\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 68aa5f034..5e501c8b0 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -475,10 +475,22 @@ test_expect_success 'pull.rebase=interactive' '\n> \tfalse\n> \tEOF\n> \ttest_set_editor \"$TRASH_DIRECTORY/fake-editor\" &&\n> +\ttest_when_finished \"test_might_fail git rebase --abort\" &&\n> \ttest_must_fail git pull --rebase=interactive . copy &&\n> \ttest \"I was here\" = \"$(cat fake.out)\"\n> '\n>\n> +test_expect_success 'pull --rebase=i' '\n> +\twrite_script \"$TRASH_DIRECTORY/fake-editor\" <<-\\EOF &&\n> +\techo I was here, too >fake.out &&\n> +\tfalse\n> +\tEOF\n> +\ttest_set_editor \"$TRASH_DIRECTORY/fake-editor\" &&\n> +\ttest_when_finished \"test_might_fail git rebase --abort\" &&\n> +\ttest_must_fail git pull --rebase=i . copy &&\n> +\ttest \"I was here, too\" = \"$(cat fake.out)\"\n> +'\n> +\n> test_expect_success 'pull.rebase=invalid fails' '\n> \tgit reset --hard before-preserve-rebase &&\n> \ttest_config pull.rebase invalid &&\n\n...but this test is only for 1/3. I haven't run this, but it looks like\nthe tests will still pass if we remove --rebase=p and --rebase=m.\n"},{"id":"354924","messageId":"xmqqmutwfxh0.fsf@gitster-ct.c.googlers.com","threadId":"49035","inReplyTo":"87sh3qrn6d.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/1] pull --rebase=: allow single-letter abbreviations for the type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-08T19:51:39Z","receivedAt":"2018-08-08T19:51:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> -\telse if (!strcmp(value, \"preserve\"))\n>> +\telse if (!strcmp(value, \"preserve\") || !strcmp(value, \"p\"))\n>> \t\treturn REBASE_PRESERVE;\n>> -\telse if (!strcmp(value, \"merges\"))\n>> +\telse if (!strcmp(value, \"merges\") || !strcmp(value, \"m\"))\n>> \t\treturn REBASE_MERGES;\n>> -\telse if (!strcmp(value, \"interactive\"))\n>> +\telse if (!strcmp(value, \"interactive\") || !strcmp(value, \"i\"))\n>> \t\treturn REBASE_INTERACTIVE;\n>\n> Here 3 special cases are added...\n> ...\n>> +test_expect_success 'pull --rebase=i' '\n>> ...\n>> +'\n>> +\n>> test_expect_success 'pull.rebase=invalid fails' '\n>> \tgit reset --hard before-preserve-rebase &&\n>> \ttest_config pull.rebase invalid &&\n>\n> ...but this test is only for 1/3. I haven't run this, but it looks like\n> the tests will still pass if we remove --rebase=p and --rebase=m.\n\nGood eyes. It's not like that parsing these three is implemented\nwith one thing; in other words, it is not hard to break one without\nbreaking the other two.\n\n\n\n\n"},{"id":"354943","messageId":"xmqqk1p0ec2v.fsf@gitster-ct.c.googlers.com","threadId":"49035","inReplyTo":"xmqqmutwfxh0.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] pull --rebase=: allow single-letter abbreviations for the type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-08T22:19:04Z","receivedAt":"2018-08-08T22:19:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> -\telse if (!strcmp(value, \"preserve\"))\n>>> +\telse if (!strcmp(value, \"preserve\") || !strcmp(value, \"p\"))\n>>> \t\treturn REBASE_PRESERVE;\n>>> -\telse if (!strcmp(value, \"merges\"))\n>>> +\telse if (!strcmp(value, \"merges\") || !strcmp(value, \"m\"))\n>>> \t\treturn REBASE_MERGES;\n>>> -\telse if (!strcmp(value, \"interactive\"))\n>>> +\telse if (!strcmp(value, \"interactive\") || !strcmp(value, \"i\"))\n>>> \t\treturn REBASE_INTERACTIVE;\n>>\n>> Here 3 special cases are added...\n>> ...\n>>> +test_expect_success 'pull --rebase=i' '\n>>> ...\n>>> +'\n>>> +\n>>> test_expect_success 'pull.rebase=invalid fails' '\n>>> \tgit reset --hard before-preserve-rebase &&\n>>> \ttest_config pull.rebase invalid &&\n>>\n>> ...but this test is only for 1/3. I haven't run this, but it looks like\n>> the tests will still pass if we remove --rebase=p and --rebase=m.\n>\n> Good eyes. It's not like that parsing these three is implemented\n> with one thing; in other words, it is not hard to break one without\n> breaking the other two.\n\nHaving said that, that can be done as a follow-up \"oops, the\noriginal was sloppy\" patch. It's not as bad compared to \"oops, the\noriginal was totally borked and here is a fix\", so I am OK with that\n;-)\n"}]}