revert: clarify label on conflict hunks
When reverting a commit, the commit being merged is not the commit to revert itself but its parent. Add “parent of” to the conflict hunk label to make this more clear. The conflict hunk labels are all pieces of a single string written in the new get_message() function. Avoid some complication by using mempcpy to advance a pointer as the result is written. Also free the corresponding temporary buffer (it was leaked before). This is not important because it is a small one-time allocation. It would become a memory leak if unnoticed when libifying revert. This patch uses calls to strlen() instead of integer constants in some places. GCC will compute the length at compile time; I am not sure about other compilers, but this is not performance-critical anyway. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Nieder committed
Mar 20, 2010 at 19:45 UTC
d68565402a69fac3fe0e0653718feca7c80c178b
2 files changed
+64
-41
builtin/revert.c
+62
-39
@@ -45,6 +45,8 @@ static const char *me;
45
46
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
47
48
+static char *get_encoding(const char *message);
49
+
50
static void parse_args(int argc, const char **argv)
51
{
52
const char * const * usage_str =
@@ -73,33 +75,64 @@ static void parse_args(int argc, const char **argv)
75
exit(1);
76
}
77
76
-static char *get_oneline(const char *message)
78
+struct commit_message {
79
+ char *parent_label;
80
+ const char *label;
81
+ const char *subject;
82
+ char *reencoded_message;
83
+ const char *message;
84
+};
85
+
86
+static int get_message(const char *raw_message, struct commit_message *out)
87
{
78
- char *result;
79
- const char *p = message, *abbrev, *eol;
88
+ const char *encoding;
89
+ const char *p, *abbrev, *eol;
90
+ char *q;
91
int abbrev_len, oneline_len;
92
82
- if (!p)
83
- die ("Could not read commit message of %s",
84
- sha1_to_hex(commit->object.sha1));
93
+ if (!raw_message)
94
+ return -1;
95
+ encoding = get_encoding(raw_message);
96
+ if (!encoding)
97
+ encoding = "UTF-8";
98
+ if (!git_commit_encoding)
99
+ git_commit_encoding = "UTF-8";
100
+ if ((out->reencoded_message = reencode_string(raw_message,
101
+ git_commit_encoding, encoding)))
102
+ out->message = out->reencoded_message;
103
+
104
+ abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
105
+ abbrev_len = strlen(abbrev);
106
+
107
+ /* Find beginning and end of commit subject. */
108
+ p = out->message;
109
while (*p && (*p != '\n' || p[1] != '\n'))
110
p++;
87
-
111
if (*p) {
112
p += 2;
113
for (eol = p + 1; *eol && *eol != '\n'; eol++)
114
; /* do nothing */
115
} else
116
eol = p;
94
- abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
95
- abbrev_len = strlen(abbrev);
117
oneline_len = eol - p;
97
- result = xmalloc(abbrev_len + 5 + oneline_len);
98
- memcpy(result, abbrev, abbrev_len);
99
- memcpy(result + abbrev_len, "... ", 4);
100
- memcpy(result + abbrev_len + 4, p, oneline_len);
101
- result[abbrev_len + 4 + oneline_len] = '\0';
102
- return result;
118
+
119
+ out->parent_label = xmalloc(strlen("parent of ") + abbrev_len +
120
+ strlen("... ") + oneline_len + 1);
121
+ q = out->parent_label;
122
+ q = mempcpy(q, "parent of ", strlen("parent of "));
123
+ out->label = q;
124
+ q = mempcpy(q, abbrev, abbrev_len);
125
+ q = mempcpy(q, "... ", strlen("... "));
126
+ out->subject = q;
127
+ q = mempcpy(q, p, oneline_len);
128
+ *q = '\0';
129
+ return 0;
130
+}
131
+
132
+static void free_message(struct commit_message *msg)
133
+{
134
+ free(msg->parent_label);
135
+ free(msg->reencoded_message);
136
}
137
138
static char *get_encoding(const char *message)
@@ -248,9 +281,10 @@ static int revert_or_cherry_pick(int argc, const char **argv)
281
{
282
unsigned char head[20];
283
struct commit *base, *next, *parent;
284
+ const char *next_label;
285
int i, index_fd, clean;
252
- char *oneline, *reencoded_message = NULL;
253
- const char *message, *encoding;
286
+ struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
287
+
288
char *defmsg = git_pathdup("MERGE_MSG");
289
struct merge_options o;
290
struct tree *result, *next_tree, *base_tree, *head_tree;
@@ -314,14 +348,14 @@ static int revert_or_cherry_pick(int argc, const char **argv)
348
else
349
parent = commit->parents->item;
350
317
- if (!(message = commit->buffer))
318
- die ("Cannot get commit message for %s",
319
- sha1_to_hex(commit->object.sha1));
320
-
351
if (parent && parse_commit(parent) < 0)
352
die("%s: cannot parse parent commit %s",
353
me, sha1_to_hex(parent->object.sha1));
354
355
+ if (get_message(commit->buffer, &msg) != 0)
356
+ die("Cannot get commit message for %s",
357
+ sha1_to_hex(commit->object.sha1));
358
+
359
/*
360
* "commit" is an existing commit. We would want to apply
361
* the difference it introduces since its first parent "prev"
@@ -332,24 +366,12 @@ static int revert_or_cherry_pick(int argc, const char **argv)
366
msg_fd = hold_lock_file_for_update(&msg_file, defmsg,
367
LOCK_DIE_ON_ERROR);
368
335
- encoding = get_encoding(message);
336
- if (!encoding)
337
- encoding = "UTF-8";
338
- if (!git_commit_encoding)
339
- git_commit_encoding = "UTF-8";
340
- if ((reencoded_message = reencode_string(message,
341
- git_commit_encoding, encoding)))
342
- message = reencoded_message;
343
-
344
- oneline = get_oneline(message);
345
-
369
if (action == REVERT) {
347
- char *oneline_body = strchr(oneline, ' ');
348
-
370
base = commit;
371
next = parent;
372
+ next_label = msg.parent_label;
373
add_to_msg("Revert \"");
352
- add_to_msg(oneline_body + 1);
374
+ add_to_msg(msg.subject);
375
add_to_msg("\"\n\nThis reverts commit ");
376
add_to_msg(sha1_to_hex(commit->object.sha1));
377
@@ -361,8 +383,9 @@ static int revert_or_cherry_pick(int argc, const char **argv)
383
} else {
384
base = parent;
385
next = commit;
364
- set_author_ident_env(message);
365
- add_message_to_msg(message);
386
+ next_label = msg.label;
387
+ set_author_ident_env(msg.message);
388
+ add_message_to_msg(msg.message);
389
if (no_replay) {
390
add_to_msg("(cherry picked from commit ");
391
add_to_msg(sha1_to_hex(commit->object.sha1));
@@ -373,7 +396,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
396
read_cache();
397
init_merge_options(&o);
398
o.branch1 = "HEAD";
376
- o.branch2 = oneline;
399
+ o.branch2 = next ? next_label : "(empty tree)";
400
401
head_tree = parse_tree_indirect(head);
402
next_tree = next ? next->tree : empty_tree();
@@ -437,7 +460,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
460
args[i] = NULL;
461
return execv_git_cmd(args);
462
}
440
- free(reencoded_message);
463
+ free_message(&msg);
464
free(defmsg);
465
466
return 0;
t/t3507-cherry-pick-conflict.sh
+2
-2
@@ -138,7 +138,7 @@ test_expect_success 'revert also handles conflicts sanely' '
138
a
139
=======
140
b
141
- >>>>>>> objid picked
141
+ >>>>>>> parent of objid picked
142
EOF
143
{
144
git checkout picked -- foo &&
@@ -183,7 +183,7 @@ test_expect_success 'revert conflict, diff3 -m style' '
183
c
184
=======
185
b
186
- >>>>>>> objid picked
186
+ >>>>>>> parent of objid picked
187
EOF
188
189
git update-index --refresh &&