{"thread":{"id":"65350","subject":"[RFC PATCH 0/1] add -p: support discarding hunks","startedAt":"2026-03-25T07:52:36Z","lastAt":"2026-03-25T19:23:34Z","messageCount":10,"participants":["Luiz Campos","D. Ben Knoble","Phillip Wood","Johannes Schindelin","Luiz Eduardo Campos","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"539919","messageId":"20260325075055.354709-1-luizedc1@gmail.com","threadId":"65350","inReplyTo":null,"subject":"[RFC PATCH 0/1] add -p: support discarding hunks","fromName":"Luiz Campos","fromEmail":"luizedc1@gmail.com","sentAt":"2026-03-25T07:50:54Z","receivedAt":"2026-03-25T07:52:36Z","isPatch":true,"sender":{"key":"luizedc1@gmail.com","avatar":null},"body":"Hi,\n\nThis is an RFC for adding a 'discard hunk' action to `git add -p`.\n\nCurrently, when using `git add -p`, users can stage or skip hunks,\nbut cannot discard unwanted changes directly from the working tree.\nThis often leads to repeatedly skipping the same hunks across\nmultiple passes.\n\nThis patch introduces a new 'x' action to discard the current hunk\nby reverse-applying it to the working tree.\n\nThis idea was previously discussed on the mailing list:\nhttps://lore.kernel.org/git/X%2FiFCo0bXLR%2BLZXs@coredump.intra.peff.net/t/#m0576e6f3c6375e11cc4693b9dca3c1fc57baadd0\n\nOpen questions:\n- Should discard happen immediately or be deferred until patch application?\n- Are there edge cases involving overlapping hunks or edited hunks?\n\nFeedback is very welcome.\n\nThanks,\nLuiz\n\nLuiz Campos (1):\n [RFC PATCH 0/1] add -p: support discarding hunks with 'x'\n\n Documentation/git-add.adoc | 7 +-\n add-patch.c | 137 ++++++++++++++++++++++++++++---------\n t/t3701-add-interactive.sh | 58 ++++++++++------\n 3 files changed, 149 insertions(+), 53 deletions(-)\n\n-- \n2.43.0\n\n"},{"id":"539920","messageId":"20260325075055.354709-2-luizedc1@gmail.com","threadId":"65350","inReplyTo":"20260325075055.354709-1-luizedc1@gmail.com","subject":"[RFC PATCH 1/1] add -p: support discarding hunks with 'x'","fromName":"Luiz Campos","fromEmail":"luizedc1@gmail.com","sentAt":"2026-03-25T07:50:55Z","receivedAt":"2026-03-25T07:52:53Z","isPatch":true,"sender":{"key":"luizedc1@gmail.com","avatar":null},"body":"When using `git add -p`, users can stage or skip hunks,\nbut cannot discard unwanted changes from the working tree.\n\nIntroduce a new 'x' action to discard the current hunk by\nreverse-applying it.\n\nThis idea was suggested in a previous mailing list discussion:\nhttps://lore.kernel.org/git/X%2FiFCo0bXLR%2BLZXs@coredump.intra.peff.net/t/#m0576e6f3c6375e11cc4693b9dca3c1fc57baadd0\n\nFeedback is very welcome.\n\nSigned-off-by: Luiz Campos \n---\n Documentation/git-add.adoc | 7 +-\n add-patch.c | 137 ++++++++++++++++++++++++++++---------\n t/t3701-add-interactive.sh | 58 ++++++++++------\n 3 files changed, 149 insertions(+), 53 deletions(-)\n\ndiff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc\nindex 941135dc63..0ab81e5615 100644\n--- a/Documentation/git-add.adoc\n+++ b/Documentation/git-add.adoc\n@@ -351,12 +351,15 @@ patch::\n K - go to the previous hunk, roll over at the top\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n+ x - discard this hunk from the worktree\n p - print the current hunk\n P - print the current hunk using the pager\n ? - print help\n +\n-After deciding the fate for all hunks, if there is any hunk\n-that was chosen, the index is updated with the selected hunks.\n+After deciding the fate for all hunks, any hunks marked for\n+discard are removed from the working tree (reverted to the index\n+version for those lines). Then, if there is any hunk chosen for\n+staging, the index is updated with those hunks.\n +\n You can omit having to type return here, by setting the configuration\n variable `interactive.singleKey` to `true`.\ndiff --git a/add-patch.c b/add-patch.c\nindex 4e28e5c187..ea38ab453e 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -259,7 +259,7 @@ struct hunk_header {\n struct hunk {\n \tsize_t start, end, colored_start, colored_end, splittable_into;\n \tssize_t delta;\n-\tenum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use;\n+\tenum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK, DISCARD_HUNK } use;\n \tstruct hunk_header header;\n };\n \n@@ -884,17 +884,35 @@ static void render_diff_header(struct add_p_state *s,\n \t}\n }\n \n+static bool should_merge_hunk(struct file_diff *file_diff,\n+\t\t\t size_t hunk_index, int use_all,\n+\t\t\t int merge_for_discard)\n+{\n+\tif (use_all)\n+\t\treturn true;\n+\treturn merge_for_discard\n+\t\t? file_diff->hunk[hunk_index].use == DISCARD_HUNK\n+\t\t: file_diff->hunk[hunk_index].use == USE_HUNK;\n+}\n+\n+enum reassemble_mode {\n+\tREASSEMBLE_STAGE,\n+\tREASSEMBLE_DISCARD\n+};\n+\n /* Coalesce hunks again that were split */\n static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,\n-\t\t size_t *hunk_index, int use_all, struct hunk *merged)\n+\t\t size_t *hunk_index, int use_all, struct hunk *merged,\n+\t\t int merge_for_discard)\n {\n \tsize_t i = *hunk_index, delta;\n \tstruct hunk *hunk = file_diff->hunk + i;\n \t/* `header` corresponds to the merged hunk */\n \tstruct hunk_header *header = &merged->header, *next;\n \n-\tif (!use_all && hunk->use != USE_HUNK)\n+\tif (!should_merge_hunk(file_diff, *hunk_index, use_all, merge_for_discard)) {\n \t\treturn 0;\n+\t}\n \n \t*merged = *hunk;\n \t/* We simply skip the colored part (if any) when merging hunks */\n@@ -907,10 +925,10 @@ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,\n \t\t/*\n \t\t * Stop merging hunks when:\n \t\t *\n-\t\t * - the hunk is not selected for use, or\n+\t\t * - the hunk is not selected for use (or discard, when merging discards), or\n \t\t * - the hunk does not overlap with the already-merged hunk(s)\n \t\t */\n-\t\tif ((!use_all && hunk->use != USE_HUNK) ||\n+\t\tif (!should_merge_hunk(file_diff, i + 1, use_all, merge_for_discard) ||\n \t\t header->new_offset >= next->new_offset + merged->delta ||\n \t\t header->new_offset + header->new_count\n \t\t < next->new_offset + merged->delta)\n@@ -1014,11 +1032,13 @@ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,\n \n static void reassemble_patch(struct add_p_state *s,\n \t\t\t struct file_diff *file_diff, int use_all,\n+\t\t\t enum reassemble_mode mode,\n \t\t\t struct strbuf *out)\n {\n \tstruct hunk *hunk;\n \tsize_t save_len = s->plain.len, i;\n \tssize_t delta = 0;\n+\tint merge_for_discard = (mode == REASSEMBLE_DISCARD);\n \n \trender_diff_header(s, file_diff, 0, out);\n \n@@ -1026,25 +1046,26 @@ static void reassemble_patch(struct add_p_state *s,\n \t\tstruct hunk merged = { 0 };\n \n \t\thunk = file_diff->hunk + i;\n-\t\tif (!use_all && hunk->use != USE_HUNK)\n+\t\tif (!should_merge_hunk(file_diff, i, use_all, merge_for_discard)) {\n \t\t\tdelta += hunk->header.old_count\n \t\t\t\t- hunk->header.new_count;\n-\t\telse {\n-\t\t\t/* merge overlapping hunks into a temporary hunk */\n-\t\t\tif (merge_hunks(s, file_diff, &i, use_all, &merged))\n-\t\t\t\thunk = &merged;\n+\t\t\tcontinue;\n+\t\t}\n \n-\t\t\trender_hunk(s, hunk, delta, 0, out);\n+\t\tif (merge_hunks(s, file_diff, &i, use_all, &merged,\n+\t\t\t\tmerge_for_discard))\n+\t\t\thunk = &merged;\n \n-\t\t\t/*\n-\t\t\t * In case `merge_hunks()` used `plain` as a scratch\n-\t\t\t * pad (this happens when an edited hunk had to be\n-\t\t\t * coalesced with another hunk).\n-\t\t\t */\n-\t\t\tstrbuf_setlen(&s->plain, save_len);\n+\t\trender_hunk(s, hunk, delta, 0, out);\n \n-\t\t\tdelta += hunk->delta;\n-\t\t}\n+\t\t/*\n+\t\t * In case `merge_hunks()` used `plain` as a scratch\n+\t\t * pad (this happens when an edited hunk had to be\n+\t\t * coalesced with another hunk).\n+\t\t */\n+\t\tstrbuf_setlen(&s->plain, save_len);\n+\n+\t\tdelta += hunk->delta;\n \t}\n }\n \n@@ -1348,7 +1369,7 @@ static int run_apply_check(struct add_p_state *s,\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \n \tstrbuf_reset(&s->buf);\n-\treassemble_patch(s, file_diff, 1, &s->buf);\n+\treassemble_patch(s, file_diff, 1, REASSEMBLE_STAGE, &s->buf);\n \n \tsetup_child_process(s, &cp,\n \t\t\t \"apply\", \"--check\", NULL);\n@@ -1522,7 +1543,8 @@ static size_t display_hunks(struct add_p_state *s,\n \n \t\tstrbuf_reset(&s->buf);\n \t\tstrbuf_addf(&s->buf, \"%c%2d: \", hunk->use == USE_HUNK ? '+'\n-\t\t\t : hunk->use == SKIP_HUNK ? '-' : ' ',\n+\t\t\t : hunk->use == SKIP_HUNK ? '-'\n+\t\t\t : hunk->use == DISCARD_HUNK ? 'x' : ' ',\n \t\t\t (int)start_index);\n \t\tsummarize_hunk(s, hunk, &s->buf);\n \t\tfputs(s->buf.buf, stdout);\n@@ -1540,6 +1562,7 @@ N_(\"j - go to the next undecided hunk, roll over at the bottom\\n\"\n \"/ - search for a hunk matching the given regex\\n\"\n \"s - split the current hunk into smaller hunks\\n\"\n \"e - manually edit the current hunk\\n\"\n+ \"x - discard this hunk from the worktree\\n\"\n \"p - print the current hunk\\n\"\n \"P - print the current hunk using the pager\\n\"\n \"> - go to the next file, roll over at the bottom\\n\"\n@@ -1547,21 +1570,57 @@ N_(\"j - go to the next undecided hunk, roll over at the bottom\\n\"\n \"? - print help\\n\"\n \"HUNKS SUMMARY - Hunks: %d, USE: %d, SKIP: %d\\n\");\n \n+static int apply_discard_hunks(struct add_p_state *s,\n+\t\t\t struct file_diff *file_diff)\n+{\n+\tstruct child_process check_cp = CHILD_PROCESS_INIT;\n+\tstruct child_process apply_cp = CHILD_PROCESS_INIT;\n+\n+\tstrbuf_reset(&s->buf);\n+\treassemble_patch(s, file_diff, 0, REASSEMBLE_DISCARD, &s->buf);\n+\n+\tdiscard_index(s->index);\n+\n+\tsetup_child_process(s, &check_cp, \"apply\", \"-R\", \"--check\", NULL);\n+\tif (pipe_command(&check_cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) {\n+\t\terror(_(\"'git apply -R --check' failed\"));\n+\t\treturn -1;\n+\t}\n+\n+\tsetup_child_process(s, &apply_cp, \"apply\", \"-R\", NULL);\n+\tif (pipe_command(&apply_cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) {\n+\t\terror(_(\"'git apply -R' failed\"));\n+\t\treturn -1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n static void apply_patch(struct add_p_state *s, struct file_diff *file_diff)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tsize_t j;\n+\tint needs_refresh = 0;\n+\n+\tif (s->mode == &patch_mode_add) {\n+\t\tfor (j = 0; j < file_diff->hunk_nr; j++) {\n+\t\t\tif (file_diff->hunk[j].use == DISCARD_HUNK)\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif (j < file_diff->hunk_nr && apply_discard_hunks(s, file_diff))\n+\t\t\treturn;\n+\t\tif (j < file_diff->hunk_nr)\n+\t\t\tneeds_refresh = 1;\n+\t}\n \n-\t/* Any hunk to be used? */\n \tfor (j = 0; j < file_diff->hunk_nr; j++)\n \t\tif (file_diff->hunk[j].use == USE_HUNK)\n \t\t\tbreak;\n \n \tif (j < file_diff->hunk_nr ||\n-\t\t(!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) {\n-\t\t/* At least one hunk selected: apply */\n+\t (!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) {\n \t\tstrbuf_reset(&s->buf);\n-\t\treassemble_patch(s, file_diff, 0, &s->buf);\n+\t\treassemble_patch(s, file_diff, 0, REASSEMBLE_STAGE, &s->buf);\n \n \t\tdiscard_index(s->index);\n \t\tif (s->mode->apply_for_checkout)\n@@ -1574,13 +1633,15 @@ static void apply_patch(struct add_p_state *s, struct file_diff *file_diff)\n \t\t\t\t\tNULL, 0, NULL, 0))\n \t\t\t\terror(_(\"'git apply' failed\"));\n \t\t}\n-\t\tif (read_index_from(s->index, s->index_file, s->r->gitdir) >= 0 &&\n-\t\t s->index == s->r->index) {\n-\t\t\trepo_refresh_and_write_index(s->r, REFRESH_QUIET, 0,\n-\t\t\t\t\t\t 1, NULL, NULL, NULL);\n-\t\t}\n+\t\tneeds_refresh = 1;\n \t}\n \n+\tif (needs_refresh &&\n+\t read_index_from(s->index, s->index_file, s->r->gitdir) >= 0 &&\n+\t s->index == s->r->index) {\n+\t\trepo_refresh_and_write_index(s->r, REFRESH_QUIET, 0,\n+\t\t\t\t\t 1, NULL, NULL, NULL);\n+\t}\n }\n \n static size_t dec_mod(size_t a, size_t m)\n@@ -1636,7 +1697,8 @@ static size_t patch_update_file(struct add_p_state *s,\n \t\t\tALLOW_SPLIT = 1 << 5,\n \t\t\tALLOW_EDIT = 1 << 6,\n \t\t\tALLOW_GOTO_PREVIOUS_FILE = 1 << 7,\n-\t\t\tALLOW_GOTO_NEXT_FILE = 1 << 8\n+\t\t\tALLOW_GOTO_NEXT_FILE = 1 << 8,\n+\t\t\tALLOW_DISCARD = 1 << 9\n \t\t} permitted = 0;\n \n \t\tif (hunk_index >= file_diff->hunk_nr)\n@@ -1722,6 +1784,10 @@ static size_t patch_update_file(struct add_p_state *s,\n \t\t\t !file_diff->deleted) {\n \t\t\t\tpermitted |= ALLOW_EDIT;\n \t\t\t\tstrbuf_addstr(&s->buf, \",e\");\n+\t\t\t\tif (s->mode == &patch_mode_add) {\n+\t\t\t\t\tpermitted |= ALLOW_DISCARD;\n+\t\t\t\t\tstrbuf_addstr(&s->buf, \",x\");\n+\t\t\t\t}\n \t\t\t}\n \t\t\tif (!s->cfg.auto_advance && s->file_diff_nr > 1) {\n \t\t\t\tpermitted |= ALLOW_GOTO_NEXT_FILE;\n@@ -1750,6 +1816,8 @@ static size_t patch_update_file(struct add_p_state *s,\n \t\tif (hunk->use != UNDECIDED_HUNK) {\n \t\t\tif (hunk->use == USE_HUNK)\n \t\t\t\thunk_use_decision = _(\" (was: y)\");\n+\t\t\telse if (hunk->use == DISCARD_HUNK)\n+\t\t\t\thunk_use_decision = _(\" (was: x)\");\n \t\t\telse\n \t\t\t\thunk_use_decision = _(\" (was: n)\");\n \t\t}\n@@ -1780,6 +1848,13 @@ static size_t patch_update_file(struct add_p_state *s,\n \t\t} else if (ch == 'n') {\n \t\t\thunk->use = SKIP_HUNK;\n \t\t\tgoto soft_increment;\n+\t\t} else if (ch == 'x') {\n+\t\t\tif (!(permitted & ALLOW_DISCARD))\n+\t\t\t\terr(s, _(\"Sorry, cannot discard this hunk\"));\n+\t\t\telse {\n+\t\t\t\thunk->use = DISCARD_HUNK;\n+\t\t\t\tgoto soft_increment;\n+\t\t\t}\n \t\t} else if (ch == 'a') {\n \t\t\tif (file_diff->hunk_nr) {\n \t\t\t\tfor (; hunk_index < file_diff->hunk_nr; hunk_index++) {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 6e120a4001..0eb813f74c 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -48,8 +48,8 @@ test_expect_success 'unknown command' '\n \tgit add -N command &&\n \tgit diff command >expect &&\n \tcat >>expect <<-EOF &&\n-\t(1/1) Stage addition [y,n,q,a,d,e,p,P,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help)\n-\t(1/1) Stage addition [y,n,q,a,d,e,p,P,?]?$SP\n+\t(1/1) Stage addition [y,n,q,a,d,e,x,p,P,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help)\n+\t(1/1) Stage addition [y,n,q,a,d,e,x,p,P,?]?$SP\n \tEOF\n \tgit add -p -- command actual 2>&1 &&\n \ttest_cmp expect actual\n@@ -334,7 +334,7 @@ test_expect_success 'different prompts for mode change/deleted' '\n \tcat >expect <<-\\EOF &&\n \t(1/1) Stage deletion [y,n,q,a,d,p,P,?]?\n \t(1/2) Stage mode change [y,n,q,a,d,k,K,j,J,g,/,p,P,?]?\n-\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]?\n+\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,x,p,P,?]?\n \tEOF\n \ttest_cmp expect actual.filtered\n '\n@@ -447,6 +447,24 @@ test_expect_success 'add first line works' '\n \ttest_cmp expected-output output\n '\n \n+test_expect_success 'add -p discard removes worktree change' '\n+\ttest_when_finished \"rm -rf discard-testrepo\" &&\n+\tmkdir discard-testrepo &&\n+\t(\n+\t\tcd discard-testrepo &&\n+\t\tgit init -b main &&\n+\t\techo clean >discard-me &&\n+\t\tgit add discard-me &&\n+\t\tgit commit -m base &&\n+\t\techo extra >>discard-me &&\n+\t\ttest_write_lines x | git add -p discard-me &&\n+\t\tprintf \"clean\\n\" >expect &&\n+\t\ttest_cmp expect discard-me &&\n+\t\tgit diff --cached >tmp &&\n+\t\ttest_must_be_empty tmp\n+\t)\n+'\n+\n test_expect_success 'setup expected' '\n \tcat >expected <<-\\EOF\n \tdiff --git a/non-empty b/non-empty\n@@ -521,13 +539,13 @@ test_expect_success 'split hunk setup' '\n test_expect_success 'goto hunk 1 with \"g 1\"' '\n \ttest_when_finished \"git reset\" &&\n \ttr _ \" \" >expect <<-EOF &&\n-\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? + 1: -1,2 +1,3 +15\n+\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,x,p,P,?]? + 1: -1,2 +1,3 +15\n \t_ 2: -2,4 +3,8 +21\n \tgo to which hunk? @@ -1,2 +1,3 @@\n \t_10\n \t+15\n \t_20\n-\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n+\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n \tEOF\n \ttest_write_lines s y g 1 | git add -p >actual &&\n \ttail -n 7 actual.trimmed &&\n@@ -540,7 +558,7 @@ test_expect_success 'goto hunk 1 with \"g1\"' '\n \t_10\n \t+15\n \t_20\n-\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n+\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n \tEOF\n \ttest_write_lines s y g1 | git add -p >actual &&\n \ttail -n 4 actual.trimmed &&\n@@ -550,11 +568,11 @@ test_expect_success 'goto hunk 1 with \"g1\"' '\n test_expect_success 'navigate to hunk via regex /pattern' '\n \ttest_when_finished \"git reset\" &&\n \ttr _ \" \" >expect <<-EOF &&\n-\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@\n+\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,x,p,P,?]? @@ -1,2 +1,3 @@\n \t_10\n \t+15\n \t_20\n-\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n+\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n \tEOF\n \ttest_write_lines s y /1,2 | git add -p >actual &&\n \ttail -n 5 actual.trimmed &&\n@@ -567,7 +585,7 @@ test_expect_success 'navigate to hunk via regex / pattern' '\n \t_10\n \t+15\n \t_20\n-\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n+\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n \tEOF\n \ttest_write_lines s y / 1,2 | git add -p >actual &&\n \ttail -n 4 actual.trimmed &&\n@@ -579,11 +597,11 @@ test_expect_success 'print again the hunk' '\n \ttr _ \" \" >expect <<-EOF &&\n \t+15\n \t 20\n-\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@\n+\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? @@ -1,2 +1,3 @@\n \t 10\n \t+15\n \t 20\n-\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n+\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n \tEOF\n \ttest_write_lines s y g 1 p | git add -p >actual &&\n \ttail -n 7 actual.trimmed &&\n@@ -595,11 +613,11 @@ test_expect_success TTY 'print again the hunk (PAGER)' '\n \tcat >expect <<-EOF &&\n \t+15\n \t 20\n-\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? PAGER @@ -1,2 +1,3 @@\n+\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? PAGER @@ -1,2 +1,3 @@\n \tPAGER 10\n \tPAGER +15\n \tPAGER 20\n-\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? \n+\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? \n \tEOF\n \ttest_write_lines s y g 1 P |\n \t(\n@@ -796,21 +814,21 @@ test_expect_success 'colors can be overridden' '\n \t+new\n \t more-context\n \t+another-one\n-\t(1/1) Stage this hunk [y,n,q,a,d,s,e,p,P,?]? Split into 2 hunks.\n+\t(1/1) Stage this hunk [y,n,q,a,d,s,e,x,p,P,?]? Split into 2 hunks.\n \t@@ -1,3 +1,3 @@\n \t context\n \t-old\n \t+new\n \t more-context\n-\t(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? @@ -3 +3,2 @@\n+\t(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? @@ -3 +3,2 @@\n \t more-context\n \t+another-one\n-\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? @@ -1,3 +1,3 @@\n+\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,x,p,P,?]? @@ -1,3 +1,3 @@\n \t context\n \t-old\n \t+new\n \t more-context\n-\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? \n+\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? \n \tEOF\n \ttest_cmp expect actual\n '\n@@ -1424,9 +1442,9 @@ test_expect_success 'invalid option s is rejected' '\n \ttest_write_lines j s q | git add -p >out &&\n \tsed -ne \"s/ @@.*//\" -e \"s/ \\$//\" -e \"/^(/p\" actual &&\n \tcat >expect <<-EOF &&\n-\t(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,p,P,?]?\n-\t(2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? Sorry, cannot split this hunk\n-\t(2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?\n+\t(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,x,p,P,?]?\n+\t(2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? Sorry, cannot split this hunk\n+\t(2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?\n \tEOF\n \ttest_cmp expect actual\n '\n-- \n2.43.0\n\n"},{"id":"539947","messageId":"CALnO6CD15Tcs+Sr7XDO0eB3KSC7RT2oawTiSpUGdrQkfbPJQtg@mail.gmail.com","threadId":"65350","inReplyTo":"20260325075055.354709-2-luizedc1@gmail.com","subject":"Re: [RFC PATCH 1/1] add -p: support discarding hunks with 'x'","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-03-25T15:44:26Z","receivedAt":"2026-03-25T15:44:38Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Wed, Mar 25, 2026 at 3:53 AM Luiz Campos wrote:\n>\n> When using `git add -p`, users can stage or skip hunks,\n> but cannot discard unwanted changes from the working tree.\n>\n> Introduce a new 'x' action to discard the current hunk by\n> reverse-applying it.\n>\n> This idea was suggested in a previous mailing list discussion:\n> https://lore.kernel.org/git/X%2FiFCo0bXLR%2BLZXs@coredump.intra.peff.net/t/#m0576e6f3c6375e11cc4693b9dca3c1fc57baadd0\n>\n> Feedback is very welcome.\n>\n> Signed-off-by: Luiz Campos \n\nOne feature the Fugitive Git client for Vim supports is to discard\nhunks; when it does so, it also prints a message explaining how to\nrecover the hunk if you need it.\n\nI think it writes the file as a blob to Git's database before\nrestoring from the index.\n\nI'm not suggesting Git copy this necessarily, but we might want to\nconsider how to help folks when they lose a hunk they didn't mean to.\nIf it's never been in the index, it can be impossible to recover!\n\nPS How different is this from \"git restore -p\" ?\n\n\n--\nD. Ben Knoble\n"},{"id":"539954","messageId":"2a0ccbfe-3d26-4146-89ed-3b942bdc861e@gmail.com","threadId":"65350","inReplyTo":"20260325075055.354709-2-luizedc1@gmail.com","subject":"Re: [RFC PATCH 1/1] add -p: support discarding hunks with 'x'","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-25T16:24:25Z","receivedAt":"2026-03-25T16:24:37Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Luiz\n\nOn 25/03/2026 07:50, Luiz Campos wrote:\n> When using `git add -p`, users can stage or skip hunks,\n> but cannot discard unwanted changes from the working tree.\n> \n> Introduce a new 'x' action to discard the current hunk by\n> reverse-applying it.\n> \n> This idea was suggested in a previous mailing list discussion:\n> https://lore.kernel.org/git/X%2FiFCo0bXLR%2BLZXs@coredump.intra.peff.net/t/#m0576e6f3c6375e11cc4693b9dca3c1fc57baadd0\n\nI tend to agree with peff's comments in that thread that it is rather \nunexpected for \"git add\" to modify the working copy. I also think that a \ncommand that lets you stage some changes and discard others could be \nuseful as I do both fairly frequently from my editor. Regardless of \nwhether we want a new command the implementation will be similar so I've \nleft some comments on the code below.\n\n> diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc\n> index 941135dc63..0ab81e5615 100644\n> --- a/Documentation/git-add.adoc\n> +++ b/Documentation/git-add.adoc\n> @@ -351,12 +351,15 @@ patch::\n> K - go to the previous hunk, roll over at the top\n> s - split the current hunk into smaller hunks\n> e - manually edit the current hunk\n> + x - discard this hunk from the worktree\n> p - print the current hunk\n> P - print the current hunk using the pager\n> ? - print help\n> +\n> -After deciding the fate for all hunks, if there is any hunk\n> -that was chosen, the index is updated with the selected hunks.\n> +After deciding the fate for all hunks, any hunks marked for\n> +discard are removed from the working tree (reverted to the index\n> +version for those lines). Then, if there is any hunk chosen for\n> +staging, the index is updated with those hunks.\n\nMakes sense.\n\n> +\n> You can omit having to type return here, by setting the configuration\n> variable `interactive.singleKey` to `true`.\n> diff --git a/add-patch.c b/add-patch.c\n> index 4e28e5c187..ea38ab453e 100644\n> --- a/add-patch.c\n> +++ b/add-patch.c\n> @@ -259,7 +259,7 @@ struct hunk_header {\n> struct hunk {\n> \tsize_t start, end, colored_start, colored_end, splittable_into;\n> \tssize_t delta;\n> -\tenum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use;\n> +\tenum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK, DISCARD_HUNK } use;\n> \tstruct hunk_header header;\n> };\n> \n> @@ -884,17 +884,35 @@ static void render_diff_header(struct add_p_state *s,\n> \t}\n> }\n> \n> +static bool should_merge_hunk(struct file_diff *file_diff,\n> +\t\t\t size_t hunk_index, int use_all,\n> +\t\t\t int merge_for_discard)\n> +{\n> +\tif (use_all)\n> +\t\treturn true;\n\nIf we're looking for hunks to discard then we want to return false if \nall the hunks have been selected to be staged.\n\n> +\treturn merge_for_discard\n> +\t\t? file_diff->hunk[hunk_index].use == DISCARD_HUNK\n> +\t\t: file_diff->hunk[hunk_index].use == USE_HUNK;\n\nIt would be simpler just to take USE_HUNK or DISCARD_HUNK as an argument \nrather than a boolean here.\n\n> /* Coalesce hunks again that were split */\n> static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,\n> -\t\t size_t *hunk_index, int use_all, struct hunk *merged)\n> +\t\t size_t *hunk_index, int use_all, struct hunk *merged,\n> +\t\t int merge_for_discard)\n\nTaking the type of hunk we want to retain (USE_HUNK or DISCARD_HUNK) \nwould avoid having to convert merge_for_discard back into the hunk type \nin should_merge_hunk().\n\n> {\n> \tsize_t i = *hunk_index, delta;\n> \tstruct hunk *hunk = file_diff->hunk + i;\n> \t/* `header` corresponds to the merged hunk */\n> \tstruct hunk_header *header = &merged->header, *next;\n> \n> -\tif (!use_all && hunk->use != USE_HUNK)\n> +\tif (!should_merge_hunk(file_diff, *hunk_index, use_all, merge_for_discard)) {\n> \t\treturn 0;\n> +\t}\n\nThere's no need to add braces here\n\n> @@ -1014,11 +1032,13 @@ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,\n> \n> static void reassemble_patch(struct add_p_state *s,\n> \t\t\t struct file_diff *file_diff, int use_all,\n> +\t\t\t enum reassemble_mode mode,\n> \t\t\t struct strbuf *out)\n> {\n> \tstruct hunk *hunk;\n> \tsize_t save_len = s->plain.len, i;\n> \tssize_t delta = 0;\n> +\tint merge_for_discard = (mode == REASSEMBLE_DISCARD);\n\nIt would by simpler just to take USE_HUNK or DISCARD_HUNK as a parameter \nand pass that to reassemble_patch() rather than forcing the caller to \npass an enum that we then transform to a boolean.\n\n> \n> \trender_diff_header(s, file_diff, 0, out);\n> \n> @@ -1026,25 +1046,26 @@ static void reassemble_patch(struct add_p_state *s,\n> \t\tstruct hunk merged = { 0 };\n> \n> \t\thunk = file_diff->hunk + i;\n> -\t\tif (!use_all && hunk->use != USE_HUNK)\n> +\t\tif (!should_merge_hunk(file_diff, i, use_all, merge_for_discard)) {\n> \t\t\tdelta += hunk->header.old_count\n> \t\t\t\t- hunk->header.new_count;\n> -\t\telse {\n> -\t\t\t/* merge overlapping hunks into a temporary hunk */\n> -\t\t\tif (merge_hunks(s, file_diff, &i, use_all, &merged))\n> -\t\t\t\thunk = &merged;\n> +\t\t\tcontinue;\n\nI'm not sure this is an improvement - it certainly makes the patch \nharder to read because you end up changing the indentation of otherwise \nunchanged lines that were in the else clause.\n\n> +\t\t}\n> \n> -\t\t\trender_hunk(s, hunk, delta, 0, out);\n> +\t\tif (merge_hunks(s, file_diff, &i, use_all, &merged,\n> +\t\t\t\tmerge_for_discard))\n> +\t\t\thunk = &merged;\n> \n> -\t\t\t/*\n> -\t\t\t * In case `merge_hunks()` used `plain` as a scratch\n> -\t\t\t * pad (this happens when an edited hunk had to be\n> -\t\t\t * coalesced with another hunk).\n> -\t\t\t */\n> -\t\t\tstrbuf_setlen(&s->plain, save_len);\n> +\t\trender_hunk(s, hunk, delta, 0, out);\n\nWe need to tell render_hunk whether a hunk is being applied in reverse \nor not so that it knows whether to apply delta to the old offset or the \nnew offset. Currently it uses s->mode->reverse but that will not be \ncorrect for the patch that discards changes from the working tree. I'm \nnot sure what the best way of doing that is, the simplest approach is to \ninvert s->mode->reverse when we want to keep hunks marked DISCARD_HUNK \nand then restore the original value.\n\n> -\t\t\tdelta += hunk->delta;\n> -\t\t}\n> +\t\t/*\n> +\t\t * In case `merge_hunks()` used `plain` as a scratch\n> +\t\t * pad (this happens when an edited hunk had to be\n> +\t\t * coalesced with another hunk).\n> +\t\t */\n> +\t\tstrbuf_setlen(&s->plain, save_len);\n> +\n> +\t\tdelta += hunk->delta;\n\nThis is unchanged code that re-indented because of the addition of \n\"continue\" above.\n\n> \t}\n> }\n\n> @@ -1540,6 +1562,7 @@ N_(\"j - go to the next undecided hunk, roll over at the bottom\\n\"\n> \"/ - search for a hunk matching the given regex\\n\"\n> \"s - split the current hunk into smaller hunks\\n\"\n> \"e - manually edit the current hunk\\n\"\n> + \"x - discard this hunk from the worktree\\n\"\n\nIt would be nice to avoid showing this for 'checkout -p' etc.\n\n> \"p - print the current hunk\\n\"\n> \"P - print the current hunk using the pager\\n\"\n> \"> - go to the next file, roll over at the bottom\\n\"\n> @@ -1547,21 +1570,57 @@ N_(\"j - go to the next undecided hunk, roll over at the bottom\\n\"\n> \"? - print help\\n\"\n> \"HUNKS SUMMARY - Hunks: %d, USE: %d, SKIP: %d\\n\");\n> \n> +static int apply_discard_hunks(struct add_p_state *s,\n> +\t\t\t struct file_diff *file_diff)\n> +{\n> +\tstruct child_process check_cp = CHILD_PROCESS_INIT;\n> +\tstruct child_process apply_cp = CHILD_PROCESS_INIT;\n> +\n> +\tstrbuf_reset(&s->buf);\n> +\treassemble_patch(s, file_diff, 0, REASSEMBLE_DISCARD, &s->buf);\n> +\n> +\tdiscard_index(s->index);\n> +\n> +\tsetup_child_process(s, &check_cp, \"apply\", \"-R\", \"--check\", NULL);\n> +\tif (pipe_command(&check_cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) {\n> +\t\terror(_(\"'git apply -R --check' failed\"));\n> +\t\treturn -1;\n> +\t}\n\nWhy do we need to run \"git apply --check\" here?\n\n> +\tsetup_child_process(s, &apply_cp, \"apply\", \"-R\", NULL);\n> +\tif (pipe_command(&apply_cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) {\n> +\t\terror(_(\"'git apply -R' failed\"));\n> +\t\treturn -1;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> static void apply_patch(struct add_p_state *s, struct file_diff *file_diff)\n> {\n> \tstruct child_process cp = CHILD_PROCESS_INIT;\n> \tsize_t j;\n> +\tint needs_refresh = 0;\n> +\n> +\tif (s->mode == &patch_mode_add) {\n> +\t\tfor (j = 0; j < file_diff->hunk_nr; j++) {\n> +\t\t\tif (file_diff->hunk[j].use == DISCARD_HUNK)\n> +\t\t\t\tbreak;\n> +\t\t}\n> +\t\tif (j < file_diff->hunk_nr && apply_discard_hunks(s, file_diff))\n> +\t\t\treturn;\n> +\t\tif (j < file_diff->hunk_nr)\n> +\t\t\tneeds_refresh = 1;\n> +\t}\n> \n> -\t/* Any hunk to be used? */\n\nIsn't this comment still relevant?\n\n> \tfor (j = 0; j < file_diff->hunk_nr; j++)\n> \t\tif (file_diff->hunk[j].use == USE_HUNK)\n> \t\t\tbreak;\n> \n> \tif (j < file_diff->hunk_nr ||\n> -\t\t(!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) {\n> -\t\t/* At least one hunk selected: apply */\n> +\t (!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) {\n\nWhat's the point of this change?\n\n> \t\tstrbuf_reset(&s->buf);\n> -\t\treassemble_patch(s, file_diff, 0, &s->buf);\n> +\t\treassemble_patch(s, file_diff, 0, REASSEMBLE_STAGE, &s->buf);\n> \n> \t\tdiscard_index(s->index);\n> \t\tif (s->mode->apply_for_checkout)\n> @@ -1574,13 +1633,15 @@ static void apply_patch(struct add_p_state *s, struct file_diff *file_diff)\n> \t\t\t\t\tNULL, 0, NULL, 0))\n> \t\t\t\terror(_(\"'git apply' failed\"));\n> \t\t}\n> -\t\tif (read_index_from(s->index, s->index_file, s->r->gitdir) >= 0 &&\n> -\t\t s->index == s->r->index) {\n> -\t\t\trepo_refresh_and_write_index(s->r, REFRESH_QUIET, 0,\n> -\t\t\t\t\t\t 1, NULL, NULL, NULL);\n> -\t\t}\n> +\t\tneeds_refresh = 1;\n> \t}\n> \n> +\tif (needs_refresh &&\n> +\t read_index_from(s->index, s->index_file, s->r->gitdir) >= 0 &&\n> +\t s->index == s->r->index) {\n> +\t\trepo_refresh_and_write_index(s->r, REFRESH_QUIET, 0,\n> +\t\t\t\t\t 1, NULL, NULL, NULL);\n\nWe now wait until we've applied both patches before refreshing the index \n- sounds sensible.\n\n> +\t}\n> }\n\n\n> @@ -1722,6 +1784,10 @@ static size_t patch_update_file(struct add_p_state *s,\n> \t\t\t !file_diff->deleted) {\n> \t\t\t\tpermitted |= ALLOW_EDIT;\n> \t\t\t\tstrbuf_addstr(&s->buf, \",e\");\n> +\t\t\t\tif (s->mode == &patch_mode_add) {\n> +\t\t\t\t\tpermitted |= ALLOW_DISCARD;\n> +\t\t\t\t\tstrbuf_addstr(&s->buf, \",x\");\n> +\t\t\t\t}\n\nSo 'x' is only permitted if 'e' is what's the reason for that?\n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> [...]\n> +test_expect_success 'add -p discard removes worktree change' '\n> +\ttest_when_finished \"rm -rf discard-testrepo\" &&\n> +\tmkdir discard-testrepo &&\n\nIt's nice to see a test for 'x', but I'm not sure why this test needs to \nbe in a separate repository - why can't it use the same repository as \nthe other tests?\n\n> +\t(\n> +\t\tcd discard-testrepo &&\n> +\t\tgit init -b main &&\n> +\t\techo clean >discard-me &&\n> +\t\tgit add discard-me &&\n> +\t\tgit commit -m base &&\n> +\t\techo extra >>discard-me &&\n> +\t\ttest_write_lines x | git add -p discard-me &&\n> +\t\tprintf \"clean\\n\" >expect &&\n> +\t\ttest_cmp expect discard-me &&\n> +\t\tgit diff --cached >tmp &&\n> +\t\ttest_must_be_empty tmp\n\nIt would be nice to see tests that split a hunk like\n\n-a\n+A\n b\n-c\n+C\n d\n-e\n+E\n\nand then (1) stage the first and third sub-hunks and discard the second, \n(2) discard the first and third sub-hunks and stage the second. It would \nalso be nice to see a test that discards a hunk with pathological \ncontext lines - see 2bd69b9024c (add -p: fix checkout -p with \npathological context, 2019-06-12) for an example.\n\nThanks\n\nPhillip\n\n\n> +\t)\n> +'\n> +\n> test_expect_success 'setup expected' '\n> \tcat >expected <<-\\EOF\n> \tdiff --git a/non-empty b/non-empty\n> @@ -521,13 +539,13 @@ test_expect_success 'split hunk setup' '\n> test_expect_success 'goto hunk 1 with \"g 1\"' '\n> \ttest_when_finished \"git reset\" &&\n> \ttr _ \" \" >expect <<-EOF &&\n> -\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? + 1: -1,2 +1,3 +15\n> +\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,x,p,P,?]? + 1: -1,2 +1,3 +15\n> \t_ 2: -2,4 +3,8 +21\n> \tgo to which hunk? @@ -1,2 +1,3 @@\n> \t_10\n> \t+15\n> \t_20\n> -\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n> +\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n> \tEOF\n> \ttest_write_lines s y g 1 | git add -p >actual &&\n> \ttail -n 7 actual.trimmed &&\n> @@ -540,7 +558,7 @@ test_expect_success 'goto hunk 1 with \"g1\"' '\n> \t_10\n> \t+15\n> \t_20\n> -\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n> +\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n> \tEOF\n> \ttest_write_lines s y g1 | git add -p >actual &&\n> \ttail -n 4 actual.trimmed &&\n> @@ -550,11 +568,11 @@ test_expect_success 'goto hunk 1 with \"g1\"' '\n> test_expect_success 'navigate to hunk via regex /pattern' '\n> \ttest_when_finished \"git reset\" &&\n> \ttr _ \" \" >expect <<-EOF &&\n> -\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@\n> +\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,x,p,P,?]? @@ -1,2 +1,3 @@\n> \t_10\n> \t+15\n> \t_20\n> -\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n> +\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n> \tEOF\n> \ttest_write_lines s y /1,2 | git add -p >actual &&\n> \ttail -n 5 actual.trimmed &&\n> @@ -567,7 +585,7 @@ test_expect_success 'navigate to hunk via regex / pattern' '\n> \t_10\n> \t+15\n> \t_20\n> -\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n> +\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n> \tEOF\n> \ttest_write_lines s y / 1,2 | git add -p >actual &&\n> \ttail -n 4 actual.trimmed &&\n> @@ -579,11 +597,11 @@ test_expect_success 'print again the hunk' '\n> \ttr _ \" \" >expect <<-EOF &&\n> \t+15\n> \t 20\n> -\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@\n> +\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? @@ -1,2 +1,3 @@\n> \t 10\n> \t+15\n> \t 20\n> -\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n> +\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n> \tEOF\n> \ttest_write_lines s y g 1 p | git add -p >actual &&\n> \ttail -n 7 actual.trimmed &&\n> @@ -595,11 +613,11 @@ test_expect_success TTY 'print again the hunk (PAGER)' '\n> \tcat >expect <<-EOF &&\n> \t+15\n> \t 20\n> -\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? PAGER @@ -1,2 +1,3 @@\n> +\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? PAGER @@ -1,2 +1,3 @@\n> \tPAGER 10\n> \tPAGER +15\n> \tPAGER 20\n> -\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? \n> +\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? \n> \tEOF\n> \ttest_write_lines s y g 1 P |\n> \t(\n> @@ -796,21 +814,21 @@ test_expect_success 'colors can be overridden' '\n> \t+new\n> \t more-context\n> \t+another-one\n> -\t(1/1) Stage this hunk [y,n,q,a,d,s,e,p,P,?]? Split into 2 hunks.\n> +\t(1/1) Stage this hunk [y,n,q,a,d,s,e,x,p,P,?]? Split into 2 hunks.\n> \t@@ -1,3 +1,3 @@\n> \t context\n> \t-old\n> \t+new\n> \t more-context\n> -\t(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? @@ -3 +3,2 @@\n> +\t(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? @@ -3 +3,2 @@\n> \t more-context\n> \t+another-one\n> -\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? @@ -1,3 +1,3 @@\n> +\t(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,x,p,P,?]? @@ -1,3 +1,3 @@\n> \t context\n> \t-old\n> \t+new\n> \t more-context\n> -\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? \n> +\t(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? \n> \tEOF\n> \ttest_cmp expect actual\n> '\n> @@ -1424,9 +1442,9 @@ test_expect_success 'invalid option s is rejected' '\n> \ttest_write_lines j s q | git add -p >out &&\n> \tsed -ne \"s/ @@.*//\" -e \"s/ \\$//\" -e \"/^(/p\" actual &&\n> \tcat >expect <<-EOF &&\n> -\t(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,p,P,?]?\n> -\t(2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? Sorry, cannot split this hunk\n> -\t(2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?\n> +\t(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,x,p,P,?]?\n> +\t(2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? Sorry, cannot split this hunk\n> +\t(2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?\n> \tEOF\n> \ttest_cmp expect actual\n> '\n\n"},{"id":"539963","messageId":"a4305ef7-50ff-4a68-ab42-fe2fa73e8f37@gmx.de","threadId":"65350","inReplyTo":"20260325075055.354709-2-luizedc1@gmail.com","subject":"Re: [RFC PATCH 1/1] add -p: support discarding hunks with 'x'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-03-25T16:49:40Z","receivedAt":"2026-03-25T16:49:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Luiz,\n\nOn Wed, 25 Mar 2026, Luiz Campos wrote:\n\n> When using `git add -p`, users can stage or skip hunks,\n> but cannot discard unwanted changes from the working tree.\n> \n> Introduce a new 'x' action to discard the current hunk by\n> reverse-applying it.\n> \n> This idea was suggested in a previous mailing list discussion:\n> https://lore.kernel.org/git/X%2FiFCo0bXLR%2BLZXs@coredump.intra.peff.net/t/#m0576e6f3c6375e11cc4693b9dca3c1fc57baadd0\n\nSounds good!\n\nJust two minor comments (not really actionable, I think):\n\n> @@ -1026,25 +1046,26 @@ static void reassemble_patch(struct add_p_state *s,\n> \t\tstruct hunk merged = { 0 };\n> \n> \t\thunk = file_diff->hunk + i;\n> -\t\tif (!use_all && hunk->use != USE_HUNK)\n> +\t\tif (!should_merge_hunk(file_diff, i, use_all, merge_for_discard)) {\n> \t\t\tdelta += hunk->header.old_count\n> \t\t\t\t- hunk->header.new_count;\n> -\t\telse {\n> -\t\t\t/* merge overlapping hunks into a temporary hunk */\n> -\t\t\tif (merge_hunks(s, file_diff, &i, use_all, &merged))\n> -\t\t\t\thunk = &merged;\n> +\t\t\tcontinue;\n> +\t\t}\n> \n> -\t\t\trender_hunk(s, hunk, delta, 0, out);\n> +\t\tif (merge_hunks(s, file_diff, &i, use_all, &merged,\n> +\t\t\t\tmerge_for_discard))\n> +\t\t\thunk = &merged;\n> \n> -\t\t\t/*\n> -\t\t\t * In case `merge_hunks()` used `plain` as a scratch\n> -\t\t\t * pad (this happens when an edited hunk had to be\n> -\t\t\t * coalesced with another hunk).\n> -\t\t\t */\n> -\t\t\tstrbuf_setlen(&s->plain, save_len);\n> +\t\trender_hunk(s, hunk, delta, 0, out);\n> \n> -\t\t\tdelta += hunk->delta;\n> -\t\t}\n> +\t\t/*\n> +\t\t * In case `merge_hunks()` used `plain` as a scratch\n> +\t\t * pad (this happens when an edited hunk had to be\n> +\t\t * coalesced with another hunk).\n> +\t\t */\n> +\t\tstrbuf_setlen(&s->plain, save_len);\n> +\n> +\t\tdelta += hunk->delta;\n\nThis hunk is quite hard to read because of the `if ... else ...` -> `if {\n... continue; } ...` change that de-indents a large chunk of code.\n\nAfter pouring over the diff for a bit, I was able to convince myself that\nthe diff is correct.\n\n> @@ -1547,21 +1570,57 @@ N_(\"j - go to the next undecided hunk, roll over at the bottom\\n\"\n> \"? - print help\\n\"\n> \"HUNKS SUMMARY - Hunks: %d, USE: %d, SKIP: %d\\n\");\n> \n> +static int apply_discard_hunks(struct add_p_state *s,\n> +\t\t\t struct file_diff *file_diff)\n> +{\n> +\tstruct child_process check_cp = CHILD_PROCESS_INIT;\n> +\tstruct child_process apply_cp = CHILD_PROCESS_INIT;\n> +\n> +\tstrbuf_reset(&s->buf);\n> +\treassemble_patch(s, file_diff, 0, REASSEMBLE_DISCARD, &s->buf);\n\nIf you detect an empty patch here and indicate this via an early return\nvalue, then...\n\n> +\n> +\tdiscard_index(s->index);\n> +\n> +\tsetup_child_process(s, &check_cp, \"apply\", \"-R\", \"--check\", NULL);\n> +\tif (pipe_command(&check_cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) {\n> +\t\terror(_(\"'git apply -R --check' failed\"));\n> +\t\treturn -1;\n> +\t}\n> +\n> +\tsetup_child_process(s, &apply_cp, \"apply\", \"-R\", NULL);\n> +\tif (pipe_command(&apply_cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) {\n> +\t\terror(_(\"'git apply -R' failed\"));\n> +\t\treturn -1;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> static void apply_patch(struct add_p_state *s, struct file_diff *file_diff)\n> {\n> \tstruct child_process cp = CHILD_PROCESS_INIT;\n> \tsize_t j;\n> +\tint needs_refresh = 0;\n> +\n> +\tif (s->mode == &patch_mode_add) {\n> +\t\tfor (j = 0; j < file_diff->hunk_nr; j++) {\n> +\t\t\tif (file_diff->hunk[j].use == DISCARD_HUNK)\n> +\t\t\t\tbreak;\n> +\t\t}\n> +\t\tif (j < file_diff->hunk_nr && apply_discard_hunks(s, file_diff))\n> +\t\t\treturn;\n> +\t\tif (j < file_diff->hunk_nr)\n> +\t\t\tneeds_refresh = 1;\n> +\t}\n\n... then this loop is no longer necessary.\n\nOther than that, looks good to me!\n\nCiao,\nJohannes\n"},{"id":"539965","messageId":"CAN+A6TuhiC9U==QvuMXtnTHtwzSMh1fSe12kxqj3iy03jF-Cxg@mail.gmail.com","threadId":"65350","inReplyTo":"CALnO6CD15Tcs+Sr7XDO0eB3KSC7RT2oawTiSpUGdrQkfbPJQtg@mail.gmail.com","subject":"Re: [RFC PATCH 1/1] add -p: support discarding hunks with 'x'","fromName":"Luiz Eduardo Campos","fromEmail":"luizedc1@gmail.com","sentAt":"2026-03-25T17:04:03Z","receivedAt":"2026-03-25T17:05:25Z","isPatch":true,"sender":{"key":"luizedc1@gmail.com","avatar":null},"body":"Hi Ben,\n\nThanks for the feedback!\n\n(Sending this message again because I had not configured my\nemail client to send it plain text, so I apologize if you received\nduplicated content)\n\n> One feature the Fugitive Git client for Vim supports is to discard\n> hunks; when it does so, it also prints a message explaining how to\n> recover the hunk if you need it.\n\nThat’s a great point. I hadn’t considered recoverability in this\ninitial version. Storing the discarded hunk as a blob (or otherwise\nmaking it recoverable) seems like a useful safety measure.\n\n> If it's never been in the index, it can be impossible to recover!\n\nAgreed — this is probably something that should be addressed before\nconsidering this feature complete.\n\n> PS How different is this from \"git restore -p\" ?\n\nMy understanding is that `git restore -p` already allows discarding\nchanges interactively, but it requires a separate pass. The goal here\nwas to allow discarding during `git add -p`, so users can decide what\nto do with each hunk in a single pass.\n\nThat said, I’m not yet sure whether integrating this into `add -p` is\nthe best approach, or if this should be handled differently.\n\nThanks again for the insights!\n\nLuiz\n\n\nEm qua., 25 de mar. de 2026 às 12:44, D. Ben Knoble\n escreveu:\n>\n> On Wed, Mar 25, 2026 at 3:53 AM Luiz Campos wrote:\n> >\n> > When using `git add -p`, users can stage or skip hunks,\n> > but cannot discard unwanted changes from the working tree.\n> >\n> > Introduce a new 'x' action to discard the current hunk by\n> > reverse-applying it.\n> >\n> > This idea was suggested in a previous mailing list discussion:\n> > https://lore.kernel.org/git/X%2FiFCo0bXLR%2BLZXs@coredump.intra.peff.net/t/#m0576e6f3c6375e11cc4693b9dca3c1fc57baadd0\n> >\n> > Feedback is very welcome.\n> >\n> > Signed-off-by: Luiz Campos \n>\n> One feature the Fugitive Git client for Vim supports is to discard\n> hunks; when it does so, it also prints a message explaining how to\n> recover the hunk if you need it.\n>\n> I think it writes the file as a blob to Git's database before\n> restoring from the index.\n>\n> I'm not suggesting Git copy this necessarily, but we might want to\n> consider how to help folks when they lose a hunk they didn't mean to.\n> If it's never been in the index, it can be impossible to recover!\n>\n> PS How different is this from \"git restore -p\" ?\n>\n>\n> --\n> D. Ben Knoble\n"},{"id":"539976","messageId":"xmqqcy0rvlao.fsf@gitster.g","threadId":"65350","inReplyTo":"20260325075055.354709-1-luizedc1@gmail.com","subject":"Re: [RFC PATCH 0/1] add -p: support discarding hunks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-25T18:03:27Z","receivedAt":"2026-03-25T18:03:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luiz Campos writes:\n\n> Hi,\n>\n> This is an RFC for adding a 'discard hunk' action to `git add -p`.\n>\n> Currently, when using `git add -p`, users can stage or skip hunks,\n> but cannot discard unwanted changes directly from the working tree.\n> This often leads to repeatedly skipping the same hunks across\n> multiple passes.\n>\n> This patch introduces a new 'x' action to discard the current hunk\n> by reverse-applying it to the working tree.\n>\n> This idea was previously discussed on the mailing list:\n> https://lore.kernel.org/git/X%2FiFCo0bXLR%2BLZXs@coredump.intra.peff.net/t/#m0576e6f3c6375e11cc4693b9dca3c1fc57baadd0\n>\n> Open questions:\n> - Should discard happen immediately or be deferred until patch application?\n> - Are there edge cases involving overlapping hunks or edited hunks?\n\nAfter reading the discussion (by the way, I do not recall seeing it,\nso thank you very much for having a link to it), I agree with what\nPeff said back then. \"add -p\" that touches the working tree feels\nquite weird.\n\nIn addition to that, letting it make destructive change makes the\nidea even less appetizing. Once you remove the changes introduced\nby the hunk, it is forever gone. A \"discard\" in \"add -p\" would not\nsolve your problem without adding many unhappy users who lost their\nwork by mistake. I do not want to see people trigger \"discard\" by\nmistake in \"add -p\" session _and_ find that there is no way to undo\nthat mistaken discard.\n\n\"stash -p\" followed by \"add -p\" is probably the best we can do that\nis safe. When the unwanted change is truly unwanted garbage that\nyou would never ever want to see again, \"restore -p\" followed by\n\"add -p\" would be an alternative.\n\nOne reason why they are not satisfying is because during the later\n\"add -p\" session, we will notice that some unwanted things we failed\nto notice and get rid of (either by sending them to stash or restoring\nit away) are still there, reminding us that we are imperfect human,\nand at that point, it is not easy to switch back to the \"stash -p\"\nor \"restore -p\" from there.\n\nWhat you want is probably a _single_ command that lets you inspect\nthe differences among the HEAD, the index, and the working tree, and\nallows you to move things hunk-by-hunk in different directions.\n\n * You can go through the \"git diff --cached\" (i.e., changes already in\n the index), and selectively undo/revert the changes to the index,\n similar to \"git reset -p\".\n\n * You can go through the \"git diff\" (i.e., changes between the\n index and the working tree), and selectively apply the changes to\n the index, similar to \"git add -p\".\n\n * You can go through the \"git diff HEAD\" (i.e. changes since your\n last commit), and selectively send the changes to a stash entry,\n similar to \"git stash -p\". This is not destructive.\n\nAnd if the single command lets you switch among working with these\nmodes, you no longer need to worry about forgetting to send\nsome changes to stash to concentrate on working on the rest.\n\nIn addition, optionally you can also have this in the same command:\n\n * You can go through the \"git diff\", and selectively revert the\n changes to the working tree, similar to \"git restore -p\".\n\nThis additional mode *is* destructive, but if you know from the hunk\nthat you will never need the change in it, it would be a right tool\nfor it.\n"},{"id":"539978","messageId":"CAN+A6TtxG_-WuzvAxwoRB_dz4swFbjFG8me09WsShboQWKwang@mail.gmail.com","threadId":"65350","inReplyTo":"2a0ccbfe-3d26-4146-89ed-3b942bdc861e@gmail.com","subject":"Re: [RFC PATCH 1/1] add -p: support discarding hunks with 'x'","fromName":"Luiz Eduardo Campos","fromEmail":"luizedc1@gmail.com","sentAt":"2026-03-25T18:38:50Z","receivedAt":"2026-03-25T18:40:12Z","isPatch":true,"sender":{"key":"luizedc1@gmail.com","avatar":null},"body":"Hi Phillip,\n\nThanks for the thorough review.\n\n> it is rather unexpected for \"git add\" to modify the working copy\n\nI share that concern. My motivation was allowing stage-and-discard in\na single pass, which I find myself wanting regularly, and discussion in\nthe linked thread suggested it could live in add -p. That said, I can\nsee the argument that git add modifying the worktree breaks the mental\nmodel. I'm open to discussing whether a separate command or a flag on\ngit restore -p would be a better home for this — the implementation\nwould be largely the same either way.\n\n> It would be simpler just to take USE_HUNK or DISCARD_HUNK as an argument\n\nAgreed. I'll drop both the merge_for_discard boolean and the enum\nreassemble_mode, and instead pass the desired hunk type directly\n(USE_HUNK or DISCARD_HUNK) through reassemble_patch,\nmerge_hunks, and should_merge_hunk. That removes the indirection\nentirely.\n\n> There's no need to add braces here\n\nRight, the project style omits braces for single-statement blocks.\nI'll drop them.\n\n> I'm not sure this is an improvement — it certainly makes the patch harder to\n> read because you end up changing the indentation of otherwise unchanged\n> lines that were in the else clause.\n\n> This is unchanged code that re-indented because of the addition of\n> \"continue\" above.\n\nGood point. With the simplified should_merge_hunk() taking a hunk\ntype, I can keep the original if / else structure in\nreassemble_patch() and just swap out the condition. That avoids the\ncontinue rewrite and the indentation churn entirely.\n\n> We need to tell render_hunk whether a hunk is being applied in reverse or\n> not so that it knows whether to apply delta to the old offset or the new offset.\n\nYou're right. I was wrong to think the delta direction would be the same as for\nstaging. For the discard patch, skipped hunks' changes are still present in\nthe worktree, so the new_offset (worktree side) is already correct and must not\nbe adjusted. The adjustment needs to go to old_offset instead — exactly the\nis_reverse = 1 behaviour. This is consistent with patch_mode_checkout_index,\nwhich does the same operation (diff-files + apply -R to worktree) and\nsets is_reverse = 1.\n\nI'll temporarily flip is_reverse (or pass an explicit flag to render_hunk) when\nassembling the discard patch, and add the split-hunk tests below to confirm the\noffsets come out correct.\n\n> It would be nice to avoid showing this for 'checkout -p' etc.\n\nThe help text display already filters lines from\nhelp_patch_remainder against the options present in s->buf\n(the prompt string). Since 'x' is only added to the prompt in\npatch_mode_add, the \"x - discard...\" line will already be suppressed\nfor checkout -p, reset -p, etc.\n\nI'll add a comment in the next version to make that clearer, and add a\ntest that verifies checkout -p help output does not mention 'x'.\n\n> Why do we need to run \"git apply --check\" here?\n\nWe don't — the existing staging codepath in apply_patch does not run a\nseparate --check before applying either; it just runs git apply\ndirectly and checks the exit code. I'll do the same and remove the\nredundant --check pass.\n\nI'll remove the redundant check and just run git apply -R directly.\n\n> Isn't this comment still relevant?\n\nYes, I'll restore it. I removed it by accident when restructuring the\nfunction; it still applies to the USE_HUNK scan that follows.\n\n> What's the point of this change?\n\nThere is none — it's an accidental whitespace change on the alignment\nof the (!file_diff->hunk_nr continuation line. I'll drop it.\n\n> So 'x' is only permitted if 'e' is — what's the reason for that?\n\nThe nesting was unintentional, but the conditions guarding 'e' —\nhunk_index + 1 > file_diff->mode_change and\n!file_diff->deleted — do also apply to 'x', since reverse-applying\na mode-change or deletion hunk is not meaningful.\n\nHowever, the ADD_P_DISALLOW_EDIT flag (used by git history split)\nshould not gate 'x'. I'll give 'x' its own block that checks only\nmode_change and deleted, independent of ALLOW_EDIT.\n\n> I'm not sure why this test needs to be in a separate repository\n\nThe sub-repo was a workaround: the test creates a commit\n(git commit -m base), and that extra commit shifted the history and\ncaused later tests that depend on specific HEAD state to fail.\n\nInstead, I'll rework the test to use a file already tracked in the\nshared repo, dirty it, run git add -p with 'x', and clean up with\ntest_when_finished. No extra commit needed.\n\n> It would be nice to see tests that split a hunk [...] and pathological context lines\n\nAgreed. I'll add tests that:\n\n(1) split a three-subhunk change (-a/+A, -c/+C, -e/+E), stage the\nfirst and third, and discard the second\n(2) the inverse — discard the first and third, stage the second\n(3) a pathological-context case along the lines of 2bd69b9024c\n\nThanks again for the review — very helpful.\n\nLuiz\n\nEm qua., 25 de mar. de 2026 às 13:24, Phillip Wood\n escreveu:\n>\n> Hi Luiz\n>\n> On 25/03/2026 07:50, Luiz Campos wrote:\n> > When using `git add -p`, users can stage or skip hunks,\n> > but cannot discard unwanted changes from the working tree.\n> >\n> > Introduce a new 'x' action to discard the current hunk by\n> > reverse-applying it.\n> >\n> > This idea was suggested in a previous mailing list discussion:\n> > https://lore.kernel.org/git/X%2FiFCo0bXLR%2BLZXs@coredump.intra.peff.net/t/#m0576e6f3c6375e11cc4693b9dca3c1fc57baadd0\n>\n> I tend to agree with peff's comments in that thread that it is rather\n> unexpected for \"git add\" to modify the working copy. I also think that a\n> command that lets you stage some changes and discard others could be\n> useful as I do both fairly frequently from my editor. Regardless of\n> whether we want a new command the implementation will be similar so I've\n> left some comments on the code below.\n>\n> > diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc\n> > index 941135dc63..0ab81e5615 100644\n> > --- a/Documentation/git-add.adoc\n> > +++ b/Documentation/git-add.adoc\n> > @@ -351,12 +351,15 @@ patch::\n> > K - go to the previous hunk, roll over at the top\n> > s - split the current hunk into smaller hunks\n> > e - manually edit the current hunk\n> > + x - discard this hunk from the worktree\n> > p - print the current hunk\n> > P - print the current hunk using the pager\n> > ? - print help\n> > +\n> > -After deciding the fate for all hunks, if there is any hunk\n> > -that was chosen, the index is updated with the selected hunks.\n> > +After deciding the fate for all hunks, any hunks marked for\n> > +discard are removed from the working tree (reverted to the index\n> > +version for those lines). Then, if there is any hunk chosen for\n> > +staging, the index is updated with those hunks.\n>\n> Makes sense.\n>\n> > +\n> > You can omit having to type return here, by setting the configuration\n> > variable `interactive.singleKey` to `true`.\n> > diff --git a/add-patch.c b/add-patch.c\n> > index 4e28e5c187..ea38ab453e 100644\n> > --- a/add-patch.c\n> > +++ b/add-patch.c\n> > @@ -259,7 +259,7 @@ struct hunk_header {\n> > struct hunk {\n> > size_t start, end, colored_start, colored_end, splittable_into;\n> > ssize_t delta;\n> > - enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use;\n> > + enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK, DISCARD_HUNK } use;\n> > struct hunk_header header;\n> > };\n> >\n> > @@ -884,17 +884,35 @@ static void render_diff_header(struct add_p_state *s,\n> > }\n> > }\n> >\n> > +static bool should_merge_hunk(struct file_diff *file_diff,\n> > + size_t hunk_index, int use_all,\n> > + int merge_for_discard)\n> > +{\n> > + if (use_all)\n> > + return true;\n>\n> If we're looking for hunks to discard then we want to return false if\n> all the hunks have been selected to be staged.\n>\n> > + return merge_for_discard\n> > + ? file_diff->hunk[hunk_index].use == DISCARD_HUNK\n> > + : file_diff->hunk[hunk_index].use == USE_HUNK;\n>\n> It would be simpler just to take USE_HUNK or DISCARD_HUNK as an argument\n> rather than a boolean here.\n>\n> > /* Coalesce hunks again that were split */\n> > static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,\n> > - size_t *hunk_index, int use_all, struct hunk *merged)\n> > + size_t *hunk_index, int use_all, struct hunk *merged,\n> > + int merge_for_discard)\n>\n> Taking the type of hunk we want to retain (USE_HUNK or DISCARD_HUNK)\n> would avoid having to convert merge_for_discard back into the hunk type\n> in should_merge_hunk().\n>\n> > {\n> > size_t i = *hunk_index, delta;\n> > struct hunk *hunk = file_diff->hunk + i;\n> > /* `header` corresponds to the merged hunk */\n> > struct hunk_header *header = &merged->header, *next;\n> >\n> > - if (!use_all && hunk->use != USE_HUNK)\n> > + if (!should_merge_hunk(file_diff, *hunk_index, use_all, merge_for_discard)) {\n> > return 0;\n> > + }\n>\n> There's no need to add braces here\n>\n> > @@ -1014,11 +1032,13 @@ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,\n> >\n> > static void reassemble_patch(struct add_p_state *s,\n> > struct file_diff *file_diff, int use_all,\n> > + enum reassemble_mode mode,\n> > struct strbuf *out)\n> > {\n> > struct hunk *hunk;\n> > size_t save_len = s->plain.len, i;\n> > ssize_t delta = 0;\n> > + int merge_for_discard = (mode == REASSEMBLE_DISCARD);\n>\n> It would by simpler just to take USE_HUNK or DISCARD_HUNK as a parameter\n> and pass that to reassemble_patch() rather than forcing the caller to\n> pass an enum that we then transform to a boolean.\n>\n> >\n> > render_diff_header(s, file_diff, 0, out);\n> >\n> > @@ -1026,25 +1046,26 @@ static void reassemble_patch(struct add_p_state *s,\n> > struct hunk merged = { 0 };\n> >\n> > hunk = file_diff->hunk + i;\n> > - if (!use_all && hunk->use != USE_HUNK)\n> > + if (!should_merge_hunk(file_diff, i, use_all, merge_for_discard)) {\n> > delta += hunk->header.old_count\n> > - hunk->header.new_count;\n> > - else {\n> > - /* merge overlapping hunks into a temporary hunk */\n> > - if (merge_hunks(s, file_diff, &i, use_all, &merged))\n> > - hunk = &merged;\n> > + continue;\n>\n> I'm not sure this is an improvement - it certainly makes the patch\n> harder to read because you end up changing the indentation of otherwise\n> unchanged lines that were in the else clause.\n>\n> > + }\n> >\n> > - render_hunk(s, hunk, delta, 0, out);\n> > + if (merge_hunks(s, file_diff, &i, use_all, &merged,\n> > + merge_for_discard))\n> > + hunk = &merged;\n> >\n> > - /*\n> > - * In case `merge_hunks()` used `plain` as a scratch\n> > - * pad (this happens when an edited hunk had to be\n> > - * coalesced with another hunk).\n> > - */\n> > - strbuf_setlen(&s->plain, save_len);\n> > + render_hunk(s, hunk, delta, 0, out);\n>\n> We need to tell render_hunk whether a hunk is being applied in reverse\n> or not so that it knows whether to apply delta to the old offset or the\n> new offset. Currently it uses s->mode->reverse but that will not be\n> correct for the patch that discards changes from the working tree. I'm\n> not sure what the best way of doing that is, the simplest approach is to\n> invert s->mode->reverse when we want to keep hunks marked DISCARD_HUNK\n> and then restore the original value.\n>\n> > - delta += hunk->delta;\n> > - }\n> > + /*\n> > + * In case `merge_hunks()` used `plain` as a scratch\n> > + * pad (this happens when an edited hunk had to be\n> > + * coalesced with another hunk).\n> > + */\n> > + strbuf_setlen(&s->plain, save_len);\n> > +\n> > + delta += hunk->delta;\n>\n> This is unchanged code that re-indented because of the addition of\n> \"continue\" above.\n>\n> > }\n> > }\n>\n> > @@ -1540,6 +1562,7 @@ N_(\"j - go to the next undecided hunk, roll over at the bottom\\n\"\n> > \"/ - search for a hunk matching the given regex\\n\"\n> > \"s - split the current hunk into smaller hunks\\n\"\n> > \"e - manually edit the current hunk\\n\"\n> > + \"x - discard this hunk from the worktree\\n\"\n>\n> It would be nice to avoid showing this for 'checkout -p' etc.\n>\n> > \"p - print the current hunk\\n\"\n> > \"P - print the current hunk using the pager\\n\"\n> > \"> - go to the next file, roll over at the bottom\\n\"\n> > @@ -1547,21 +1570,57 @@ N_(\"j - go to the next undecided hunk, roll over at the bottom\\n\"\n> > \"? - print help\\n\"\n> > \"HUNKS SUMMARY - Hunks: %d, USE: %d, SKIP: %d\\n\");\n> >\n> > +static int apply_discard_hunks(struct add_p_state *s,\n> > + struct file_diff *file_diff)\n> > +{\n> > + struct child_process check_cp = CHILD_PROCESS_INIT;\n> > + struct child_process apply_cp = CHILD_PROCESS_INIT;\n> > +\n> > + strbuf_reset(&s->buf);\n> > + reassemble_patch(s, file_diff, 0, REASSEMBLE_DISCARD, &s->buf);\n> > +\n> > + discard_index(s->index);\n> > +\n> > + setup_child_process(s, &check_cp, \"apply\", \"-R\", \"--check\", NULL);\n> > + if (pipe_command(&check_cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) {\n> > + error(_(\"'git apply -R --check' failed\"));\n> > + return -1;\n> > + }\n>\n> Why do we need to run \"git apply --check\" here?\n>\n> > + setup_child_process(s, &apply_cp, \"apply\", \"-R\", NULL);\n> > + if (pipe_command(&apply_cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) {\n> > + error(_(\"'git apply -R' failed\"));\n> > + return -1;\n> > + }\n> > +\n> > + return 0;\n> > +}\n> > +\n> > static void apply_patch(struct add_p_state *s, struct file_diff *file_diff)\n> > {\n> > struct child_process cp = CHILD_PROCESS_INIT;\n> > size_t j;\n> > + int needs_refresh = 0;\n> > +\n> > + if (s->mode == &patch_mode_add) {\n> > + for (j = 0; j < file_diff->hunk_nr; j++) {\n> > + if (file_diff->hunk[j].use == DISCARD_HUNK)\n> > + break;\n> > + }\n> > + if (j < file_diff->hunk_nr && apply_discard_hunks(s, file_diff))\n> > + return;\n> > + if (j < file_diff->hunk_nr)\n> > + needs_refresh = 1;\n> > + }\n> >\n> > - /* Any hunk to be used? */\n>\n> Isn't this comment still relevant?\n>\n> > for (j = 0; j < file_diff->hunk_nr; j++)\n> > if (file_diff->hunk[j].use == USE_HUNK)\n> > break;\n> >\n> > if (j < file_diff->hunk_nr ||\n> > - (!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) {\n> > - /* At least one hunk selected: apply */\n> > + (!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) {\n>\n> What's the point of this change?\n>\n> > strbuf_reset(&s->buf);\n> > - reassemble_patch(s, file_diff, 0, &s->buf);\n> > + reassemble_patch(s, file_diff, 0, REASSEMBLE_STAGE, &s->buf);\n> >\n> > discard_index(s->index);\n> > if (s->mode->apply_for_checkout)\n> > @@ -1574,13 +1633,15 @@ static void apply_patch(struct add_p_state *s, struct file_diff *file_diff)\n> > NULL, 0, NULL, 0))\n> > error(_(\"'git apply' failed\"));\n> > }\n> > - if (read_index_from(s->index, s->index_file, s->r->gitdir) >= 0 &&\n> > - s->index == s->r->index) {\n> > - repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0,\n> > - 1, NULL, NULL, NULL);\n> > - }\n> > + needs_refresh = 1;\n> > }\n> >\n> > + if (needs_refresh &&\n> > + read_index_from(s->index, s->index_file, s->r->gitdir) >= 0 &&\n> > + s->index == s->r->index) {\n> > + repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0,\n> > + 1, NULL, NULL, NULL);\n>\n> We now wait until we've applied both patches before refreshing the index\n> - sounds sensible.\n>\n> > + }\n> > }\n>\n>\n> > @@ -1722,6 +1784,10 @@ static size_t patch_update_file(struct add_p_state *s,\n> > !file_diff->deleted) {\n> > permitted |= ALLOW_EDIT;\n> > strbuf_addstr(&s->buf, \",e\");\n> > + if (s->mode == &patch_mode_add) {\n> > + permitted |= ALLOW_DISCARD;\n> > + strbuf_addstr(&s->buf, \",x\");\n> > + }\n>\n> So 'x' is only permitted if 'e' is what's the reason for that?\n> > diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> > [...]\n> > +test_expect_success 'add -p discard removes worktree change' '\n> > + test_when_finished \"rm -rf discard-testrepo\" &&\n> > + mkdir discard-testrepo &&\n>\n> It's nice to see a test for 'x', but I'm not sure why this test needs to\n> be in a separate repository - why can't it use the same repository as\n> the other tests?\n>\n> > + (\n> > + cd discard-testrepo &&\n> > + git init -b main &&\n> > + echo clean >discard-me &&\n> > + git add discard-me &&\n> > + git commit -m base &&\n> > + echo extra >>discard-me &&\n> > + test_write_lines x | git add -p discard-me &&\n> > + printf \"clean\\n\" >expect &&\n> > + test_cmp expect discard-me &&\n> > + git diff --cached >tmp &&\n> > + test_must_be_empty tmp\n>\n> It would be nice to see tests that split a hunk like\n>\n> -a\n> +A\n> b\n> -c\n> +C\n> d\n> -e\n> +E\n>\n> and then (1) stage the first and third sub-hunks and discard the second,\n> (2) discard the first and third sub-hunks and stage the second. It would\n> also be nice to see a test that discards a hunk with pathological\n> context lines - see 2bd69b9024c (add -p: fix checkout -p with\n> pathological context, 2019-06-12) for an example.\n>\n> Thanks\n>\n> Phillip\n>\n>\n> > + )\n> > +'\n> > +\n> > test_expect_success 'setup expected' '\n> > cat >expected <<-\\EOF\n> > diff --git a/non-empty b/non-empty\n> > @@ -521,13 +539,13 @@ test_expect_success 'split hunk setup' '\n> > test_expect_success 'goto hunk 1 with \"g 1\"' '\n> > test_when_finished \"git reset\" &&\n> > tr _ \" \" >expect <<-EOF &&\n> > - (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? + 1: -1,2 +1,3 +15\n> > + (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,x,p,P,?]? + 1: -1,2 +1,3 +15\n> > _ 2: -2,4 +3,8 +21\n> > go to which hunk? @@ -1,2 +1,3 @@\n> > _10\n> > +15\n> > _20\n> > - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n> > + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n> > EOF\n> > test_write_lines s y g 1 | git add -p >actual &&\n> > tail -n 7 actual.trimmed &&\n> > @@ -540,7 +558,7 @@ test_expect_success 'goto hunk 1 with \"g1\"' '\n> > _10\n> > +15\n> > _20\n> > - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n> > + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n> > EOF\n> > test_write_lines s y g1 | git add -p >actual &&\n> > tail -n 4 actual.trimmed &&\n> > @@ -550,11 +568,11 @@ test_expect_success 'goto hunk 1 with \"g1\"' '\n> > test_expect_success 'navigate to hunk via regex /pattern' '\n> > test_when_finished \"git reset\" &&\n> > tr _ \" \" >expect <<-EOF &&\n> > - (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@\n> > + (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,x,p,P,?]? @@ -1,2 +1,3 @@\n> > _10\n> > +15\n> > _20\n> > - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n> > + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n> > EOF\n> > test_write_lines s y /1,2 | git add -p >actual &&\n> > tail -n 5 actual.trimmed &&\n> > @@ -567,7 +585,7 @@ test_expect_success 'navigate to hunk via regex / pattern' '\n> > _10\n> > +15\n> > _20\n> > - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n> > + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n> > EOF\n> > test_write_lines s y / 1,2 | git add -p >actual &&\n> > tail -n 4 actual.trimmed &&\n> > @@ -579,11 +597,11 @@ test_expect_success 'print again the hunk' '\n> > tr _ \" \" >expect <<-EOF &&\n> > +15\n> > 20\n> > - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@\n> > + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? @@ -1,2 +1,3 @@\n> > 10\n> > +15\n> > 20\n> > - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_\n> > + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?_\n> > EOF\n> > test_write_lines s y g 1 p | git add -p >actual &&\n> > tail -n 7 actual.trimmed &&\n> > @@ -595,11 +613,11 @@ test_expect_success TTY 'print again the hunk (PAGER)' '\n> > cat >expect <<-EOF &&\n> > +15\n> > 20\n> > - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? PAGER @@ -1,2 +1,3 @@\n> > + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? PAGER @@ -1,2 +1,3 @@\n> > PAGER 10\n> > PAGER +15\n> > PAGER 20\n> > - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? \n> > + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? \n> > EOF\n> > test_write_lines s y g 1 P |\n> > (\n> > @@ -796,21 +814,21 @@ test_expect_success 'colors can be overridden' '\n> > +new\n> > more-context\n> > +another-one\n> > - (1/1) Stage this hunk [y,n,q,a,d,s,e,p,P,?]? Split into 2 hunks.\n> > + (1/1) Stage this hunk [y,n,q,a,d,s,e,x,p,P,?]? Split into 2 hunks.\n> > @@ -1,3 +1,3 @@\n> > context\n> > -old\n> > +new\n> > more-context\n> > - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? @@ -3 +3,2 @@\n> > + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? @@ -3 +3,2 @@\n> > more-context\n> > +another-one\n> > - (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? @@ -1,3 +1,3 @@\n> > + (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,x,p,P,?]? @@ -1,3 +1,3 @@\n> > context\n> > -old\n> > +new\n> > more-context\n> > - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? \n> > + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? \n> > EOF\n> > test_cmp expect actual\n> > '\n> > @@ -1424,9 +1442,9 @@ test_expect_success 'invalid option s is rejected' '\n> > test_write_lines j s q | git add -p >out &&\n> > sed -ne \"s/ @@.*//\" -e \"s/ \\$//\" -e \"/^(/p\" actual &&\n> > cat >expect <<-EOF &&\n> > - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,p,P,?]?\n> > - (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? Sorry, cannot split this hunk\n> > - (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?\n> > + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,x,p,P,?]?\n> > + (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]? Sorry, cannot split this hunk\n> > + (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,x,p,P,?]?\n> > EOF\n> > test_cmp expect actual\n> > '\n>\n"},{"id":"539982","messageId":"CAN+A6Tsmc9zo+jYCurEjG+oz+FtNJv1CbVGBrJaRKY27N-=pTA@mail.gmail.com","threadId":"65350","inReplyTo":"a4305ef7-50ff-4a68-ab42-fe2fa73e8f37@gmx.de","subject":"Re: [RFC PATCH 1/1] add -p: support discarding hunks with 'x'","fromName":"Luiz Eduardo Campos","fromEmail":"luizedc1@gmail.com","sentAt":"2026-03-25T18:58:13Z","receivedAt":"2026-03-25T18:59:34Z","isPatch":true,"sender":{"key":"luizedc1@gmail.com","avatar":null},"body":"Hi Johannes,\n\nThank you for taking the time to review this!\n\n> This hunk is quite hard to read because of the `if ... else ...` -> `if {\n> ... continue; } ...` change that de-indents a large chunk of code.\n\nYou are right. Even though you marked it as \"not really actionable\",\nI think it is worth fixing: I can keep the original if/else structure\nand just replace the condition with the should_merge_hunk() call.\nThat avoids the indentation churn and keeps the diff focused on what\nactually changes.\n\n> If you detect an empty patch here and indicate this via an early return\n> value, then...\n> ... then this loop is no longer necessary.\n\nGood idea. I will have apply_discard_hunks() check whether any hunk\nis marked DISCARD_HUNK before going through the apply machinery, and\nuse the return value to distinguish \"nothing to do\" from \"applied\" and\n\"error\". With that in place the pre-scan loop in apply_patch() can be\ndropped, and needs_refresh can just be set based on whether\napply_discard_hunks() actually applied something.\n\nI will address this in v2; the implementation might still live in add -p,\nor I may fold it into a shared path that fits better (suggestions are\nwelcome!)\n\nThanks,\nLuiz\n\nEm qua., 25 de mar. de 2026 às 13:49, Johannes Schindelin\n escreveu:\n>\n> Hi Luiz,\n>\n> On Wed, 25 Mar 2026, Luiz Campos wrote:\n>\n> > When using `git add -p`, users can stage or skip hunks,\n> > but cannot discard unwanted changes from the working tree.\n> >\n> > Introduce a new 'x' action to discard the current hunk by\n> > reverse-applying it.\n> >\n> > This idea was suggested in a previous mailing list discussion:\n> > https://lore.kernel.org/git/X%2FiFCo0bXLR%2BLZXs@coredump.intra.peff.net/t/#m0576e6f3c6375e11cc4693b9dca3c1fc57baadd0\n>\n> Sounds good!\n>\n> Just two minor comments (not really actionable, I think):\n>\n> > @@ -1026,25 +1046,26 @@ static void reassemble_patch(struct add_p_state *s,\n> > struct hunk merged = { 0 };\n> >\n> > hunk = file_diff->hunk + i;\n> > - if (!use_all && hunk->use != USE_HUNK)\n> > + if (!should_merge_hunk(file_diff, i, use_all, merge_for_discard)) {\n> > delta += hunk->header.old_count\n> > - hunk->header.new_count;\n> > - else {\n> > - /* merge overlapping hunks into a temporary hunk */\n> > - if (merge_hunks(s, file_diff, &i, use_all, &merged))\n> > - hunk = &merged;\n> > + continue;\n> > + }\n> >\n> > - render_hunk(s, hunk, delta, 0, out);\n> > + if (merge_hunks(s, file_diff, &i, use_all, &merged,\n> > + merge_for_discard))\n> > + hunk = &merged;\n> >\n> > - /*\n> > - * In case `merge_hunks()` used `plain` as a scratch\n> > - * pad (this happens when an edited hunk had to be\n> > - * coalesced with another hunk).\n> > - */\n> > - strbuf_setlen(&s->plain, save_len);\n> > + render_hunk(s, hunk, delta, 0, out);\n> >\n> > - delta += hunk->delta;\n> > - }\n> > + /*\n> > + * In case `merge_hunks()` used `plain` as a scratch\n> > + * pad (this happens when an edited hunk had to be\n> > + * coalesced with another hunk).\n> > + */\n> > + strbuf_setlen(&s->plain, save_len);\n> > +\n> > + delta += hunk->delta;\n>\n> This hunk is quite hard to read because of the `if ... else ...` -> `if {\n> ... continue; } ...` change that de-indents a large chunk of code.\n>\n> After pouring over the diff for a bit, I was able to convince myself that\n> the diff is correct.\n>\n> > @@ -1547,21 +1570,57 @@ N_(\"j - go to the next undecided hunk, roll over at the bottom\\n\"\n> > \"? - print help\\n\"\n> > \"HUNKS SUMMARY - Hunks: %d, USE: %d, SKIP: %d\\n\");\n> >\n> > +static int apply_discard_hunks(struct add_p_state *s,\n> > + struct file_diff *file_diff)\n> > +{\n> > + struct child_process check_cp = CHILD_PROCESS_INIT;\n> > + struct child_process apply_cp = CHILD_PROCESS_INIT;\n> > +\n> > + strbuf_reset(&s->buf);\n> > + reassemble_patch(s, file_diff, 0, REASSEMBLE_DISCARD, &s->buf);\n>\n> If you detect an empty patch here and indicate this via an early return\n> value, then...\n>\n> > +\n> > + discard_index(s->index);\n> > +\n> > + setup_child_process(s, &check_cp, \"apply\", \"-R\", \"--check\", NULL);\n> > + if (pipe_command(&check_cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) {\n> > + error(_(\"'git apply -R --check' failed\"));\n> > + return -1;\n> > + }\n> > +\n> > + setup_child_process(s, &apply_cp, \"apply\", \"-R\", NULL);\n> > + if (pipe_command(&apply_cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0)) {\n> > + error(_(\"'git apply -R' failed\"));\n> > + return -1;\n> > + }\n> > +\n> > + return 0;\n> > +}\n> > +\n> > static void apply_patch(struct add_p_state *s, struct file_diff *file_diff)\n> > {\n> > struct child_process cp = CHILD_PROCESS_INIT;\n> > size_t j;\n> > + int needs_refresh = 0;\n> > +\n> > + if (s->mode == &patch_mode_add) {\n> > + for (j = 0; j < file_diff->hunk_nr; j++) {\n> > + if (file_diff->hunk[j].use == DISCARD_HUNK)\n> > + break;\n> > + }\n> > + if (j < file_diff->hunk_nr && apply_discard_hunks(s, file_diff))\n> > + return;\n> > + if (j < file_diff->hunk_nr)\n> > + needs_refresh = 1;\n> > + }\n>\n> ... then this loop is no longer necessary.\n>\n> Other than that, looks good to me!\n>\n> Ciao,\n> Johannes\n"},{"id":"539986","messageId":"CAN+A6TtsGGQZ+3Q+MSp_kKzxcwMgmCp1bd+tD6y9U2FfPqSLFQ@mail.gmail.com","threadId":"65350","inReplyTo":"xmqqcy0rvlao.fsf@gitster.g","subject":"Re: [RFC PATCH 0/1] add -p: support discarding hunks","fromName":"Luiz Eduardo Campos","fromEmail":"luizedc1@gmail.com","sentAt":"2026-03-25T19:22:12Z","receivedAt":"2026-03-25T19:23:34Z","isPatch":true,"sender":{"key":"luizedc1@gmail.com","avatar":null},"body":"Hi Junio,\n\nThanks a lot for the detailed feedback.\n\n> \"add -p\" that touches the working tree feels quite weird.\n\nI understand the concern, and I agree that having `git add -p`\nperform destructive changes on the working tree is a significant\ndeparture from its current mental model.\n\n> people trigger \"discard\" by mistake ... no way to undo\n\nThis is a very good point. I had been thinking of this primarily\nas a convenience for workflows where users repeatedly skip hunks,\nbut you are right that making such an operation easy to trigger\nin an interactive session could lead to accidental data loss,\nand that would be problematic.\n\n> What you want is probably a single command ...\n\nThis is a very interesting direction. My original motivation was\nexactly to avoid having to switch between `git add -p`,\n`git restore -p`, and `git stash -p` when reviewing changes,\nbut I had not considered approaching it as a single interface with\nmodes over the different views (HEAD, index, worktree) and\nnon-destructive flows like stashing.\n\nThat does seem like a more coherent model. In the meantime I will\nrely on the workflows you mentioned (`stash -p` or `restore -p`\nfollowed by `git add -p`) rather than pursuing discard within\n`add -p` alone.\n\nI will step back from the narrow \"discard in add -p\" RFC while I\nthink more about this broader direction. One question I would like\nguidance on is whether something like what you describe would be\nmore appropriate as a new top-level command, or as an extension of\nan existing one (and if so, which entry point would be the least\nsurprising).\n\nIf a unified tool ever includes a destructive \"revert hunk in the\nworktree\" mode, I agree it would need to be very clearly separated\nand hard to trigger by mistake.\n\nThanks again for the guidance; it is very helpful.\n\nLuiz\n\nEm qua., 25 de mar. de 2026 às 15:03, Junio C Hamano\n escreveu:\n>\n> Luiz Campos writes:\n>\n> > Hi,\n> >\n> > This is an RFC for adding a 'discard hunk' action to `git add -p`.\n> >\n> > Currently, when using `git add -p`, users can stage or skip hunks,\n> > but cannot discard unwanted changes directly from the working tree.\n> > This often leads to repeatedly skipping the same hunks across\n> > multiple passes.\n> >\n> > This patch introduces a new 'x' action to discard the current hunk\n> > by reverse-applying it to the working tree.\n> >\n> > This idea was previously discussed on the mailing list:\n> > https://lore.kernel.org/git/X%2FiFCo0bXLR%2BLZXs@coredump.intra.peff.net/t/#m0576e6f3c6375e11cc4693b9dca3c1fc57baadd0\n> >\n> > Open questions:\n> > - Should discard happen immediately or be deferred until patch application?\n> > - Are there edge cases involving overlapping hunks or edited hunks?\n>\n> After reading the discussion (by the way, I do not recall seeing it,\n> so thank you very much for having a link to it), I agree with what\n> Peff said back then. \"add -p\" that touches the working tree feels\n> quite weird.\n>\n> In addition to that, letting it make destructive change makes the\n> idea even less appetizing. Once you remove the changes introduced\n> by the hunk, it is forever gone. A \"discard\" in \"add -p\" would not\n> solve your problem without adding many unhappy users who lost their\n> work by mistake. I do not want to see people trigger \"discard\" by\n> mistake in \"add -p\" session _and_ find that there is no way to undo\n> that mistaken discard.\n>\n> \"stash -p\" followed by \"add -p\" is probably the best we can do that\n> is safe. When the unwanted change is truly unwanted garbage that\n> you would never ever want to see again, \"restore -p\" followed by\n> \"add -p\" would be an alternative.\n>\n> One reason why they are not satisfying is because during the later\n> \"add -p\" session, we will notice that some unwanted things we failed\n> to notice and get rid of (either by sending them to stash or restoring\n> it away) are still there, reminding us that we are imperfect human,\n> and at that point, it is not easy to switch back to the \"stash -p\"\n> or \"restore -p\" from there.\n>\n> What you want is probably a _single_ command that lets you inspect\n> the differences among the HEAD, the index, and the working tree, and\n> allows you to move things hunk-by-hunk in different directions.\n>\n> * You can go through the \"git diff --cached\" (i.e., changes already in\n> the index), and selectively undo/revert the changes to the index,\n> similar to \"git reset -p\".\n>\n> * You can go through the \"git diff\" (i.e., changes between the\n> index and the working tree), and selectively apply the changes to\n> the index, similar to \"git add -p\".\n>\n> * You can go through the \"git diff HEAD\" (i.e. changes since your\n> last commit), and selectively send the changes to a stash entry,\n> similar to \"git stash -p\". This is not destructive.\n>\n> And if the single command lets you switch among working with these\n> modes, you no longer need to worry about forgetting to send\n> some changes to stash to concentrate on working on the rest.\n>\n> In addition, optionally you can also have this in the same command:\n>\n> * You can go through the \"git diff\", and selectively revert the\n> changes to the working tree, similar to \"git restore -p\".\n>\n> This additional mode *is* destructive, but if you know from the hunk\n> that you will never need the change in it, it would be a right tool\n> for it.\n"}]}