Junio C Hamano <gitster@xxxxxxxxx> writes:
>> + OPT_END(),
>> OPT_END(),
>> OPT_END(),
>> OPT_END(),
>> @@ -106,6 +112,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
>> OPT_BOOL(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
>> OPT_BOOL(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
>> OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
>> + OPT_BOOL(0, "skip-redundant-commits", &opts->skip_redundant_commits, N_("skip redundant, empty commits")),
>> OPT_END(),
>> };
>
> This however makes me wonder what should happen when both are
> specified. Shouldn't this patch change the keep_redundant_commits
> field from a bool to a tristate that tells us what to do with
> redundant ones? int/enum opts.redundant_commit can take 0 (fail,
> which would be the default), 1 (keep) or 2 (skip), or something
> like that.
This makes good sense.
>> diff --git a/sequencer.c b/sequencer.c
>> index 8c58fa2..12361e7 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -185,6 +185,7 @@ static void print_advice(int show_hint, struct replay_opts *opts)
>> else
>> advise(_("after resolving the conflicts, mark the corrected paths\n"
>> "with 'git add <paths>' or 'git rm <paths>'\n"
>> +
>
> ???
Oops. :)
>> @@ -614,6 +615,28 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
>> res = allow;
>> goto leave;
>> }
>> +
>> + // If told, do not try to commit things that don't make any
>> + // changes.
>
> No C++/C99 comments, please.
Will fix.
>> + if (opts->skip_redundant_commits) {
>> + int index_unchanged = is_index_unchanged();
>> + if (index_unchanged < 0) {
>> + // Something bad happened readhing HEAD or the
>> + // index. Abort.
>> + res = index_unchanged;
>> + goto leave;
>> + }
>> + if (index_unchanged) {
>> + fputs(_("Skipping redundant commit "), stderr);
>> + fputs(find_unique_abbrev(commit->object.oid.hash,
>> + GIT_SHA1_HEXSZ),
>> + stderr);
>> + fputs("\n", stderr);
>
> This is a bad i18n; we do not know the sentence "Skipping commit X"
> is translated to have X at the end of the sentence in all languages.
>
> fprintf(stderr, _("Skipping ... %s\n"), find_unique_abbrev(...));
>
> would allow it to be tranlated to "Commit X is getting skipped", for
> example.
Ok, thank you for the guidance.
>> diff --git a/sequencer.h b/sequencer.h
>> index 5ed5cb1..ad6145d 100644
>> --- a/sequencer.h
>> +++ b/sequencer.h
>> @@ -34,6 +34,7 @@ struct replay_opts {
>> int allow_empty;
>> int allow_empty_message;
>> int keep_redundant_commits;
>> + int skip_redundant_commits;
>
> Continuing from the top-part of the comments, this may be better to
> be:
>
> enum {
> REPLAY_REDUNDANT_FAIL = 0,
> REPLAY_REDUNDANT_KEEP,
> REPLAY_REDUNDANT_SKIP
> } redundant_commits;
>
> or something like that.
Agreed.
I've also resumed work on my earlier rebase --keep-redundant-commits
change. I think I'm going to reorganize things and send the cherry-pick
changes separate from the rebase changes since the latter depends on the
former. Then all of the redundant commit work on rebase can be in one
series for review.
-David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html