Thomas Rast <trast@xxxxxxxxxxxxxxx> writes:
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 4c36aa9..3c5cfec 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
I haven't looked at your code, but I think the output should be kept the
same when <refname> is "refs/notes/commits", and it probably is a good
idea to omit "refs/notes/" part. E.g. "Notes (amlog):" for notes stored
in the refs/notes/amlog hierarchy.
> @@ -1300,6 +1300,17 @@ mergetool.keepTemporaries::
> mergetool.prompt::
> Prompt before each invocation of the merge resolution program.
>
> +notes.displayRef::
> + When showing commit messages, also show notes which are stored
> + in the given ref. The ref may also be a glob, in which case
If this were a description that follows an example
[notes]
displayRef = <ref>
then the above sentences make sense, but otherwise "in the given ref" does
not make much sense. "The ref may also be a glob" is a lot worse. A "ref"
is never be a glob. You can specify what refs to display by setting the
_value_ of this variable to a glob.
notes.displayRef::
The refname in the notes namespace from which to show notes when
showing commit messages. The value of this variable can be set to
a glob, in whch case ...
> + ... If a ref
> + does not exist, it will be skipped silently.
Hmph... Even when it is not a globbing specification? If I said
[notes]
displayRef = refs/notes/tr/*
and there is no notes in the tr/ hierarchy yet, I would appreciate if the
code doesn't complain, but if I said a more concrete
[notes]
displayRef = refs/notes/anlog
and there is no "anlog", because I meant "amlog" and the above was a typo,
I would appreciate if the code somehow lets me know about it.
> +static int notes_display_config(const char *k, const char *v, void *cb)
> +{
> + if (!strcmp(k, "notes.displayref")) {
> + string_list_add_refs_by_glob(&display_notes_refs, v);
Somebody in the callchain should say
if (!v)
config_error_nonbool(k)
before you use v here.
I don't think string_list_insert() has a desirable property for this.
If I wrote
[notes]
displayRef = refs/notes/commit
displayRef = refs/notes/amlog
I would expect that the normal notes come first and then notes from the
amlog tree, not in the alphabetical "amlog happens to sort before commit"
order. If I used a glob, e.g. "refs/notes/tr/*", I am saying "all notes
in the tr/ namespace, I do not care about the particular order among them.
Just use something sensible", and it would make sense to use some stable
ordering, e.g. the order for_each_glob_ref() calls you back, but receiving
the data and calling string_list_insert() will destroy the order the user
gave you the configuration, no?
> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> index 90178f9..0dfff82 100755
> --- a/t/t3301-notes.sh
> +++ b/t/t3301-notes.sh
> @@ -98,7 +98,7 @@ Date: Thu Apr 7 15:14:13 2005 -0700
>
> 2nd
>
> -Notes:
> +Notes from notes/commits:
Please don't.
--
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