Re: [PATCH v2 10/12] attr: convert git_all_attrs() to use "struct git_attr_check"

Junio C Hamano [off-list ref] writes:

+struct git_attr_check *git_attr_check_alloc(void)
+{
+	return xcalloc(1, sizeof(struct git_attr_check));
+}
+
+void git_attr_check_append(struct git_attr_check *check, const char *name)
+{
+	struct git_attr *attr;
+
+	if (check->finalized)
+		die("BUG: append after git_attr_check structure is finalized");
+
+	attr = git_attr(name);
+	if (!attr)
+		die("%s: not a valid attribute name", name);
+	ALLOC_GROW(check->check, check->check_nr + 1, check->check_alloc);
+	check->check[check->check_nr++].attr = attr;
+}

Given that one of the two expected callers, namely, "check-attr" and
Stefan's pathspec label magic, of this "alloc and then append" side
of the API wants to have an access to git_attr(name), I think
the function signature for this one should be updated to take not
"const char *name" but instead take "struct git_attr *attr", i.e.

void git_attr_check_append(struct git_attr_check *check,
			   struct git_attr *attr);

Then the loop in check-attr below

+		check = git_attr_check_alloc();
+		for (i = 0; i < cnt; i++)
+			git_attr_check_append(check, argv[i]);

would become

		check = git_attr_check_alloc();
		for (i = 0; i < cnt; i++)
			git_attr_check_append(check, git_attr(argv[i]));

And this part in $gmane/294855

    Then while parsing ":(attr:VAR1=VAL1 -VAR2 VAR3...)path/to/dir/",
    you would first do:

            p->attr_check = git_attr_check_alloc();

    once, and then send each of VAR1=VAL2, -VAR2, VAR3... to your
    parse_one_item() helper function which would:

     * parse the match-mode like your code does;

     * parse out the attribute name (i.e. VAR1, VAR2 and VAR3), and
       instead of keeping it as a "(const) char *", call git_attr()
       to intern it (and keep it in local variable "attr"), and save
       it in p->attr_match[p->attr_nr].attr;

     * call git_attr_check_append(p->attr_check, git_attr_name(attr))

would look like

	... saw ":(attr:", expect "VAR1=VAL1 -VAR2...)" to follow;
        ... char *scan points at byte after attr: now.

	item->attr_check = git_attr_check_alloc();
	while (scan) {
		const char *var, *val;
                enum attr_match_mode mode;
		struct git_attr *attr;
		struct attr_match *match;

        	scan = parse_attr_match(scan, &var, &val, &mode);
                ... var points at VAR1, val points at VAL,
                ... mode becomes MATCH_VALUE; scan skips
                ... forward to pint at -VAR2.  parse_attr_match()
                ... would return NULL once input runs out.

                ALLOC_GROW(item->attr_match, ...);
                match = &item->attr_match[item->attr_match_nr++];

        	attr = git_attr(var);

		match->attr = attr;
                match->value = val;
                match->match_mode = mode;
                
                git_attr_check_append(item->attr_check, attr);
	}

Then matching part would get the "item" and would do:

	... after making sure that the path matches
        ... the pathspec the magic is attached to ...

	git_check_attr(path, item->attr_check);
	for (i = 0; i < item->attr_match_nr; i++) {
		struct git_attr *attr = item->attr_check->check[i].attr;
		const char *value = item->attr_check->check[i].value;
                struct attr_match *match = &item->attr_match[i];

                ... compare what "match" expects with <attr, value> ...
        }