"Pat Notz" <patnotz@xxxxxxxxx> writes:
> + } else if (fixup_message) {
> + unsigned char sha1[20];
> + struct commit *commit;
> + struct pretty_print_context ctx = {0};
> + if (get_sha1(fixup_message, sha1))
> + die("could not lookup commit %s", fixup_message);
> + commit = lookup_commit_reference(sha1);
> + if (!commit || parse_commit(commit))
> + die("could not parse commit %s", fixup_message);
> + format_commit_message(commit, "fixup! %s\n\n", &sb, &ctx);
> + hook_arg1 = "message";
I notice that the above is a half-copy-and-paste from "if (use_message)"
codepath that handles -c/-C. A few issues to think about (i.e. not
complaints; I haven't thought about them myself):
(1) Is it worth refactoring the original instead of copying;
(2) What happens/should happen when the original commit is encoded
differently from the current commit encoding? -c/-C codepath takes
pains to re-encode. Should we do so somewhere in this codepath, too?
(3) If the answer to (2) is "Yes", notice that format_commit_message()
does not re-encode the commit log message ("log" output codepath uses
pretty.c::pretty_print_commit(), which reencodes for log output
encoding). Maybe we need an option to tell format_commit_message()
to do so?
The last is not exactly an issue this patch alone should address, but I
thought I'd better mention it anyway.
My knee-jerk answers to the above are:
(1) The first handful of lines in this new "if (fixup_message)" codeblock
up to "die" might want to use a helper function shared with the
existing "if (use_message)" codepath;
(2) We probably want to re-encode to the log output encoding the string
we receive from format_commit_message() in this codepath.
(3) No need yet.
--
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