Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> @@ -43,6 +44,20 @@ static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
> */
> static GIT_PATH_FUNC(rebase_path_author_script, "rebase-merge/author-script")
> /*
It is minor, but please have a blank line to separate the logical
blocks. If you have "comment for thing A" before "thing A", then
having a blank after that before "comment for thing B" and "thing B"
that follow would help. Otherwise "thing A" immediately followed by
"comment for thing B" are (mis)read together, leading to nonsense.
> + * When an "edit" rebase command is being processed, the SHA1 of the
> + * commit to be edited is recorded in this file. When "git rebase
> + * --continue" is executed, if there are any staged changes then they
> + * will be amended to the HEAD commit, but only provided the HEAD
> + * commit is still the commit to be edited. When any other rebase
> + * command is processed, this file is deleted.
> + */
> +static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
> +/*
> + * When we stop at a given patch via the "edit" command, this file contains
> + * the long commit name of the corresponding patch.
If you abbreviate an object name to 38-hex that is still long but
that is not full; if you meant full 40-hex, better spell it out as
"full"---that conveys useful information to programmers (e.g. they
can just use get_sha1_hex()).
But I think you are writing short_commit_name() to it? So perhaps
"an abbreviated commit object name"?
> @@ -1301,9 +1318,87 @@ static int save_opts(struct replay_opts *opts)
> return res;
> }
>
> +static int make_patch(struct commit *commit, struct replay_opts *opts)
> +{
> + struct strbuf buf = STRBUF_INIT;
> + struct rev_info log_tree_opt;
> + const char *commit_buffer = get_commit_buffer(commit, NULL), *subject, *p;
> + int res = 0;
> +
> + p = short_commit_name(commit);
> + if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
> + return -1;
> +
> + strbuf_addf(&buf, "%s/patch", get_dir(opts));
> + memset(&log_tree_opt, 0, sizeof(log_tree_opt));
> + init_revisions(&log_tree_opt, NULL);
> + log_tree_opt.abbrev = 0;
> + log_tree_opt.diff = 1;
> + log_tree_opt.diffopt.output_format = DIFF_FORMAT_PATCH;
> + log_tree_opt.disable_stdin = 1;
> + log_tree_opt.no_commit_id = 1;
> + log_tree_opt.diffopt.file = fopen(buf.buf, "w");
> + log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER;
> + if (!log_tree_opt.diffopt.file)
> + res |= error_errno(_("could not open '%s'"), buf.buf);
> + else {
> + res |= log_tree_commit(&log_tree_opt, commit);
> + fclose(log_tree_opt.diffopt.file);
> + }
> + strbuf_reset(&buf);
> + strbuf_addf(&buf, "%s/message", get_dir(opts));
> + if (!file_exists(buf.buf)) {
> + find_commit_subject(commit_buffer, &subject);
> + res |= write_message(subject, strlen(subject), buf.buf, 1);
> + unuse_commit_buffer(commit, commit_buffer);
> + }
> + strbuf_release(&buf);
> +
> + return res;
> +}
OK. This seems to match what scripted make_patch does in a handful
of lines. We probably should have given you a helper to reduce
boilerplate that sets up log_tree_opt so that this function does not
have to be this long, but that is a separate topic.
Does it matter output_format is set to FORMAT_PATCH here, though?
With --no-commit-id set, I suspect there is no log message or
authorship information given to the output.
As you are only interested in seeing the patch/diff in this file and
the log is stored in a separate "message" file, as long as "patch"
file gets the patch correctly, it is not a problem, but it just
looked strange to see FORMAT_PATCH there.