Re: [PATCH v4 02/11] Support showing notes from more than one notes tree
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:48:20
Thomas Rast [off-list ref] writes:
quoted hunk ↗ jump to hunk
diff --git a/Documentation/config.txt b/Documentation/config.txt index 4c36aa9..a0f2b0f 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt@@ -517,7 +517,7 @@ core.notesRef:: after the full SHA-1 of the commit they annotate. + If such a file exists in the given ref, the referenced blob is read, and -appended to the commit message, separated by a "Notes:" line. If the +appended to the commit message, separated by a "Notes from <refname>:" line. If the
Hmm...
quoted hunk ↗ jump to hunk
diff --git a/notes.c b/notes.c index 3ba3e6d..c480370 100644 --- a/notes.c +++ b/notes.c@@ -68,6 +70,9 @@ struct non_note { struct notes_tree default_notes_tree; +struct string_list display_notes_refs; +struct notes_tree **display_notes_trees;
Do these need to be extern?
+void string_list_add_refs_from_glob_list(struct string_list *list,
+ const char *globs)
+{
+ struct strbuf globbuf = STRBUF_INIT;
+ struct strbuf **split;
+ int i;
+
+ strbuf_addstr(&globbuf, globs);
+ split = strbuf_split(&globbuf, ':');
+
+ for (i = 0; split[i]; i++) {
+ if (!split[i]->len)
+ continue;
+ if (split[i]->buf[split[i]->len-1] == ':')
+ strbuf_setlen(split[i], split[i]->len-1);
+ string_list_add_refs_by_glob(list, split[i]->buf);
+ }Nice use of strbuf_split(). I wonder if we should automatically add "refs/" and/or "refs/notes/" when the input is missing the prefix. I don't have strong preference myself but the users might make noises. Either way it needs to be documented in the final version before the series goes to 'master'.
+static int notes_display_config(const char *k, const char *v, void *cb)
+{
+ /* Warning! This is currently not executed if
+ * GIT_NOTES_DISPLAY_REF is set. Move the git_config() call
+ * outside the test if you add more options. */Yuck. If you know what needs to be done, do that before other poeple add more options, please.
quoted hunk ↗ jump to hunk
@@ -1016,7 +1135,18 @@ void format_note(struct notes_tree *t, const unsigned char *object_sha1, if (msglen && msg[msglen - 1] == '\n') msglen--; - if (flags & NOTES_SHOW_HEADER) + if (flags & NOTES_SHOW_HEADER_WITH_REF && t->ref) { + const char *ref = t->ref; + if (!strcmp(ref, GIT_NOTES_DEFAULT_REF)) { + strbuf_addstr(sb, "\nNotes:\n"); + } else { + if (!prefixcmp(ref, "refs/")) + ref += 5; + if (!prefixcmp(ref, "notes/")) + ref += 6; + strbuf_addf(sb, "\nNotes (%s):\n", ref); + } + } else if (flags & (NOTES_SHOW_HEADER|NOTES_SHOW_HEADER_WITH_REF)) strbuf_addstr(sb, "\nNotes:\n");
It is not clear what the distinction between NOTES_SHOW_HEADER and NOTES_SHOW_HEADER_WITH_REF. Does anybody still call this function with NOTES_SHOW_HEADER alone without NOTES_SHOW_HEADER_WITH_REF? I expected to see "Notes:\n" regardless of the mode if the notes is coming from the default refs/notes/commits tree, but it probably is better to say "Notes (commits):\n" like your patch does.