* git add intent-to-add then git add patch no longer allows edit @ 2020-08-21 4:27 Thomas Sullivan 2020-08-21 5:25 ` Raymond E. Pasco 0 siblings, 1 reply; 16+ messages in thread From: Thomas Sullivan @ 2020-08-21 4:27 UTC (permalink / raw) To: git What did you do before the bug happened? (Steps to reproduce your issue) rm -rf .git cat << EOF > file Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. EOF git init . git add --intent-to-add file git add --patch file What did you expect to happen? (Expected behavior) To be able to edit the single hunk (`e` option) What happened instead? (Actual behavior) The `e` option isn't available (and providing it anyway errors with `Sorry, cannot edit this hunk`) Anything else you want to add: Output of version 2.28.0: diff --git a/file b/file index 0000000..85e73d4 --- /dev/null +++ b/file new file mode 100644 @@ -0,0 +1 @@ +Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. (1/1) Stage addition [y,n,q,a,d,?]? Output of version 2.27.0: diff --git a/file b/file index e69de29..85e73d4 100644 --- a/file +++ b/file @@ -0,0 +1 @@ +Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. (1/1) Stage this hunk [y,n,q,a,d,e,?]? Bisecting between the two tags reports this commit as the one introducing the change: [feea6946a5b746ff4ebf8ccdf959e303203a6011] diff-files: treat "i-t-a" files as "not-in-index" [System Info] git version: git version 2.28.0 cpu: x86_64 no commit associated with this build sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh uname: Darwin 17.7.0 Darwin Kernel Version 17.7.0: Thu Jun 18 21:21:34 PDT 2020; root:xnu-4570.71.82.5~1/RELEASE_X86_64 x86_64 compiler info: clang: 10.0.0 (clang-1000.10.44.4) libc info: no libc information available $SHELL (typically, interactive shell): /bin/bash [Enabled Hooks] Regards, Tom Sullivan Head Developer Most Significant Bit Software e: tom@msbit.com.au p: +61 407 890 821 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-08-21 4:27 git add intent-to-add then git add patch no longer allows edit Thomas Sullivan @ 2020-08-21 5:25 ` Raymond E. Pasco 2020-08-21 16:27 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Raymond E. Pasco @ 2020-08-21 5:25 UTC (permalink / raw) To: Thomas Sullivan, git; +Cc: Phillip Wood I fixed half of this in a topic that's on master now (it errors out entirely if you try to stage it at all in 2.28.0), but new file diffs still aren't splittable into hunks. Phillip Wood (on cc) is looking into that; the tricky part is that when split into hunks only the first hunk actually staged can be a "new file" patch. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-08-21 5:25 ` Raymond E. Pasco @ 2020-08-21 16:27 ` Junio C Hamano 2020-08-23 16:03 ` Phillip Wood 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2020-08-21 16:27 UTC (permalink / raw) To: Raymond E. Pasco; +Cc: Thomas Sullivan, git, Phillip Wood "Raymond E. Pasco" <ray@ameretat.dev> writes: > I fixed half of this in a topic that's on master now (it errors out > entirely if you try to stage it at all in 2.28.0), Yup, thanks for that one. > but new file diffs > still aren't splittable into hunks. Phillip Wood (on cc) is looking into > that; the tricky part is that when split into hunks only the first hunk > actually staged can be a "new file" patch. Out of a change that adds a file with three parts A, B and C (in this order), you could pick the parts A and C, while leaving the change to further add B in the middle, and create a patch to add a file that has A and C, and apply that to the index alone (i.e. "add -p", pick A and C, and "add" that part by applying that "new file" diff). After that, the path is no longer i-t-a but has the real contents (i.e. part A followed by part C), so further "add -p" would see the difference between the index and the working tree as a modification patch. So as long as you could come up with a good UI to pick parts from a single hunk "new file" diff, "the second and later application must be done as modification" should fall out naturally, no? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-08-21 16:27 ` Junio C Hamano @ 2020-08-23 16:03 ` Phillip Wood 2020-08-24 16:23 ` Raymond E. Pasco 0 siblings, 1 reply; 16+ messages in thread From: Phillip Wood @ 2020-08-23 16:03 UTC (permalink / raw) To: Junio C Hamano, Raymond E. Pasco; +Cc: Thomas Sullivan, git On 21/08/2020 17:27, Junio C Hamano wrote: > "Raymond E. Pasco" <ray@ameretat.dev> writes: > >> I fixed half of this in a topic that's on master now (it errors out >> entirely if you try to stage it at all in 2.28.0), > > Yup, thanks for that one. > >> but new file diffs >> still aren't splittable into hunks. Phillip Wood (on cc) is looking into >> that; the tricky part is that when split into hunks only the first hunk >> actually staged can be a "new file" patch. > > Out of a change that adds a file with three parts A, B and C (in > this order), you could pick the parts A and C, while leaving the > change to further add B in the middle, and create a patch to add a > file that has A and C, and apply that to the index alone (i.e. "add > -p", pick A and C, and "add" that part by applying that "new file" > diff). After that, the path is no longer i-t-a but has the real > contents (i.e. part A followed by part C), so further "add -p" would > see the difference between the index and the working tree as a > modification patch. > > So as long as you could come up with a good UI to pick parts from a > single hunk "new file" diff, "the second and later application must > be done as modification" should fall out naturally, no? I think I was talking about edit rather than split. I'd forgotten that it used to work with i-t-a additions. I just checked seen and it seems to be working again since dscho's patch although the user is presented with the full diff header rather than just a hunk header in the editor. As you say once the user has staged some part of the diff the rest falls out naturally. Best Wishes Phillip (I'm about to go off line for a while) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-08-23 16:03 ` Phillip Wood @ 2020-08-24 16:23 ` Raymond E. Pasco 2020-08-24 17:28 ` Phillip Wood 0 siblings, 1 reply; 16+ messages in thread From: Raymond E. Pasco @ 2020-08-24 16:23 UTC (permalink / raw) To: phillip.wood, Junio C Hamano; +Cc: Thomas Sullivan, git On Sun Aug 23, 2020 at 12:03 PM EDT, Phillip Wood wrote: > I think I was talking about edit rather than split. I'd forgotten that > it used to work with i-t-a additions. I just checked seen and it seems > to be working again since dscho's patch although the user is presented > with the full diff header rather than just a hunk header in the editor. > As you say once the user has staged some part of the diff the rest falls > out naturally. Which topic is this? I can't find one where it works (it's always "Sorry, cannot edit this hunk" on seen 2.28.0.508.g7d1bebc7fe). Yeah, it's split that would be a problem, edit just stages and moves on. Split would be nice, but I don't actually recall it ever working before - it doesn't work now on diffs from actual blank files. Getting edit back (if there's a topic that does this already) makes it work for my usage. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-08-24 16:23 ` Raymond E. Pasco @ 2020-08-24 17:28 ` Phillip Wood 2020-08-24 21:03 ` Raymond E. Pasco 0 siblings, 1 reply; 16+ messages in thread From: Phillip Wood @ 2020-08-24 17:28 UTC (permalink / raw) To: Raymond E. Pasco, phillip.wood, Junio C Hamano; +Cc: Thomas Sullivan, git Hi Raymond On 24/08/2020 17:23, Raymond E. Pasco wrote: > On Sun Aug 23, 2020 at 12:03 PM EDT, Phillip Wood wrote: >> I think I was talking about edit rather than split. I'd forgotten that >> it used to work with i-t-a additions. I just checked seen and it seems >> to be working again since dscho's patch although the user is presented >> with the full diff header rather than just a hunk header in the editor. >> As you say once the user has staged some part of the diff the rest falls >> out naturally. > > Which topic is this? I can't find one where it works (it's always > "Sorry, cannot edit this hunk" on seen 2.28.0.508.g7d1bebc7fe). The patch I was referring to is 2c8bd8471a ("checkout -p: handle new files correctly", 2020-05-27) I tested seen at 3981657b13 ("Merge branch 'rp/apply-cached-doc' into seen", 2020-08-21). I was using the C version of 'add -p' which is opt-in at the moment by setting add.interactive.usebuiltin=true in your config (or with git -c). I hope that helps, I'm going off line now for 10-14 days Best Wishes Phillip > Yeah, it's split that would be a problem, edit just stages and moves on. > Split would be nice, but I don't actually recall it ever working before > - it doesn't work now on diffs from actual blank files. Getting edit > back (if there's a topic that does this already) makes it work for my > usage. > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-08-24 17:28 ` Phillip Wood @ 2020-08-24 21:03 ` Raymond E. Pasco 2020-09-04 10:05 ` Phillip Wood 0 siblings, 1 reply; 16+ messages in thread From: Raymond E. Pasco @ 2020-08-24 21:03 UTC (permalink / raw) To: phillip.wood, phillip.wood, Junio C Hamano; +Cc: Thomas Sullivan, git On Mon Aug 24, 2020 at 1:28 PM EDT, Phillip Wood wrote: > The patch I was referring to is 2c8bd8471a ("checkout -p: handle new > files correctly", 2020-05-27) > > I tested seen at 3981657b13 ("Merge branch 'rp/apply-cached-doc' into > seen", 2020-08-21). I was using the C version of 'add -p' which is > opt-in at the moment by setting add.interactive.usebuiltin=true in your > config (or with git -c). I hope that helps, I'm going off line now for > 10-14 days Indeed, this works and restores my workflow (although it errors out if I don't manually edit the range information, which isn't necessary with diffs to existing files). It's a bit unsatisfying as it stands, but perhaps there are patches I can write. No need to reply, enjoy your vacation! ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-08-24 21:03 ` Raymond E. Pasco @ 2020-09-04 10:05 ` Phillip Wood 2020-09-04 6:56 ` Johannes Schindelin 2020-09-05 18:36 ` Raymond E. Pasco 0 siblings, 2 replies; 16+ messages in thread From: Phillip Wood @ 2020-09-04 10:05 UTC (permalink / raw) To: Raymond E. Pasco, phillip.wood, Junio C Hamano Cc: Thomas Sullivan, git, Johannes Schindelin On 24/08/2020 22:03, Raymond E. Pasco wrote: > On Mon Aug 24, 2020 at 1:28 PM EDT, Phillip Wood wrote: >> The patch I was referring to is 2c8bd8471a ("checkout -p: handle new >> files correctly", 2020-05-27) >> >> I tested seen at 3981657b13 ("Merge branch 'rp/apply-cached-doc' into >> seen", 2020-08-21). I was using the C version of 'add -p' which is >> opt-in at the moment by setting add.interactive.usebuiltin=true in your >> config (or with git -c). I hope that helps, I'm going off line now for >> 10-14 days > > Indeed, this works and restores my workflow (although it errors out if I > don't manually edit the range information, which isn't necessary with > diffs to existing files). It's a bit unsatisfying as it stands, but > perhaps there are patches I can write. > > No need to reply, enjoy your vacation! Thanks, it was really good to get a change of scene. The patch below fixes the hunk editing for new files in the C version of `add -p` if anyone wants to try it out. I haven't looked at fixing the perl version yet - dscho what are your plans for switching over to the C version? Best Wishes Phillip ---- >8 ---- From b0df1953308f8de5224a2d99d435f93cc4093a17 Mon Sep 17 00:00:00 2001 From: Phillip Wood <phillip.wood@dunelm.org.uk> Date: Wed, 2 Sep 2020 15:25:55 +0100 Subject: [PATCH] add -p: fix editing of intent-to-add paths A popular way of partially staging a new file is to run `git add -N <path>` and then use the hunk editing of `git add -p` to select the part of the file that the user wishes to stage. Since 85953a3187 ("diff-files --raw: show correct post-image of intent-to-add files", 2020-07-01) this has stopped working as intent-to-add paths are now show as new files rather than changes to an empty blob and `git apply` refused to apply a creation patch for a path that was marked as intent-to-add. 7cfde3fa0f ("apply: allow "new file" patches on i-t-a entries", 2020-08-06) fixed the problem with apply but it still wasn't possible to edit the added hunk properly. 2c8bd8471a ("checkout -p: handle new files correctly", 2020-05-27) had previously changed `add -p` to handle new files but it did not implement patch editing correctly. The perl version simply forbade editing and the C version opened the editor with the full diff rather that just the hunk which meant that the user had to edit the hunk header manually to get it to work. This patch only fixes the C version to correctly edit new file patches. To test the C version the tests must be run with GIT_TEST_ADD_I_USE_BUILTIN=1. It is best viewed with --color-moved-ws=allow-indentation-change Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Reported-by: Thomas Sullivan <tom@msbit.com.au> --- add-patch.c | 83 +++++++++++++++++++++----------------- t/t3701-add-interactive.sh | 44 +++++++++++++++++++- 2 files changed, 89 insertions(+), 38 deletions(-) diff --git a/add-patch.c b/add-patch.c index f67b304a55..209a63e4f2 100644 --- a/add-patch.c +++ b/add-patch.c @@ -451,7 +451,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) pend = p + plain->len; while (p != pend) { char *eol = memchr(p, '\n', pend - p); - const char *deleted = NULL, *added = NULL, *mode_change = NULL; + const char *mode_change = NULL; if (!eol) eol = pend; @@ -470,12 +470,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } else if (p == plain->buf) BUG("diff starts with unexpected line:\n" "%.*s\n", (int)(eol - p), p); - else if (file_diff->deleted || file_diff->added) - ; /* keep the rest of the file in a single "hunk" */ - else if (starts_with(p, "@@ ") || - (hunk == &file_diff->head && - (skip_prefix(p, "deleted file", &deleted) || - skip_prefix(p, "new file", &added)))) { + else if (starts_with(p, "@@ ")) { if (marker == '-' || marker == '+') /* * Should not happen; previous hunk did not end @@ -493,18 +488,20 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) if (colored) hunk->colored_start = colored_p - colored->buf; - if (deleted) - file_diff->deleted = 1; - else if (added) - file_diff->added = 1; - else if (parse_hunk_header(s, hunk) < 0) + if (parse_hunk_header(s, hunk) < 0) return -1; /* * Start counting into how many hunks this one can be * split */ marker = *p; + } else if (hunk == &file_diff->head && + starts_with(p, "new file")) { + file_diff->added = 1; + } else if (hunk == &file_diff->head && + starts_with(p, "deleted file")) { + file_diff->deleted = 1; } else if (hunk == &file_diff->head && skip_prefix(p, "old mode ", &mode_change) && is_octal(mode_change, eol - mode_change)) { @@ -1358,39 +1355,46 @@ static int patch_update_file(struct add_p_state *s, int colored = !!s->colored.len, quit = 0; enum prompt_mode_type prompt_mode_type; - if (!file_diff->hunk_nr) + /* Empty added and deleted files have no hunks */ + if (!file_diff->hunk_nr && !file_diff->added && !file_diff->deleted) return 0; strbuf_reset(&s->buf); render_diff_header(s, file_diff, colored, &s->buf); fputs(s->buf.buf, stdout); for (;;) { - if (hunk_index >= file_diff->hunk_nr) - hunk_index = 0; - hunk = file_diff->hunk + hunk_index; + if (file_diff->hunk_nr) { + if (hunk_index >= file_diff->hunk_nr) + hunk_index = 0; + hunk = file_diff->hunk + hunk_index; - undecided_previous = -1; - for (i = hunk_index - 1; i >= 0; i--) - if (file_diff->hunk[i].use == UNDECIDED_HUNK) { - undecided_previous = i; - break; - } + undecided_previous = -1; + for (i = hunk_index - 1; i >= 0; i--) + if (file_diff->hunk[i].use == UNDECIDED_HUNK) { + undecided_previous = i; + break; + } - undecided_next = -1; - for (i = hunk_index + 1; i < file_diff->hunk_nr; i++) - if (file_diff->hunk[i].use == UNDECIDED_HUNK) { - undecided_next = i; - break; - } + undecided_next = -1; + for (i = hunk_index + 1; i < file_diff->hunk_nr; i++) + if (file_diff->hunk[i].use == UNDECIDED_HUNK) { + undecided_next = i; + break; + } - /* Everything decided? */ - if (undecided_previous < 0 && undecided_next < 0 && - hunk->use != UNDECIDED_HUNK) - break; + /* Everything decided? */ + if (undecided_previous < 0 && undecided_next < 0 && + hunk->use != UNDECIDED_HUNK) + break; - strbuf_reset(&s->buf); - render_hunk(s, hunk, 0, colored, &s->buf); - fputs(s->buf.buf, stdout); + strbuf_reset(&s->buf); + render_hunk(s, hunk, 0, colored, &s->buf); + fputs(s->buf.buf, stdout); + } else { + hunk = &file_diff->head; + undecided_next = -1; + undecided_previous = -1; + } strbuf_reset(&s->buf); if (undecided_previous >= 0) @@ -1421,7 +1425,9 @@ static int patch_update_file(struct add_p_state *s, color_fprintf(stdout, s->s.prompt_color, "(%"PRIuMAX"/%"PRIuMAX") ", (uintmax_t)hunk_index + 1, - (uintmax_t)file_diff->hunk_nr); + (uintmax_t)(file_diff->hunk_nr + ? file_diff->hunk_nr + : 1)); color_fprintf(stdout, s->s.prompt_color, _(s->mode->prompt_mode[prompt_mode_type]), s->buf.buf); @@ -1601,14 +1607,17 @@ static int patch_update_file(struct add_p_state *s, "%.*s", (int)(eol - p), p); } } + if (!file_diff->hunk_nr) + break; } /* Any hunk to be used? */ for (i = 0; i < file_diff->hunk_nr; i++) if (file_diff->hunk[i].use == USE_HUNK) break; - if (i < file_diff->hunk_nr) { + if (i < file_diff->hunk_nr || + (!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) { /* At least one hunk selected: apply */ strbuf_reset(&s->buf); reassemble_patch(s, file_diff, 0, &s->buf); diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index fb73a847cb..49d597979a 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -47,7 +47,11 @@ test_expect_success 'setup (initial)' ' echo content >file && git add file && echo more >>file && - echo lines >>file + echo lines >>file && + if test -n "$GIT_TEST_ADD_I_USE_BUILTIN" + then + test_set_prereq BUILTIN_ADD_I + fi ' test_expect_success 'status works (initial)' ' git add -i </dev/null >output && @@ -814,6 +818,44 @@ test_expect_success 'checkout -p works with pathological context lines' ' test_cmp expect a ' +# This should be called from a subshell as it sets a temporary editor +setup_new_file() { + write_script new-file-editor.sh <<-\EOF && + sed /^#/d "$1" >patch && + sed /^+c/d patch >"$1" + EOF + test_set_editor "$(pwd)/new-file-editor.sh" && + test_write_lines a b c d e f >new-file && + test_write_lines a b d e f >new-file-expect && + test_write_lines "@@ -0,0 +1,6 @@" +a +b +c +d +e +f >patch-expect +} + +test_expect_success BUILTIN_ADD_I 'add -N followed by add -p patch editing' ' + git reset --hard && + ( + setup_new_file && + git add -N new-file && + test_write_lines e n q | git add -p && + git cat-file blob :new-file >actual && + test_cmp new-file-expect actual && + test_cmp patch-expect patch + ) +' + +test_expect_success BUILTIN_ADD_I 'checkout -p patch editing of added file' ' + git reset --hard && + ( + setup_new_file && + git add new-file && + git commit -m "add new file" && + git rm new-file && + git commit -m "remove new file" && + test_write_lines e n q | git checkout -p HEAD^ && + test_cmp new-file-expect new-file && + test_cmp patch-expect patch + ) +' + test_expect_success 'show help from add--helper' ' git reset --hard && cat >expect <<-EOF && -- 2.25.1.551.gd3318bf0d3.dirty ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-09-04 10:05 ` Phillip Wood @ 2020-09-04 6:56 ` Johannes Schindelin 2020-09-06 17:10 ` Junio C Hamano 2020-09-05 18:36 ` Raymond E. Pasco 1 sibling, 1 reply; 16+ messages in thread From: Johannes Schindelin @ 2020-09-04 6:56 UTC (permalink / raw) To: Phillip Wood Cc: Raymond E. Pasco, phillip.wood, Junio C Hamano, Thomas Sullivan, git Hi Phillip, On Fri, 4 Sep 2020, Phillip Wood wrote: > [...] I haven't looked at fixing the perl version yet - dscho what are > your plans for switching over to the C version? Thanks for reminding me, I did not really think about it anymore. The built-in `git add -i`/`git add -p` has been available since v2.25.0. Since v2.26.0, we also respect that flag in the `-p` modes of `checkout`, `stash`, etc And from the way at least _I_ read the commit log, it seems that the code has been pretty stable (except for that bug fix where `e` was allowed by mistake). The next step will be to invert the default (which is `false` right now) for `add.interactive.useBuiltin`, I guess. Ciao, Dscho ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-09-04 6:56 ` Johannes Schindelin @ 2020-09-06 17:10 ` Junio C Hamano 2020-09-07 18:00 ` Phillip Wood 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2020-09-06 17:10 UTC (permalink / raw) To: Johannes Schindelin Cc: Phillip Wood, Raymond E. Pasco, phillip.wood, Thomas Sullivan, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Thanks for reminding me, I did not really think about it anymore. The > built-in `git add -i`/`git add -p` has been available since v2.25.0. Since > v2.26.0, we also respect that flag in the `-p` modes of `checkout`, > `stash`, etc That is 9a5315ed (Merge branch 'js/patch-mode-in-others-in-c', 2020-02-05) > And from the way at least _I_ read the commit log, it seems that the code > has been pretty stable (except for that bug fix where `e` was allowed by > mistake). As long as it has been widely used, that is. I do not think we deeply mind a bug like the `e` one that does not affect the utility or the correctness of the command that much. If we do not flip the "use the built-in variant" for those with feature.experimental we really should do so to widen the canarying population immediately. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-09-06 17:10 ` Junio C Hamano @ 2020-09-07 18:00 ` Phillip Wood 2020-09-08 16:05 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Phillip Wood @ 2020-09-07 18:00 UTC (permalink / raw) To: Junio C Hamano, Johannes Schindelin Cc: Raymond E. Pasco, phillip.wood, Thomas Sullivan, git On 06/09/2020 18:10, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > >> Thanks for reminding me, I did not really think about it anymore. The >> built-in `git add -i`/`git add -p` has been available since v2.25.0. Since >> v2.26.0, we also respect that flag in the `-p` modes of `checkout`, >> `stash`, etc > > That is 9a5315ed (Merge branch 'js/patch-mode-in-others-in-c', 2020-02-05) > >> And from the way at least _I_ read the commit log, it seems that the code >> has been pretty stable (except for that bug fix where `e` was allowed by >> mistake). > > As long as it has been widely used, that is. Exactly, I'm not sure it has been that widely used yet. (I'm interested in this area and only got round to using the C version a couple of weeks ago so I wonder how many others have) > I do not think we > deeply mind a bug like the `e` one that does not affect the utility > or the correctness of the command that much. The bug with editing in this thread is also fairly minor I think. I suspect the main area for serious bugs is hunk splitting and joining. > If we do not flip the > "use the built-in variant" for those with feature.experimental we > really should do so to widen the canarying population immediately. That's a good idea Best Wishes Phillip ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-09-07 18:00 ` Phillip Wood @ 2020-09-08 16:05 ` Junio C Hamano 2020-09-08 19:52 ` Johannes Schindelin 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2020-09-08 16:05 UTC (permalink / raw) To: Phillip Wood Cc: Johannes Schindelin, Raymond E. Pasco, phillip.wood, Thomas Sullivan, git Phillip Wood <phillip.wood123@gmail.com> writes: >> If we do not flip the >> "use the built-in variant" for those with feature.experimental we >> really should do so to widen the canarying population immediately. > > That's a good idea Like this? If the more specific one is specifically set, we do not look at experimental bit, but otherwise we use the built-in version. builtin/add.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index b36a99eb7c..26b6ced09e 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -192,9 +192,15 @@ int run_add_interactive(const char *revision, const char *patch_mode, int use_builtin_add_i = git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1); - if (use_builtin_add_i < 0) - git_config_get_bool("add.interactive.usebuiltin", - &use_builtin_add_i); + if (use_builtin_add_i < 0) { + int experimental; + if (!git_config_get_bool("add.interactive.usebuiltin", + &use_builtin_add_i)) + ; /* ok */ + else if (!git_config_get_bool("feature.experimental", &experimental) && + experimental) + use_builtin_add_i = 1; + } if (use_builtin_add_i == 1) { enum add_p_mode mode; ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-09-08 16:05 ` Junio C Hamano @ 2020-09-08 19:52 ` Johannes Schindelin 2020-09-08 22:00 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Johannes Schindelin @ 2020-09-08 19:52 UTC (permalink / raw) To: Junio C Hamano Cc: Phillip Wood, Raymond E. Pasco, phillip.wood, Thomas Sullivan, git Hi Junio, On Tue, 8 Sep 2020, Junio C Hamano wrote: > Phillip Wood <phillip.wood123@gmail.com> writes: > > >> If we do not flip the > >> "use the built-in variant" for those with feature.experimental we > >> really should do so to widen the canarying population immediately. > > > > That's a good idea > > Like this? If the more specific one is specifically set, we do not > look at experimental bit, but otherwise we use the built-in version. Looks fine to me, Dscho > > builtin/add.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/builtin/add.c b/builtin/add.c > index b36a99eb7c..26b6ced09e 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -192,9 +192,15 @@ int run_add_interactive(const char *revision, const char *patch_mode, > int use_builtin_add_i = > git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1); > > - if (use_builtin_add_i < 0) > - git_config_get_bool("add.interactive.usebuiltin", > - &use_builtin_add_i); > + if (use_builtin_add_i < 0) { > + int experimental; > + if (!git_config_get_bool("add.interactive.usebuiltin", > + &use_builtin_add_i)) > + ; /* ok */ > + else if (!git_config_get_bool("feature.experimental", &experimental) && > + experimental) > + use_builtin_add_i = 1; > + } > > if (use_builtin_add_i == 1) { > enum add_p_mode mode; > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-09-08 19:52 ` Johannes Schindelin @ 2020-09-08 22:00 ` Junio C Hamano 2020-09-09 9:40 ` Phillip Wood 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2020-09-08 22:00 UTC (permalink / raw) To: Johannes Schindelin Cc: Phillip Wood, Raymond E. Pasco, phillip.wood, Thomas Sullivan, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> Like this? If the more specific one is specifically set, we do not >> look at experimental bit, but otherwise we use the built-in version. > > Looks fine to me, > Dscho Thanks, with the proposed log message this time... -- >8 -- Subject: [PATCH] add -i: use the built-in version when feature.experimental is set We have had parallel implementations of "add -i/-p" since 2.25 and have been using them from various codepaths since 2.26 days, but never made the built-in version the default. We have found and fixed a handful of corner case bugs in the built-in version, and it may be a good time to start switching over the user base from the scripted version to the built-in version. Let's enable the built-in version for those who opt into the feature.experimental guinea-pig program to give wider exposure. Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/add.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index b36a99eb7c..26b6ced09e 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -192,9 +192,15 @@ int run_add_interactive(const char *revision, const char *patch_mode, int use_builtin_add_i = git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1); - if (use_builtin_add_i < 0) - git_config_get_bool("add.interactive.usebuiltin", - &use_builtin_add_i); + if (use_builtin_add_i < 0) { + int experimental; + if (!git_config_get_bool("add.interactive.usebuiltin", + &use_builtin_add_i)) + ; /* ok */ + else if (!git_config_get_bool("feature.experimental", &experimental) && + experimental) + use_builtin_add_i = 1; + } if (use_builtin_add_i == 1) { enum add_p_mode mode; -- 2.28.0-539-g66371d8995 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-09-08 22:00 ` Junio C Hamano @ 2020-09-09 9:40 ` Phillip Wood 0 siblings, 0 replies; 16+ messages in thread From: Phillip Wood @ 2020-09-09 9:40 UTC (permalink / raw) To: Junio C Hamano, Johannes Schindelin Cc: Raymond E. Pasco, phillip.wood, Thomas Sullivan, git On 08/09/2020 23:00, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > >>> Like this? If the more specific one is specifically set, we do not >>> look at experimental bit, but otherwise we use the built-in version. >> >> Looks fine to me, >> Dscho > > Thanks, with the proposed log message this time... > > -- >8 -- > Subject: [PATCH] add -i: use the built-in version when feature.experimental is set > > We have had parallel implementations of "add -i/-p" since 2.25 and > have been using them from various codepaths since 2.26 days, but > never made the built-in version the default. > > We have found and fixed a handful of corner case bugs in the > built-in version, and it may be a good time to start switching over > the user base from the scripted version to the built-in version. > Let's enable the built-in version for those who opt into the > feature.experimental guinea-pig program to give wider exposure. > > Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > builtin/add.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/builtin/add.c b/builtin/add.c > index b36a99eb7c..26b6ced09e 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -192,9 +192,15 @@ int run_add_interactive(const char *revision, const char *patch_mode, > int use_builtin_add_i = > git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1); > > - if (use_builtin_add_i < 0) > - git_config_get_bool("add.interactive.usebuiltin", > - &use_builtin_add_i); > + if (use_builtin_add_i < 0) { > + int experimental; > + if (!git_config_get_bool("add.interactive.usebuiltin", > + &use_builtin_add_i)) > + ; /* ok */ > + else if (!git_config_get_bool("feature.experimental", &experimental) && > + experimental) > + use_builtin_add_i = 1; > + } > > if (use_builtin_add_i == 1) { > enum add_p_mode mode; > Looks good to me as well, Thanks Phillip ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git add intent-to-add then git add patch no longer allows edit 2020-09-04 10:05 ` Phillip Wood 2020-09-04 6:56 ` Johannes Schindelin @ 2020-09-05 18:36 ` Raymond E. Pasco 1 sibling, 0 replies; 16+ messages in thread From: Raymond E. Pasco @ 2020-09-05 18:36 UTC (permalink / raw) To: Phillip Wood, phillip.wood, Junio C Hamano Cc: Thomas Sullivan, git, Johannes Schindelin On Fri Sep 4, 2020 at 6:05 AM EDT, Phillip Wood wrote: > Thanks, it was really good to get a change of scene. The patch below > fixes the hunk editing for new files in the C version of `add -p` if > anyone wants to try it out. I haven't looked at fixing the perl > version yet - dscho what are your plans for switching over to the C > version? No issues with this patch thus far - I'll continue running with it until it gets picked up. Thanks! ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-09-09 9:40 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-21 4:27 git add intent-to-add then git add patch no longer allows edit Thomas Sullivan 2020-08-21 5:25 ` Raymond E. Pasco 2020-08-21 16:27 ` Junio C Hamano 2020-08-23 16:03 ` Phillip Wood 2020-08-24 16:23 ` Raymond E. Pasco 2020-08-24 17:28 ` Phillip Wood 2020-08-24 21:03 ` Raymond E. Pasco 2020-09-04 10:05 ` Phillip Wood 2020-09-04 6:56 ` Johannes Schindelin 2020-09-06 17:10 ` Junio C Hamano 2020-09-07 18:00 ` Phillip Wood 2020-09-08 16:05 ` Junio C Hamano 2020-09-08 19:52 ` Johannes Schindelin 2020-09-08 22:00 ` Junio C Hamano 2020-09-09 9:40 ` Phillip Wood 2020-09-05 18:36 ` Raymond E. Pasco
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).