{"thread":{"id":"54078","subject":"git add intent-to-add then git add patch no longer allows edit","startedAt":"2020-08-21T04:27:37Z","lastAt":"2020-09-09T09:40:10Z","messageCount":16,"participants":["Thomas Sullivan","Raymond E. Pasco","Junio C Hamano","Phillip Wood","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"404120","messageId":"CAB+jsAW_+NTJjDLG9uMrf4cc5rcwHLD5kTLk7QgvFCgryb22Gw@mail.gmail.com","threadId":"54078","inReplyTo":null,"subject":"git add intent-to-add then git add patch no longer allows edit","fromName":"Thomas Sullivan","fromEmail":"tom@msbit.com.au","sentAt":"2020-08-21T04:27:23Z","receivedAt":"2020-08-21T04:27:37Z","isPatch":false,"sender":{"key":"tom@msbit.com.au","avatar":null},"body":"What did you do before the bug happened? (Steps to reproduce your issue)\n\n    rm -rf .git\n    cat << EOF > file\n    Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do\neiusmod tempor incididunt ut labore et dolore magna aliqua.\n    EOF\n    git init .\n    git add --intent-to-add file\n    git add --patch file\n\nWhat did you expect to happen? (Expected behavior)\n\nTo be able to edit the single hunk (`e` option)\n\nWhat happened instead? (Actual behavior)\n\nThe `e` option isn't available (and providing it anyway errors with\n`Sorry, cannot edit this hunk`)\n\nAnything else you want to add:\n\nOutput of version 2.28.0:\n\n    diff --git a/file b/file\n    index 0000000..85e73d4\n    --- /dev/null\n    +++ b/file\n    new file mode 100644\n    @@ -0,0 +1 @@\n    +Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do\neiusmod tempor incididunt ut labore et dolore magna aliqua.\n    (1/1) Stage addition [y,n,q,a,d,?]?\n\nOutput of version 2.27.0:\n\n    diff --git a/file b/file\n    index e69de29..85e73d4 100644\n    --- a/file\n    +++ b/file\n    @@ -0,0 +1 @@\n    +Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do\neiusmod tempor incididunt ut labore et dolore magna aliqua.\n    (1/1) Stage this hunk [y,n,q,a,d,e,?]?\n\nBisecting between the two tags reports this commit as the one\nintroducing the change:\n\n    [feea6946a5b746ff4ebf8ccdf959e303203a6011] diff-files: treat\n\"i-t-a\" files as \"not-in-index\"\n\n[System Info]\ngit version:\ngit version 2.28.0\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Darwin 17.7.0 Darwin Kernel Version 17.7.0: Thu Jun 18 21:21:34\nPDT 2020; root:xnu-4570.71.82.5~1/RELEASE_X86_64 x86_64\ncompiler info: clang: 10.0.0 (clang-1000.10.44.4)\nlibc info: no libc information available\n$SHELL (typically, interactive shell): /bin/bash\n\n\n[Enabled Hooks]\n\nRegards,\n\nTom Sullivan\n\nHead Developer\nMost Significant Bit Software\n\ne:  tom@msbit.com.au\np:  +61 407 890 821\n"},{"id":"404122","messageId":"C52FB27V04G0.2ML0PLGDE71MG@ziyou.local","threadId":"54078","inReplyTo":"CAB+jsAW_+NTJjDLG9uMrf4cc5rcwHLD5kTLk7QgvFCgryb22Gw@mail.gmail.com","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2020-08-21T05:25:41Z","receivedAt":"2020-08-21T05:39:53Z","isPatch":false,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"I fixed half of this in a topic that's on master now (it errors out\nentirely if you try to stage it at all in 2.28.0), but new file diffs\nstill aren't splittable into hunks. Phillip Wood (on cc) is looking into\nthat; the tricky part is that when split into hunks only the first hunk\nactually staged can be a \"new file\" patch.\n"},{"id":"404145","messageId":"xmqqmu2o9d6j.fsf@gitster.c.googlers.com","threadId":"54078","inReplyTo":"C52FB27V04G0.2ML0PLGDE71MG@ziyou.local","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-21T16:27:48Z","receivedAt":"2020-08-21T16:28:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Raymond E. Pasco\" <ray@ameretat.dev> writes:\n\n> I fixed half of this in a topic that's on master now (it errors out\n> entirely if you try to stage it at all in 2.28.0), \n\nYup, thanks for that one.\n\n> but new file diffs\n> still aren't splittable into hunks. Phillip Wood (on cc) is looking into\n> that; the tricky part is that when split into hunks only the first hunk\n> actually staged can be a \"new file\" patch.\n\nOut of a change that adds a file with three parts A, B and C (in\nthis order), you could pick the parts A and C, while leaving the\nchange to further add B in the middle, and create a patch to add a\nfile that has A and C, and apply that to the index alone (i.e. \"add\n-p\", pick A and C, and \"add\" that part by applying that \"new file\"\ndiff).  After that, the path is no longer i-t-a but has the real\ncontents (i.e. part A followed by part C), so further \"add -p\" would\nsee the difference between the index and the working tree as a\nmodification patch.\n\nSo as long as you could come up with a good UI to pick parts from a\nsingle hunk \"new file\" diff, \"the second and later application must\nbe done as modification\" should fall out naturally, no?\n\n"},{"id":"404298","messageId":"4607f4b5-a5d6-7965-553f-19a52656fb13@gmail.com","threadId":"54078","inReplyTo":"xmqqmu2o9d6j.fsf@gitster.c.googlers.com","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-08-23T16:03:57Z","receivedAt":"2020-08-23T16:04:20Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 21/08/2020 17:27, Junio C Hamano wrote:\n> \"Raymond E. Pasco\" <ray@ameretat.dev> writes:\n> \n>> I fixed half of this in a topic that's on master now (it errors out\n>> entirely if you try to stage it at all in 2.28.0),\n> \n> Yup, thanks for that one.\n> \n>> but new file diffs\n>> still aren't splittable into hunks. Phillip Wood (on cc) is looking into\n>> that; the tricky part is that when split into hunks only the first hunk\n>> actually staged can be a \"new file\" patch.\n> \n> Out of a change that adds a file with three parts A, B and C (in\n> this order), you could pick the parts A and C, while leaving the\n> change to further add B in the middle, and create a patch to add a\n> file that has A and C, and apply that to the index alone (i.e. \"add\n> -p\", pick A and C, and \"add\" that part by applying that \"new file\"\n> diff).  After that, the path is no longer i-t-a but has the real\n> contents (i.e. part A followed by part C), so further \"add -p\" would\n> see the difference between the index and the working tree as a\n> modification patch.\n> \n> So as long as you could come up with a good UI to pick parts from a\n> single hunk \"new file\" diff, \"the second and later application must\n> be done as modification\" should fall out naturally, no?\n\nI think I was talking about edit rather than split. I'd forgotten that \nit used to work with i-t-a additions. I just checked seen and it seems \nto be working again since dscho's patch although the user is presented \nwith the full diff header rather than just a hunk header in the editor. \nAs you say once the user has staged some part of the diff the rest falls \nout naturally.\n\nBest Wishes\n\nPhillip\n\n(I'm about to go off line for a while)\n"},{"id":"404328","messageId":"C55D662I6OBC.4J2O3KP58DJ2@ziyou.local","threadId":"54078","inReplyTo":"4607f4b5-a5d6-7965-553f-19a52656fb13@gmail.com","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2020-08-24T16:23:15Z","receivedAt":"2020-08-24T16:37:41Z","isPatch":false,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"On Sun Aug 23, 2020 at 12:03 PM EDT, Phillip Wood wrote:\n> I think I was talking about edit rather than split. I'd forgotten that\n> it used to work with i-t-a additions. I just checked seen and it seems\n> to be working again since dscho's patch although the user is presented\n> with the full diff header rather than just a hunk header in the editor.\n> As you say once the user has staged some part of the diff the rest falls\n> out naturally.\n\nWhich topic is this? I can't find one where it works (it's always\n\"Sorry, cannot edit this hunk\" on seen 2.28.0.508.g7d1bebc7fe).\n\nYeah, it's split that would be a problem, edit just stages and moves on.\nSplit would be nice, but I don't actually recall it ever working before\n- it doesn't work now on diffs from actual blank files. Getting edit\nback (if there's a topic that does this already) makes it work for my\nusage.\n"},{"id":"404329","messageId":"fe21c773-341c-d029-077c-00daeb6014f7@gmail.com","threadId":"54078","inReplyTo":"C55D662I6OBC.4J2O3KP58DJ2@ziyou.local","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-08-24T17:28:44Z","receivedAt":"2020-08-24T17:29:22Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Raymond\n\nOn 24/08/2020 17:23, Raymond E. Pasco wrote:\n> On Sun Aug 23, 2020 at 12:03 PM EDT, Phillip Wood wrote:\n>> I think I was talking about edit rather than split. I'd forgotten that\n>> it used to work with i-t-a additions. I just checked seen and it seems\n>> to be working again since dscho's patch although the user is presented\n>> with the full diff header rather than just a hunk header in the editor.\n>> As you say once the user has staged some part of the diff the rest falls\n>> out naturally.\n> \n> Which topic is this? I can't find one where it works (it's always\n> \"Sorry, cannot edit this hunk\" on seen 2.28.0.508.g7d1bebc7fe).\n\nThe patch I was referring to is 2c8bd8471a (\"checkout -p: handle new \nfiles correctly\", 2020-05-27)\n\nI tested seen at 3981657b13 (\"Merge branch 'rp/apply-cached-doc' into \nseen\", 2020-08-21). I was using the C version of 'add -p' which is \nopt-in at the moment by setting add.interactive.usebuiltin=true in your \nconfig (or with git -c). I hope that helps, I'm going off line now for \n10-14 days\n\nBest Wishes\n\nPhillip\n\n> Yeah, it's split that would be a problem, edit just stages and moves on.\n> Split would be nice, but I don't actually recall it ever working before\n> - it doesn't work now on diffs from actual blank files. Getting edit\n> back (if there's a topic that does this already) makes it work for my\n> usage.\n> \n"},{"id":"404379","messageId":"C55J4YTSBL48.171K3FSJLUQOA@ziyou.local","threadId":"54078","inReplyTo":"fe21c773-341c-d029-077c-00daeb6014f7@gmail.com","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2020-08-24T21:03:48Z","receivedAt":"2020-08-24T21:11:38Z","isPatch":false,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"On Mon Aug 24, 2020 at 1:28 PM EDT, Phillip Wood wrote:\n> The patch I was referring to is 2c8bd8471a (\"checkout -p: handle new\n> files correctly\", 2020-05-27)\n>\n> I tested seen at 3981657b13 (\"Merge branch 'rp/apply-cached-doc' into\n> seen\", 2020-08-21). I was using the C version of 'add -p' which is\n> opt-in at the moment by setting add.interactive.usebuiltin=true in your\n> config (or with git -c). I hope that helps, I'm going off line now for\n> 10-14 days\n\nIndeed, this works and restores my workflow (although it errors out if I\ndon't manually edit the range information, which isn't necessary with\ndiffs to existing files). It's a bit unsatisfying as it stands, but\nperhaps there are patches I can write.\n\nNo need to reply, enjoy your vacation!\n"},{"id":"405021","messageId":"1071d841-a030-30c2-e50e-6d97eb494fea@gmail.com","threadId":"54078","inReplyTo":"C55J4YTSBL48.171K3FSJLUQOA@ziyou.local","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-09-04T10:05:49Z","receivedAt":"2020-09-04T10:05:55Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 24/08/2020 22:03, Raymond E. Pasco wrote:\n> On Mon Aug 24, 2020 at 1:28 PM EDT, Phillip Wood wrote:\n>> The patch I was referring to is 2c8bd8471a (\"checkout -p: handle new\n>> files correctly\", 2020-05-27)\n>>\n>> I tested seen at 3981657b13 (\"Merge branch 'rp/apply-cached-doc' into\n>> seen\", 2020-08-21). I was using the C version of 'add -p' which is\n>> opt-in at the moment by setting add.interactive.usebuiltin=true in your\n>> config (or with git -c). I hope that helps, I'm going off line now for\n>> 10-14 days\n> \n> Indeed, this works and restores my workflow (although it errors out if I\n> don't manually edit the range information, which isn't necessary with\n> diffs to existing files). It's a bit unsatisfying as it stands, but\n> perhaps there are patches I can write.\n> \n> No need to reply, enjoy your vacation!\n\nThanks, it was really good to get a change of scene. The patch below\nfixes the hunk editing for new files in the C version of `add -p` if\nanyone wants to try it out. I haven't looked at fixing the perl\nversion yet - dscho what are your plans for switching over to the C\nversion?\n\nBest Wishes\n\nPhillip\n\n---- >8 ----\nFrom b0df1953308f8de5224a2d99d435f93cc4093a17 Mon Sep 17 00:00:00 2001\nFrom: Phillip Wood <phillip.wood@dunelm.org.uk>\nDate: Wed, 2 Sep 2020 15:25:55 +0100\nSubject: [PATCH] add -p: fix editing of intent-to-add paths\n\nA popular way of partially staging a new file is to run `git add -N\n<path>` and then use the hunk editing of `git add -p` to select the\npart of the file that the user wishes to stage. Since\n85953a3187 (\"diff-files --raw: show correct post-image of\nintent-to-add files\", 2020-07-01) this has stopped working as\nintent-to-add paths are now show as new files rather than changes to\nan empty blob and `git apply` refused to apply a creation patch for a\npath that was marked as intent-to-add. 7cfde3fa0f (\"apply: allow \"new\nfile\" patches on i-t-a entries\", 2020-08-06) fixed the problem with\napply but it still wasn't possible to edit the added hunk properly.\n\n2c8bd8471a (\"checkout -p: handle new files correctly\", 2020-05-27)\nhad previously changed `add -p` to handle new files but it did not\nimplement patch editing correctly. The perl version simply forbade\nediting and the C version opened the editor with the full diff rather\nthat just the hunk which meant that the user had to edit the hunk\nheader manually to get it to work.\n\nThis patch only fixes the C version to correctly edit new file\npatches. To test the C version the tests must be run with\nGIT_TEST_ADD_I_USE_BUILTIN=1. It is best viewed with \n--color-moved-ws=allow-indentation-change\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\nReported-by: Thomas Sullivan <tom@msbit.com.au>\n---\n add-patch.c                | 83 +++++++++++++++++++++-----------------\n t/t3701-add-interactive.sh | 44 +++++++++++++++++++-\n 2 files changed, 89 insertions(+), 38 deletions(-)\n\ndiff --git a/add-patch.c b/add-patch.c\nindex f67b304a55..209a63e4f2 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -451,7 +451,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \tpend = p + plain->len;\n \twhile (p != pend) {\n \t\tchar *eol = memchr(p, '\\n', pend - p);\n-\t\tconst char *deleted = NULL, *added = NULL, *mode_change = NULL;\n+\t\tconst char *mode_change = NULL;\n \n \t\tif (!eol)\n \t\t\teol = pend;\n@@ -470,12 +470,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t} else if (p == plain->buf)\n \t\t\tBUG(\"diff starts with unexpected line:\\n\"\n \t\t\t    \"%.*s\\n\", (int)(eol - p), p);\n-\t\telse if (file_diff->deleted || file_diff->added)\n-\t\t\t; /* keep the rest of the file in a single \"hunk\" */\n-\t\telse if (starts_with(p, \"@@ \") ||\n-\t\t\t (hunk == &file_diff->head &&\n-\t\t\t  (skip_prefix(p, \"deleted file\", &deleted) ||\n-\t\t\t   skip_prefix(p, \"new file\", &added)))) {\n+\t\telse if (starts_with(p, \"@@ \")) {\n \t\t\tif (marker == '-' || marker == '+')\n \t\t\t\t/*\n \t\t\t\t * Should not happen; previous hunk did not end\n@@ -493,18 +488,20 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t\tif (colored)\n \t\t\t\thunk->colored_start = colored_p - colored->buf;\n \n-\t\t\tif (deleted)\n-\t\t\t\tfile_diff->deleted = 1;\n-\t\t\telse if (added)\n-\t\t\t\tfile_diff->added = 1;\n-\t\t\telse if (parse_hunk_header(s, hunk) < 0)\n+\t\t\tif (parse_hunk_header(s, hunk) < 0)\n \t\t\t\treturn -1;\n \n \t\t\t/*\n \t\t\t * Start counting into how many hunks this one can be\n \t\t\t * split\n \t\t\t */\n \t\t\tmarker = *p;\n+\t\t} else if (hunk == &file_diff->head &&\n+\t\t\t   starts_with(p, \"new file\")) {\n+\t\t\tfile_diff->added = 1;\n+\t\t} else if (hunk == &file_diff->head &&\n+\t\t\t   starts_with(p, \"deleted file\")) {\n+\t\t\tfile_diff->deleted = 1;\n \t\t} else if (hunk == &file_diff->head &&\n \t\t\t   skip_prefix(p, \"old mode \", &mode_change) &&\n \t\t\t   is_octal(mode_change, eol - mode_change)) {\n@@ -1358,39 +1355,46 @@ static int patch_update_file(struct add_p_state *s,\n \tint colored = !!s->colored.len, quit = 0;\n \tenum prompt_mode_type prompt_mode_type;\n \n-\tif (!file_diff->hunk_nr)\n+\t/* Empty added and deleted files have no hunks */\n+\tif (!file_diff->hunk_nr && !file_diff->added && !file_diff->deleted)\n \t\treturn 0;\n \n \tstrbuf_reset(&s->buf);\n \trender_diff_header(s, file_diff, colored, &s->buf);\n \tfputs(s->buf.buf, stdout);\n \tfor (;;) {\n-\t\tif (hunk_index >= file_diff->hunk_nr)\n-\t\t\thunk_index = 0;\n-\t\thunk = file_diff->hunk + hunk_index;\n+\t\tif (file_diff->hunk_nr) {\n+\t\t\tif (hunk_index >= file_diff->hunk_nr)\n+\t\t\t\thunk_index = 0;\n+\t\t\thunk = file_diff->hunk + hunk_index;\n \n-\t\tundecided_previous = -1;\n-\t\tfor (i = hunk_index - 1; i >= 0; i--)\n-\t\t\tif (file_diff->hunk[i].use == UNDECIDED_HUNK) {\n-\t\t\t\tundecided_previous = i;\n-\t\t\t\tbreak;\n-\t\t\t}\n+\t\t\tundecided_previous = -1;\n+\t\t\tfor (i = hunk_index - 1; i >= 0; i--)\n+\t\t\t\tif (file_diff->hunk[i].use == UNDECIDED_HUNK) {\n+\t\t\t\t\tundecided_previous = i;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n \n-\t\tundecided_next = -1;\n-\t\tfor (i = hunk_index + 1; i < file_diff->hunk_nr; i++)\n-\t\t\tif (file_diff->hunk[i].use == UNDECIDED_HUNK) {\n-\t\t\t\tundecided_next = i;\n-\t\t\t\tbreak;\n-\t\t\t}\n+\t\t\tundecided_next = -1;\n+\t\t\tfor (i = hunk_index + 1; i < file_diff->hunk_nr; i++)\n+\t\t\t\tif (file_diff->hunk[i].use == UNDECIDED_HUNK) {\n+\t\t\t\t\tundecided_next = i;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n \n-\t\t/* Everything decided? */\n-\t\tif (undecided_previous < 0 && undecided_next < 0 &&\n-\t\t    hunk->use != UNDECIDED_HUNK)\n-\t\t\tbreak;\n+\t\t\t/* Everything decided? */\n+\t\t\tif (undecided_previous < 0 && undecided_next < 0 &&\n+\t\t\t    hunk->use != UNDECIDED_HUNK)\n+\t\t\t\tbreak;\n \n-\t\tstrbuf_reset(&s->buf);\n-\t\trender_hunk(s, hunk, 0, colored, &s->buf);\n-\t\tfputs(s->buf.buf, stdout);\n+\t\t\tstrbuf_reset(&s->buf);\n+\t\t\trender_hunk(s, hunk, 0, colored, &s->buf);\n+\t\t\tfputs(s->buf.buf, stdout);\n+\t\t} else {\n+\t\t\thunk = &file_diff->head;\n+\t\t\tundecided_next = -1;\n+\t\t\tundecided_previous = -1;\n+\t\t}\n \n \t\tstrbuf_reset(&s->buf);\n \t\tif (undecided_previous >= 0)\n@@ -1421,7 +1425,9 @@ static int patch_update_file(struct add_p_state *s,\n \t\tcolor_fprintf(stdout, s->s.prompt_color,\n \t\t\t      \"(%\"PRIuMAX\"/%\"PRIuMAX\") \",\n \t\t\t      (uintmax_t)hunk_index + 1,\n-\t\t\t      (uintmax_t)file_diff->hunk_nr);\n+\t\t\t      (uintmax_t)(file_diff->hunk_nr\n+\t\t\t\t\t\t? file_diff->hunk_nr\n+\t\t\t\t\t\t: 1));\n \t\tcolor_fprintf(stdout, s->s.prompt_color,\n \t\t\t      _(s->mode->prompt_mode[prompt_mode_type]),\n \t\t\t      s->buf.buf);\n@@ -1601,14 +1607,17 @@ static int patch_update_file(struct add_p_state *s,\n \t\t\t\t\t\t \"%.*s\", (int)(eol - p), p);\n \t\t\t}\n \t\t}\n+\t\tif (!file_diff->hunk_nr)\n+\t\t\tbreak;\n \t}\n \n \t/* Any hunk to be used? */\n \tfor (i = 0; i < file_diff->hunk_nr; i++)\n \t\tif (file_diff->hunk[i].use == USE_HUNK)\n \t\t\tbreak;\n \n-\tif (i < file_diff->hunk_nr) {\n+\tif (i < file_diff->hunk_nr ||\n+\t    (!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) {\n \t\t/* At least one hunk selected: apply */\n \t\tstrbuf_reset(&s->buf);\n \t\treassemble_patch(s, file_diff, 0, &s->buf);\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex fb73a847cb..49d597979a 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -47,7 +47,11 @@ test_expect_success 'setup (initial)' '\n \techo content >file &&\n \tgit add file &&\n \techo more >>file &&\n-\techo lines >>file\n+\techo lines >>file &&\n+\tif test -n \"$GIT_TEST_ADD_I_USE_BUILTIN\"\n+\tthen\n+\t\ttest_set_prereq BUILTIN_ADD_I\n+\tfi\n '\n test_expect_success 'status works (initial)' '\n \tgit add -i </dev/null >output &&\n@@ -814,6 +818,44 @@ test_expect_success 'checkout -p works with pathological context lines' '\n \ttest_cmp expect a\n '\n \n+# This should be called from a subshell as it sets a temporary editor\n+setup_new_file() {\n+\twrite_script new-file-editor.sh <<-\\EOF &&\n+\tsed /^#/d \"$1\" >patch &&\n+\tsed /^+c/d patch >\"$1\"\n+\tEOF\n+\ttest_set_editor \"$(pwd)/new-file-editor.sh\" &&\n+\ttest_write_lines a b c d e f >new-file &&\n+\ttest_write_lines a b d e f >new-file-expect &&\n+\ttest_write_lines \"@@ -0,0 +1,6 @@\" +a +b +c +d +e +f >patch-expect\n+}\n+\n+test_expect_success BUILTIN_ADD_I 'add -N followed by add -p patch editing' '\n+\tgit reset --hard &&\n+\t(\n+\t\tsetup_new_file &&\n+\t\tgit add -N new-file &&\n+\t\ttest_write_lines e n q | git add -p &&\n+\t\tgit cat-file blob :new-file >actual &&\n+\t\ttest_cmp new-file-expect actual &&\n+\t\ttest_cmp patch-expect patch\n+\t)\n+'\n+\n+test_expect_success BUILTIN_ADD_I 'checkout -p patch editing of added file' '\n+\tgit reset --hard &&\n+\t(\n+\t\tsetup_new_file &&\n+\t\tgit add new-file &&\n+\t\tgit commit -m \"add new file\" &&\n+\t\tgit rm new-file &&\n+\t\tgit commit -m \"remove new file\" &&\n+\t\ttest_write_lines e n q | git checkout -p HEAD^ &&\n+\t\ttest_cmp new-file-expect new-file &&\n+\t\ttest_cmp patch-expect patch\n+\t)\n+'\n+\n test_expect_success 'show help from add--helper' '\n \tgit reset --hard &&\n \tcat >expect <<-EOF &&\n-- \n2.25.1.551.gd3318bf0d3.dirty\n"},{"id":"405045","messageId":"nycvar.QRO.7.76.6.2009040831570.56@tvgsbejvaqbjf.bet","threadId":"54078","inReplyTo":"1071d841-a030-30c2-e50e-6d97eb494fea@gmail.com","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-09-04T06:56:35Z","receivedAt":"2020-09-04T15:21:20Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Fri, 4 Sep 2020, Phillip Wood wrote:\n\n> [...] I haven't looked at fixing the perl version yet - dscho what are\n> your plans for switching over to the C version?\n\nThanks for reminding me, I did not really think about it anymore. The\nbuilt-in `git add -i`/`git add -p` has been available since v2.25.0. Since\nv2.26.0, we also respect that flag in the `-p` modes of `checkout`,\n`stash`, etc\n\nAnd from the way at least _I_ read the commit log, it seems that the code\nhas been pretty stable (except for that bug fix where `e` was allowed by\nmistake).\n\nThe next step will be to invert the default (which is `false` right now)\nfor `add.interactive.useBuiltin`, I guess.\n\nCiao,\nDscho\n"},{"id":"405070","messageId":"C5FNIIPY4D4O.1NCAXB8BEBF4Z@ziyou.local","threadId":"54078","inReplyTo":"1071d841-a030-30c2-e50e-6d97eb494fea@gmail.com","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Raymond E. Pasco","fromEmail":"ray@ameretat.dev","sentAt":"2020-09-05T18:36:14Z","receivedAt":"2020-09-05T18:37:53Z","isPatch":false,"sender":{"key":"ray@ameretat.dev","avatar":"https://avatars.githubusercontent.com/u/115765?v=4"},"body":"On Fri Sep 4, 2020 at 6:05 AM EDT, Phillip Wood wrote:\n> Thanks, it was really good to get a change of scene. The patch below\n> fixes the hunk editing for new files in the C version of `add -p` if\n> anyone wants to try it out. I haven't looked at fixing the perl\n> version yet - dscho what are your plans for switching over to the C\n> version?\n\nNo issues with this patch thus far - I'll continue running with it until\nit gets picked up. Thanks!\n"},{"id":"405091","messageId":"xmqqd02y7rwj.fsf@gitster.c.googlers.com","threadId":"54078","inReplyTo":"nycvar.QRO.7.76.6.2009040831570.56@tvgsbejvaqbjf.bet","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-06T17:10:20Z","receivedAt":"2020-09-06T17:10:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Thanks for reminding me, I did not really think about it anymore. The\n> built-in `git add -i`/`git add -p` has been available since v2.25.0. Since\n> v2.26.0, we also respect that flag in the `-p` modes of `checkout`,\n> `stash`, etc\n\nThat is 9a5315ed (Merge branch 'js/patch-mode-in-others-in-c', 2020-02-05)\n\n> And from the way at least _I_ read the commit log, it seems that the code\n> has been pretty stable (except for that bug fix where `e` was allowed by\n> mistake).\n\nAs long as it has been widely used, that is.  I do not think we\ndeeply mind a bug like the `e` one that does not affect the utility\nor the correctness of the command that much.  If we do not flip the\n\"use the built-in variant\" for those with feature.experimental we\nreally should do so to widen the canarying population immediately.\n"},{"id":"405131","messageId":"a8099cb4-97e0-5596-72a1-10402762015b@gmail.com","threadId":"54078","inReplyTo":"xmqqd02y7rwj.fsf@gitster.c.googlers.com","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-09-07T18:00:13Z","receivedAt":"2020-09-07T18:00:45Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 06/09/2020 18:10, Junio C Hamano wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n>> Thanks for reminding me, I did not really think about it anymore. The\n>> built-in `git add -i`/`git add -p` has been available since v2.25.0. Since\n>> v2.26.0, we also respect that flag in the `-p` modes of `checkout`,\n>> `stash`, etc\n> \n> That is 9a5315ed (Merge branch 'js/patch-mode-in-others-in-c', 2020-02-05)\n> \n>> And from the way at least _I_ read the commit log, it seems that the code\n>> has been pretty stable (except for that bug fix where `e` was allowed by\n>> mistake).\n> \n> As long as it has been widely used, that is. \n\nExactly, I'm not sure it has been that widely used yet. (I'm\ninterested in this area and only got round to using the C version a\ncouple of weeks ago so I wonder how many others have)\n\n> I do not think we\n> deeply mind a bug like the `e` one that does not affect the utility\n> or the correctness of the command that much.\n\nThe bug with editing in this thread is also fairly minor I think. I\nsuspect the main area for serious bugs is hunk splitting and\njoining.\n\n>  If we do not flip the\n> \"use the built-in variant\" for those with feature.experimental we\n> really should do so to widen the canarying population immediately.\n\nThat's a good idea\n\nBest Wishes\n\nPhillip\n\n\n"},{"id":"405155","messageId":"xmqqsgbsz22t.fsf@gitster.c.googlers.com","threadId":"54078","inReplyTo":"a8099cb4-97e0-5596-72a1-10402762015b@gmail.com","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T16:05:14Z","receivedAt":"2020-09-08T16:07:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n>>  If we do not flip the\n>> \"use the built-in variant\" for those with feature.experimental we\n>> really should do so to widen the canarying population immediately.\n>\n> That's a good idea\n\nLike this?  If the more specific one is specifically set, we do not\nlook at experimental bit, but otherwise we use the built-in version.\n\n builtin/add.c | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex b36a99eb7c..26b6ced09e 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -192,9 +192,15 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n \tint use_builtin_add_i =\n \t\tgit_env_bool(\"GIT_TEST_ADD_I_USE_BUILTIN\", -1);\n \n-\tif (use_builtin_add_i < 0)\n-\t\tgit_config_get_bool(\"add.interactive.usebuiltin\",\n-\t\t\t\t    &use_builtin_add_i);\n+\tif (use_builtin_add_i < 0) {\n+\t\tint experimental;\n+\t\tif (!git_config_get_bool(\"add.interactive.usebuiltin\",\n+\t\t\t\t\t &use_builtin_add_i))\n+\t\t\t; /* ok */\n+\t\telse if (!git_config_get_bool(\"feature.experimental\", &experimental) &&\n+\t\t\t experimental)\n+\t\t\tuse_builtin_add_i = 1;\n+\t}\n \n \tif (use_builtin_add_i == 1) {\n \t\tenum add_p_mode mode;\n"},{"id":"405189","messageId":"nycvar.QRO.7.76.6.2009082151520.54@tvgsbejvaqbjf.bet","threadId":"54078","inReplyTo":"xmqqsgbsz22t.fsf@gitster.c.googlers.com","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-09-08T19:52:13Z","receivedAt":"2020-09-08T19:52:38Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 8 Sep 2020, Junio C Hamano wrote:\n\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>\n> >>  If we do not flip the\n> >> \"use the built-in variant\" for those with feature.experimental we\n> >> really should do so to widen the canarying population immediately.\n> >\n> > That's a good idea\n>\n> Like this?  If the more specific one is specifically set, we do not\n> look at experimental bit, but otherwise we use the built-in version.\n\nLooks fine to me,\nDscho\n\n>\n>  builtin/add.c | 12 +++++++++---\n>  1 file changed, 9 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index b36a99eb7c..26b6ced09e 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -192,9 +192,15 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n>  \tint use_builtin_add_i =\n>  \t\tgit_env_bool(\"GIT_TEST_ADD_I_USE_BUILTIN\", -1);\n>\n> -\tif (use_builtin_add_i < 0)\n> -\t\tgit_config_get_bool(\"add.interactive.usebuiltin\",\n> -\t\t\t\t    &use_builtin_add_i);\n> +\tif (use_builtin_add_i < 0) {\n> +\t\tint experimental;\n> +\t\tif (!git_config_get_bool(\"add.interactive.usebuiltin\",\n> +\t\t\t\t\t &use_builtin_add_i))\n> +\t\t\t; /* ok */\n> +\t\telse if (!git_config_get_bool(\"feature.experimental\", &experimental) &&\n> +\t\t\t experimental)\n> +\t\t\tuse_builtin_add_i = 1;\n> +\t}\n>\n>  \tif (use_builtin_add_i == 1) {\n>  \t\tenum add_p_mode mode;\n>\n"},{"id":"405204","messageId":"xmqq363sx71g.fsf@gitster.c.googlers.com","threadId":"54078","inReplyTo":"nycvar.QRO.7.76.6.2009082151520.54@tvgsbejvaqbjf.bet","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T22:00:59Z","receivedAt":"2020-09-08T22:01:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> Like this?  If the more specific one is specifically set, we do not\n>> look at experimental bit, but otherwise we use the built-in version.\n>\n> Looks fine to me,\n> Dscho\n\nThanks, with the proposed log message this time...\n\n-- >8 --\nSubject: [PATCH] add -i: use the built-in version when feature.experimental is set\n\nWe have had parallel implementations of \"add -i/-p\" since 2.25 and\nhave been using them from various codepaths since 2.26 days, but\nnever made the built-in version the default.\n\nWe have found and fixed a handful of corner case bugs in the\nbuilt-in version, and it may be a good time to start switching over\nthe user base from the scripted version to the built-in version.\nLet's enable the built-in version for those who opt into the\nfeature.experimental guinea-pig program to give wider exposure.\n\nAcked-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/add.c | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex b36a99eb7c..26b6ced09e 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -192,9 +192,15 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n \tint use_builtin_add_i =\n \t\tgit_env_bool(\"GIT_TEST_ADD_I_USE_BUILTIN\", -1);\n \n-\tif (use_builtin_add_i < 0)\n-\t\tgit_config_get_bool(\"add.interactive.usebuiltin\",\n-\t\t\t\t    &use_builtin_add_i);\n+\tif (use_builtin_add_i < 0) {\n+\t\tint experimental;\n+\t\tif (!git_config_get_bool(\"add.interactive.usebuiltin\",\n+\t\t\t\t\t &use_builtin_add_i))\n+\t\t\t; /* ok */\n+\t\telse if (!git_config_get_bool(\"feature.experimental\", &experimental) &&\n+\t\t\t experimental)\n+\t\t\tuse_builtin_add_i = 1;\n+\t}\n \n \tif (use_builtin_add_i == 1) {\n \t\tenum add_p_mode mode;\n-- \n2.28.0-539-g66371d8995\n\n"},{"id":"405237","messageId":"b0e42884-0d12-d78d-a34b-d6bc9c69ca5d@gmail.com","threadId":"54078","inReplyTo":"xmqq363sx71g.fsf@gitster.c.googlers.com","subject":"Re: git add intent-to-add then git add patch no longer allows edit","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-09-09T09:40:03Z","receivedAt":"2020-09-09T09:40:10Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 08/09/2020 23:00, Junio C Hamano wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n>>> Like this?  If the more specific one is specifically set, we do not\n>>> look at experimental bit, but otherwise we use the built-in version.\n>>\n>> Looks fine to me,\n>> Dscho\n> \n> Thanks, with the proposed log message this time...\n> \n> -- >8 --\n> Subject: [PATCH] add -i: use the built-in version when feature.experimental is set\n> \n> We have had parallel implementations of \"add -i/-p\" since 2.25 and\n> have been using them from various codepaths since 2.26 days, but\n> never made the built-in version the default.\n> \n> We have found and fixed a handful of corner case bugs in the\n> built-in version, and it may be a good time to start switching over\n> the user base from the scripted version to the built-in version.\n> Let's enable the built-in version for those who opt into the\n> feature.experimental guinea-pig program to give wider exposure.\n> \n> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  builtin/add.c | 12 +++++++++---\n>  1 file changed, 9 insertions(+), 3 deletions(-)\n> \n> diff --git a/builtin/add.c b/builtin/add.c\n> index b36a99eb7c..26b6ced09e 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -192,9 +192,15 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n>  \tint use_builtin_add_i =\n>  \t\tgit_env_bool(\"GIT_TEST_ADD_I_USE_BUILTIN\", -1);\n>  \n> -\tif (use_builtin_add_i < 0)\n> -\t\tgit_config_get_bool(\"add.interactive.usebuiltin\",\n> -\t\t\t\t    &use_builtin_add_i);\n> +\tif (use_builtin_add_i < 0) {\n> +\t\tint experimental;\n> +\t\tif (!git_config_get_bool(\"add.interactive.usebuiltin\",\n> +\t\t\t\t\t &use_builtin_add_i))\n> +\t\t\t; /* ok */\n> +\t\telse if (!git_config_get_bool(\"feature.experimental\", &experimental) &&\n> +\t\t\t experimental)\n> +\t\t\tuse_builtin_add_i = 1;\n> +\t}\n>  \n>  \tif (use_builtin_add_i == 1) {\n>  \t\tenum add_p_mode mode;\n> \n\nLooks good to me as well, Thanks\n\nPhillip\n"}]}