Skip to content

Commit 191faaf

Browse files
committed
revert: --reference should apply only to 'revert', not 'cherry-pick'
As 'revert' and 'cherry-pick' share a lot of code, it is easy to modify the behaviour of one command and inadvertently affect the other. An earlier change to teach the '--reference' option and the 'revert.reference' configuration variable to the former was not careful enough and 'cherry-pick --reference' wasn't rejected as an error. It is possible to think 'cherry-pick -x' might benefit from the '--reference' option, but it is fundamentally different from 'revert' in at least two ways to make it questionable: - 'revert' names a commit that is ancestor of the resulting commit, so an abbreviated object name with human readable title is sufficient to identify the named commit uniquely without using the full object name. On the other hand, 'cherry-pick' usually [*] picks a commit that is not an ancestor. It might be even picking a private commit that never becomes part of the public history. - The whole commit message of 'cherry-pick' is a copy of the original commit, and there is nothing gained to repeat only the title part on 'cherry-picked from' message. [*] well, you could revert and then you can pick the original that was reverted to get back to where you were, but then you can revert the revert to do the same thing. Helped-by: Phillip Wood <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 43966ab commit 191faaf

File tree

3 files changed

+14
-3
lines changed

3 files changed

+14
-3
lines changed

builtin/revert.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,6 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
116116
N_("option for merge strategy"), option_parse_x),
117117
{ OPTION_STRING, 'S', "gpg-sign", &opts->gpg_sign, N_("key-id"),
118118
N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
119-
OPT_BOOL(0, "reference", &opts->commit_use_reference,
120-
N_("use the 'reference' format to refer to commits")),
121119
OPT_END()
122120
};
123121
struct option *options = base_options;
@@ -132,6 +130,13 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
132130
OPT_END(),
133131
};
134132
options = parse_options_concat(options, cp_extra);
133+
} else if (opts->action == REPLAY_REVERT) {
134+
struct option cp_extra[] = {
135+
OPT_BOOL(0, "reference", &opts->commit_use_reference,
136+
N_("use the 'reference' format to refer to commits")),
137+
OPT_END(),
138+
};
139+
options = parse_options_concat(options, cp_extra);
135140
}
136141

137142
argc = parse_options(argc, argv, NULL, options, usage_str,

sequencer.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)
221221
return ret;
222222
}
223223

224-
if (!strcmp(k, "revert.reference"))
224+
if (opts->action == REPLAY_REVERT && !strcmp(k, "revert.reference"))
225225
opts->commit_use_reference = git_config_bool(k, v);
226226

227227
status = git_gpg_config(k, v, NULL);

t/t3501-revert-cherry-pick.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,4 +205,10 @@ test_expect_success 'identification of reverted commit (revert.reference)' '
205205
test_cmp expect actual
206206
'
207207

208+
test_expect_success 'cherry-pick is unaware of --reference (for now)' '
209+
test_when_finished "git reset --hard" &&
210+
test_must_fail git cherry-pick --reference HEAD 2>actual &&
211+
grep "^usage: git cherry-pick" actual
212+
'
213+
208214
test_done

0 commit comments

Comments
 (0)