Jacob Keller <jacob.e.keller@xxxxxxxxx> writes:
> +notes.<localref>.merge::
> + Which merge strategy to choose if the local ref for a notes merge
> + matches <localref>. Is overridden by notes.merge and takes the same
> + values. <localref> may be fully qualified or just under refs/notes/.
> + See "NOTES MERGE STRATEGIES" section in linkgit:git-notes[1] for more
> + information on each strategy.
If I have notes.refs/notes/commit.merge and notes.merge specified,
I'd expect the former overrides the latter. The second sentence may
need correcting.
I think it is a mistake to allow both notes.refs/notes/commit.merge
and notes.commit.merge. You'd end up needing to implement quite a
complicated "the last one wins" rule if you did so.
> +notes.<localref>.merge::
> + Which strategy to choose when merging into <localref>. Uses the same
> + values as notes.merge. <localref> may be either a fully qualified ref
> + or the shortname under "refs/notes/". See "NOTES MERGE STRATEGIES"
> + section above for more information about each strategy.
As a reviewer, I can tell that "Uses the same values" wants to say
that the set of allowed values is the same, but a casual reader is
bound to read it as "notes.commit.merge must be set to the same
value as the value set to notes.merge".
> diff --git a/builtin/notes.c b/builtin/notes.c
> index de0caa00df1b..b0174d1024dc 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -92,6 +92,10 @@ static const char * const git_notes_get_ref_usage[] = {
> static const char note_template[] =
> "\nWrite/edit the notes for the following object:\n";
>
> +static struct note_ref **note_refs;
> +static int note_refs_alloc;
> +static int note_refs_nr;
> +static struct hashmap note_refs_hash;
> static enum notes_merge_strategy merge_strategy;
>
> struct note_data {
> @@ -757,12 +761,87 @@ static int parse_notes_strategy(const char *arg, enum notes_merge_strategy *stra
> return 0;
> }
> ...
> +struct note_refs_hash_key {
> + const char *str;
> + int len;
> +};
> + ...
> +static void set_strategy_for_ref(const char *ref)
> +{
> + ...
> +}
Hmmm, I do not see why you need all the complexity above.
When you come to merge(), after calling default_notes_ref(), you
know exactly which notes ref you are merging into, no? Shouldn't
then the change required for this feature just the matter of asking
the configuration system values for notes.$remote_ref.merge and
notes.merge?
IOW,
struct strbuf key = STRBUF_INIT;
char *value = NULL;
strbuf_addf(&key, "notes.%s.merge", remote_ref.buf);
git_config_get_string(key.buf, &value) ||
git_config_get_string_const("notes.merge", &value));
if (value)
parse_notes_strategy(value, &configured_merge_strategy);
...
parse_options();
if (strategy)
parse_notes_strategy(value, &configured_merge_strategy);
or something?
--
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