Re: [PATCH v4 02/11] Support showing notes from more than one notes tree
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 22, 2010, 23:20 UTC
- Message-ID
- <7v1vgc26zd.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <d51082ad62153e182298242a2e4b7c3c76000a2e.1266797028.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
Show 10 quoted lines
> 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...
Show 10 quoted lines
> 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?
Show 17 quoted lines
> +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'.
Show 5 quoted lines
> +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.
Show 18 quoted lines
> @@ -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.