built-in add -p: implement hunk editing
Just like `git add --edit` allows the user to edit the diff before it is being applied to the index, this feature allows the user to edit the diff *hunk*. Naturally, it gets a bit more complicated here because the result has to play well with the remaining hunks of the overall diff. Therefore, we have to do a loop in which we let the user edit the hunk, then test whether the result would work, and if not, drop the edits and let the user decide whether to try editing the hunk again. Note: in contrast to the Perl version, we use the same diff "coalescing" (i.e. merging overlapping hunks into a single one) also for the check after editing, and we introduce a new flag for that purpose that asks the `reassemble_patch()` function to pretend that all hunks were selected for use. This allows us to continue to run `git apply` *without* the `--allow-overlap` option (unlike the Perl version), and it also fixes two known breakages in `t3701-add-interactive.sh` (which we cannot mark as resolved so far because the Perl script version is still the default and continues to have those breakages). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin committed
Dec 13, 2019 at 08:08 UTC
bcdd297b782b19782d8e09b18682ec61d7c36897
3 files changed
+325
-17
add-interactive.c
+6
@@ -46,6 +46,12 @@ void init_add_i_state(struct add_i_state *s, struct repository *r)
46
init_color(r, s, "reset", s->reset_color, GIT_COLOR_RESET);
47
init_color(r, s, "fraginfo", s->fraginfo_color,
48
diff_get_color(s->use_color, DIFF_FRAGINFO));
49
+ init_color(r, s, "context", s->context_color,
50
+ diff_get_color(s->use_color, DIFF_CONTEXT));
51
+ init_color(r, s, "old", s->file_old_color,
52
+ diff_get_color(s->use_color, DIFF_FILE_OLD));
53
+ init_color(r, s, "new", s->file_new_color,
54
+ diff_get_color(s->use_color, DIFF_FILE_NEW));
55
}
56
57
/*
add-interactive.h
+3
@@ -12,6 +12,9 @@ struct add_i_state {
12
char error_color[COLOR_MAXLEN];
13
char reset_color[COLOR_MAXLEN];
14
char fraginfo_color[COLOR_MAXLEN];
15
+ char context_color[COLOR_MAXLEN];
16
+ char file_old_color[COLOR_MAXLEN];
17
+ char file_new_color[COLOR_MAXLEN];
18
};
19
20
void init_add_i_state(struct add_i_state *s, struct repository *r);
add-patch.c
+316
-17
@@ -29,6 +29,7 @@ struct hunk_header {
29
30
struct hunk {
31
size_t start, end, colored_start, colored_end, splittable_into;
32
+ ssize_t delta;
33
enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use;
34
struct hunk_header header;
35
};
@@ -435,14 +436,14 @@ static void render_diff_header(struct add_p_state *s,
436
437
/* Coalesce hunks again that were split */
438
static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,
438
- size_t *hunk_index, struct hunk *merged)
439
+ size_t *hunk_index, int use_all, struct hunk *merged)
440
{
440
- size_t i = *hunk_index;
441
+ size_t i = *hunk_index, delta;
442
struct hunk *hunk = file_diff->hunk + i;
443
/* `header` corresponds to the merged hunk */
444
struct hunk_header *header = &merged->header, *next;
445
445
- if (hunk->use != USE_HUNK)
446
+ if (!use_all && hunk->use != USE_HUNK)
447
return 0;
448
449
*merged = *hunk;
@@ -459,20 +460,99 @@ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,
460
* - the hunk is not selected for use, or
461
* - the hunk does not overlap with the already-merged hunk(s)
462
*/
462
- if (hunk->use != USE_HUNK ||
463
- header->new_offset >= next->new_offset ||
464
- header->new_offset + header->new_count < next->new_offset ||
465
- merged->start >= hunk->start ||
466
- merged->end < hunk->start)
463
+ if ((!use_all && hunk->use != USE_HUNK) ||
464
+ header->new_offset >= next->new_offset + merged->delta ||
465
+ header->new_offset + header->new_count
466
+ < next->new_offset + merged->delta)
467
break;
468
469
- merged->end = hunk->end;
470
- merged->colored_end = hunk->colored_end;
469
+ /*
470
+ * If the hunks were not edited, and overlap, we can simply
471
+ * extend the line range.
472
+ */
473
+ if (merged->start < hunk->start && merged->end > hunk->start) {
474
+ merged->end = hunk->end;
475
+ merged->colored_end = hunk->colored_end;
476
+ delta = 0;
477
+ } else {
478
+ const char *plain = s->plain.buf;
479
+ size_t overlapping_line_count = header->new_offset
480
+ + header->new_count - merged->delta
481
+ - next->new_offset;
482
+ size_t overlap_end = hunk->start;
483
+ size_t overlap_start = overlap_end;
484
+ size_t overlap_next, len, j;
485
+
486
+ /*
487
+ * One of the hunks was edited: the modified hunk was
488
+ * appended to the strbuf `s->plain`.
489
+ *
490
+ * Let's ensure that at least the last context line of
491
+ * the first hunk overlaps with the corresponding line
492
+ * of the second hunk, and then merge.
493
+ */
494
+ for (j = 0; j < overlapping_line_count; j++) {
495
+ overlap_next = find_next_line(&s->plain,
496
+ overlap_end);
497
+
498
+ if (overlap_next > hunk->end)
499
+ BUG("failed to find %d context lines "
500
+ "in:\n%.*s",
501
+ (int)overlapping_line_count,
502
+ (int)(hunk->end - hunk->start),
503
+ plain + hunk->start);
504
+
505
+ if (plain[overlap_end] != ' ')
506
+ return error(_("expected context line "
507
+ "#%d in\n%.*s"),
508
+ (int)(j + 1),
509
+ (int)(hunk->end
510
+ - hunk->start),
511
+ plain + hunk->start);
512
+
513
+ overlap_start = overlap_end;
514
+ overlap_end = overlap_next;
515
+ }
516
+ len = overlap_end - overlap_start;
517
+
518
+ if (len > merged->end - merged->start ||
519
+ memcmp(plain + merged->end - len,
520
+ plain + overlap_start, len))
521
+ return error(_("hunks do not overlap:\n%.*s\n"
522
+ "\tdoes not end with:\n%.*s"),
523
+ (int)(merged->end - merged->start),
524
+ plain + merged->start,
525
+ (int)len, plain + overlap_start);
526
+
527
+ /*
528
+ * Since the start-end ranges are not adjacent, we
529
+ * cannot simply take the union of the ranges. To
530
+ * address that, we temporarily append the union of the
531
+ * lines to the `plain` strbuf.
532
+ */
533
+ if (merged->end != s->plain.len) {
534
+ size_t start = s->plain.len;
535
+
536
+ strbuf_add(&s->plain, plain + merged->start,
537
+ merged->end - merged->start);
538
+ plain = s->plain.buf;
539
+ merged->start = start;
540
+ merged->end = s->plain.len;
541
+ }
542
+
543
+ strbuf_add(&s->plain,
544
+ plain + overlap_end,
545
+ hunk->end - overlap_end);
546
+ merged->end = s->plain.len;
547
+ merged->splittable_into += hunk->splittable_into;
548
+ delta = merged->delta;
549
+ merged->delta += hunk->delta;
550
+ }
551
552
header->old_count = next->old_offset + next->old_count
553
- header->old_offset;
474
- header->new_count = next->new_offset + next->new_count
475
- - header->new_offset;
554
+ header->new_count = next->new_offset + delta
555
+ + next->new_count - header->new_offset;
556
}
557
558
if (i == *hunk_index)
@@ -483,10 +563,11 @@ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,
563
}
564
565
static void reassemble_patch(struct add_p_state *s,
486
- struct file_diff *file_diff, struct strbuf *out)
566
+ struct file_diff *file_diff, int use_all,
567
+ struct strbuf *out)
568
{
569
struct hunk *hunk;
489
- size_t i;
570
+ size_t save_len = s->plain.len, i;
571
ssize_t delta = 0;
572
573
render_diff_header(s, file_diff, 0, out);
@@ -495,15 +576,24 @@ static void reassemble_patch(struct add_p_state *s,
576
struct hunk merged = { 0 };
577
578
hunk = file_diff->hunk + i;
498
- if (hunk->use != USE_HUNK)
579
+ if (!use_all && hunk->use != USE_HUNK)
580
delta += hunk->header.old_count
581
- hunk->header.new_count;
582
else {
583
/* merge overlapping hunks into a temporary hunk */
503
- if (merge_hunks(s, file_diff, &i, &merged))
584
+ if (merge_hunks(s, file_diff, &i, use_all, &merged))
585
hunk = &merged;
586
587
render_hunk(s, hunk, delta, 0, out);
588
+
589
+ /*
590
+ * In case `merge_hunks()` used `plain` as a scratch
591
+ * pad (this happens when an edited hunk had to be
592
+ * coalesced with another hunk).
593
+ */
594
+ strbuf_setlen(&s->plain, save_len);
595
+
596
+ delta += hunk->delta;
597
}
598
}
599
}
@@ -667,6 +757,204 @@ next_hunk_line:
757
return 0;
758
}
759
760
+static void recolor_hunk(struct add_p_state *s, struct hunk *hunk)
761
+{
762
+ const char *plain = s->plain.buf;
763
+ size_t current, eol, next;
764
+
765
+ if (!s->colored.len)
766
+ return;
767
+
768
+ hunk->colored_start = s->colored.len;
769
+ for (current = hunk->start; current < hunk->end; ) {
770
+ for (eol = current; eol < hunk->end; eol++)
771
+ if (plain[eol] == '\n')
772
+ break;
773
+ next = eol + (eol < hunk->end);
774
+ if (eol > current && plain[eol - 1] == '\r')
775
+ eol--;
776
+
777
+ strbuf_addstr(&s->colored,
778
+ plain[current] == '-' ?
779
+ s->s.file_old_color :
780
+ plain[current] == '+' ?
781
+ s->s.file_new_color :
782
+ s->s.context_color);
783
+ strbuf_add(&s->colored, plain + current, eol - current);
784
+ strbuf_addstr(&s->colored, GIT_COLOR_RESET);
785
+ if (next > eol)
786
+ strbuf_add(&s->colored, plain + eol, next - eol);
787
+ current = next;
788
+ }
789
+ hunk->colored_end = s->colored.len;
790
+}
791
+
792
+static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
793
+{
794
+ size_t i;
795
+
796
+ strbuf_reset(&s->buf);
797
+ strbuf_commented_addf(&s->buf, _("Manual hunk edit mode -- see bottom for "
798
+ "a quick guide.\n"));
799
+ render_hunk(s, hunk, 0, 0, &s->buf);
800
+ strbuf_commented_addf(&s->buf,
801
+ _("---\n"
802
+ "To remove '%c' lines, make them ' ' lines "
803
+ "(context).\n"
804
+ "To remove '%c' lines, delete them.\n"
805
+ "Lines starting with %c will be removed.\n"),
806
+ '-', '+', comment_line_char);
807
+ strbuf_commented_addf(&s->buf,
808
+ _("If the patch applies cleanly, the edited hunk "
809
+ "will immediately be\n"
810
+ "marked for staging.\n"));
811
+ /*
812
+ * TRANSLATORS: 'it' refers to the patch mentioned in the previous
813
+ * messages.
814
+ */
815
+ strbuf_commented_addf(&s->buf,
816
+ _("If it does not apply cleanly, you will be "
817
+ "given an opportunity to\n"
818
+ "edit again. If all lines of the hunk are "
819
+ "removed, then the edit is\n"
820
+ "aborted and the hunk is left unchanged.\n"));
821
+
822
+ if (strbuf_edit_interactively(&s->buf, "addp-hunk-edit.diff", NULL) < 0)
823
+ return -1;
824
+
825
+ /* strip out commented lines */
826
+ hunk->start = s->plain.len;
827
+ for (i = 0; i < s->buf.len; ) {
828
+ size_t next = find_next_line(&s->buf, i);
829
+
830
+ if (s->buf.buf[i] != comment_line_char)
831
+ strbuf_add(&s->plain, s->buf.buf + i, next - i);
832
+ i = next;
833
+ }
834
+
835
+ hunk->end = s->plain.len;
836
+ if (hunk->end == hunk->start)
837
+ /* The user aborted editing by deleting everything */
838
+ return 0;
839
+
840
+ recolor_hunk(s, hunk);
841
+
842
+ /*
843
+ * If the hunk header is intact, parse it, otherwise simply use the
844
+ * hunk header prior to editing (which will adjust `hunk->start` to
845
+ * skip the hunk header).
846
+ */
847
+ if (s->plain.buf[hunk->start] == '@' &&
848
+ parse_hunk_header(s, hunk) < 0)
849
+ return error(_("could not parse hunk header"));
850
+
851
+ return 1;
852
+}
853
+
854
+static ssize_t recount_edited_hunk(struct add_p_state *s, struct hunk *hunk,
855
+ size_t orig_old_count, size_t orig_new_count)
856
+{
857
+ struct hunk_header *header = &hunk->header;
858
+ size_t i;
859
+
860
+ header->old_count = header->new_count = 0;
861
+ for (i = hunk->start; i < hunk->end; ) {
862
+ switch (s->plain.buf[i]) {
863
+ case '-':
864
+ header->old_count++;
865
+ break;
866
+ case '+':
867
+ header->new_count++;
868
+ break;
869
+ case ' ': case '\r': case '\n':
870
+ header->old_count++;
871
+ header->new_count++;
872
+ break;
873
+ }
874
+
875
+ i = find_next_line(&s->plain, i);
876
+ }
877
+
878
+ return orig_old_count - orig_new_count
879
+ - header->old_count + header->new_count;
880
+}
881
+
882
+static int run_apply_check(struct add_p_state *s,
883
+ struct file_diff *file_diff)
884
+{
885
+ struct child_process cp = CHILD_PROCESS_INIT;
886
+
887
+ strbuf_reset(&s->buf);
888
+ reassemble_patch(s, file_diff, 1, &s->buf);
889
+
890
+ setup_child_process(s, &cp,
891
+ "apply", "--cached", "--check", NULL);
892
+ if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0))
893
+ return error(_("'git apply --cached' failed"));
894
+
895
+ return 0;
896
+}
897
+
898
+static int prompt_yesno(struct add_p_state *s, const char *prompt)
899
+{
900
+ for (;;) {
901
+ color_fprintf(stdout, s->s.prompt_color, "%s", _(prompt));
902
+ fflush(stdout);
903
+ if (strbuf_getline(&s->answer, stdin) == EOF)
904
+ return -1;
905
+ strbuf_trim_trailing_newline(&s->answer);
906
+ switch (tolower(s->answer.buf[0])) {
907
+ case 'n': return 0;
908
+ case 'y': return 1;
909
+ }
910
+ }
911
+}
912
+
913
+static int edit_hunk_loop(struct add_p_state *s,
914
+ struct file_diff *file_diff, struct hunk *hunk)
915
+{
916
+ size_t plain_len = s->plain.len, colored_len = s->colored.len;
917
+ struct hunk backup;
918
+
919
+ backup = *hunk;
920
+
921
+ for (;;) {
922
+ int res = edit_hunk_manually(s, hunk);
923
+ if (res == 0) {
924
+ /* abandonded */
925
+ *hunk = backup;
926
+ return -1;
927
+ }
928
+
929
+ if (res > 0) {
930
+ hunk->delta +=
931
+ recount_edited_hunk(s, hunk,
932
+ backup.header.old_count,
933
+ backup.header.new_count);
934
+ if (!run_apply_check(s, file_diff))
935
+ return 0;
936
+ }
937
+
938
+ /* Drop edits (they were appended to s->plain) */
939
+ strbuf_setlen(&s->plain, plain_len);
940
+ strbuf_setlen(&s->colored, colored_len);
941
+ *hunk = backup;
942
+
943
+ /*
944
+ * TRANSLATORS: do not translate [y/n]
945
+ * The program will only accept that input at this point.
946
+ * Consider translating (saying "no" discards!) as
947
+ * (saying "n" for "no" discards!) if the translation
948
+ * of the word "no" does not start with n.
949
+ */
950
+ res = prompt_yesno(s, _("Your edited hunk does not apply. "
951
+ "Edit again (saying \"no\" discards!) "
952
+ "[y/n]? "));
953
+ if (res < 1)
954
+ return -1;
955
+ }
956
+}
957
+
958
static const char help_patch_text[] =
959
N_("y - stage this hunk\n"
960
"n - do not stage this hunk\n"
@@ -677,6 +965,7 @@ N_("y - stage this hunk\n"
965
"k - leave this hunk undecided, see previous undecided hunk\n"
966
"K - leave this hunk undecided, see previous hunk\n"
967
"s - split the current hunk into smaller hunks\n"
968
+ "e - manually edit the current hunk\n"
969
"? - print help\n");
970
971
static int patch_update_file(struct add_p_state *s,
@@ -735,6 +1024,9 @@ static int patch_update_file(struct add_p_state *s,
1024
strbuf_addstr(&s->buf, ",J");
1025
if (hunk->splittable_into > 1)
1026
strbuf_addstr(&s->buf, ",s");
1027
+ if (hunk_index + 1 > file_diff->mode_change &&
1028
+ !file_diff->deleted)
1029
+ strbuf_addstr(&s->buf, ",e");
1030
1031
if (file_diff->deleted)
1032
prompt_mode_type = PROMPT_DELETION;
@@ -806,6 +1098,13 @@ soft_increment:
1098
color_fprintf_ln(stdout, s->s.header_color,
1099
_("Split into %d hunks."),
1100
(int)splittable_into);
1101
+ } else if (s->answer.buf[0] == 'e') {
1102
+ if (hunk_index + 1 == file_diff->mode_change)
1103
+ err(s, _("Sorry, cannot edit this hunk"));
1104
+ else if (edit_hunk_loop(s, file_diff, hunk) >= 0) {
1105
+ hunk->use = USE_HUNK;
1106
+ goto soft_increment;
1107
+ }
1108
} else
1109
color_fprintf(stdout, s->s.help_color,
1110
_(help_patch_text));
@@ -819,7 +1118,7 @@ soft_increment:
1118
if (i < file_diff->hunk_nr) {
1119
/* At least one hunk selected: apply */
1120
strbuf_reset(&s->buf);
822
- reassemble_patch(s, file_diff, &s->buf);
1121
+ reassemble_patch(s, file_diff, 0, &s->buf);
1122
1123
discard_index(s->s.r->index);
1124
setup_child_process(s, &cp, "apply", "--cached", NULL);