format-patch: teach format.notes config option

In git-format-patch, notes can be appended with the --notes option. However, this must be specified by the user on an invocation-by-invocation basis. If a user is not careful, it's possible that they may forget to include it and generate a patch series without notes.

Teach git-format-patch the format.notes config option. Its value is a notes ref that will be automatically appended. The special value of "standard" can be used to specify the standard notes. This option is overridable with the --no-notes option in case a user wishes not to append notes.

Signed-off-by Denton Liu <[email protected]>

---
 Documentation/config/format.txt    | 15 +++++++
 Documentation/git-format-patch.txt |  3 ++
 builtin/log.c                      | 20 ++++++++-
 t/t4014-format-patch.sh            | 70 ++++++++++++++++++++++++++++++
 4 files changed, 107 insertions(+), 1 deletion(-)

Documentation/config/format.txt +15 -0

index dc77941c48..414a5a8a9d 100644
@@ -85,3 +85,18 @@ format.outputDirectory::
 format.useAutoBase::
        A boolean value which lets you enable the `--base=auto` option of
        format-patch by default.
+
+format.notes::
+       Provides the default value for the `--notes` option to
+       format-patch. Accepts a boolean value, or a ref which specifies
+       where to get notes. If false, format-patch defaults to
+       `--no-notes`. If true, format-patch defaults to `--notes`. If
+       set to a non-boolean value, format-patch defaults to
+       `--notes=<ref>`, where `ref` is the non-boolean value. Defaults
+       to false.
++
+If one wishes to use the ref `ref/notes/true`, please use that literal
+instead.
++
+This configuration can be specified multiple times in order to allow
+multiple notes refs to be included.

Documentation/git-format-patch.txt +3 -0

index 2c3971390e..9ce5b8aaee 100644
@@ -275,6 +275,9 @@ these explanations after `format-patch` has run but before sending,
 keeping them as Git notes allows them to be maintained between versions
 of the patch series (but see the discussion of the `notes.rewrite`
 configuration options in linkgit:git-notes[1] to use this workflow).
++
+The default is `--no-notes`, unless the `format.notes` configuration is
+set.
 
 --[no-]signature=<signature>::
        Add a signature to each message produced. Per RFC 3676 the signature

builtin/log.c +8 -0

index e43ee12fb1..dad63cffc6 100644
@@ -779,6 +779,8 @@ enum {
 
 static int git_format_config(const char *var, const char *value, void *cb)
 {
+       struct rev_info *rev = cb;
+
        if (!strcmp(var, "format.headers")) {
                if (!value)
                        die(_("format.headers without value"));

@@ -864,6 +866,22 @@ static int git_format_config(const char *var, const char *value, void *cb)
                        from = NULL;
                return 0;
        }
+       if (!strcmp(var, "format.notes")) {
+               struct strbuf buf = STRBUF_INIT;
+               int b = git_parse_maybe_bool(value);
+               if (!b)
+                       return 0;
+               rev->show_notes = 1;

Quoted 10 lines

builtin/log.c +6 -0

@@ -864,6 +866,22 @@ static int git_format_config(const char *var, const char *value, void *cb)
                        from = NULL;
                return 0;
        }
+       if (!strcmp(var, "format.notes")) {
+               struct strbuf buf = STRBUF_INIT;
+               int b = git_parse_maybe_bool(value);
+               if (!b)
+                       return 0;
+               rev->show_notes = 1;

builtin/log.c static int git_format_config(const char *var, const char *value, void *cb) · line 884

Quoted 10 lines

builtin/log.c +10 -0

+               if (b < 0) {
+                       strbuf_addstr(&buf, value);
+                       expand_notes_ref(&buf);
+                       string_list_append(&rev->notes_opt.extra_notes_refs,
+                                       strbuf_detach(&buf, NULL));
+               } else {
+                       rev->notes_opt.use_default_notes = 1;
+               }
+               return 0;
+       }

What if someone has multiple format.notes entries in their config file, but the last entry is "false" -- shouldn't that disable notes? Also, what if they specify both "true" and e.g. "refs/notes/my-cool-notes"? In that case, should it show refs/notes/my-cool-notes because that's obviously showing some notes so it satisfies true as well as the specific request about which note, or should it treat "true" the same as the-default-notes-ref and then add the two refs together and show them both?

builtin/log.c int cmd_format_patch(int argc, const char **argv, const char *prefix) · line 1639

Quoted 11 lines

builtin/log.c +1 -1

        return git_log_config(var, value, cb);
 }

@@ -1617,8 +1635,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
        extra_to.strdup_strings = 1;
        extra_cc.strdup_strings = 1;
        init_log_defaults();
-       git_config(git_format_config, NULL);
        repo_init_revisions(the_repository, &rev, prefix);
+       git_config(git_format_config, &rev);

Calling git_config() after repo_init_revisions() breaks things; generally git_config() should always be called first. Here, git_format_config() can set up parameters used by repo_init_revisions(), and by reversing the order of the two you end

up ignoring settings specified by the user (e.g. diff.context having a value of 5). This came up due to the bug report at
https://lore.kernel.org/git/[email protected]/T/#mb6a09958ff10acde295b37a9136bc3791fd4a2c2
(though fixing the issue there _also_ requires fixing git_am_config() to call git_diff_ui_config()). To break the circular dependency here, we'd need to store the information that git_format_config() discovers in some data structure besides rev, and then after the repo_init_revisions() call has finished then update rev.

I was just going to do that, but then ran into the questions above about multiple format.notes entries in the config file, and am not as sure about what should be done about that stuff (and I don't want to try to translate the current behavior as-is while tweaking where the stuff is stored, both because I'm not sure of the right behavior and because I don't want future folks to blame the code to me when they hit bugs in this area), so I'm firing off this email instead.

So, um...help?

Thanks,
Elijah