[PATCHv9 0/4] pathspec magic extension to search for attributes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This goes on top of origin/jc/attr, (396bf756f95, attr: expose validity check
for attribute names)

Patches 1 is a small fix, which could go independently as well.
I dropped the patch "string list: improve comment"
Patches 3 and 4 are refactoring pathspec.c a little.
These did not change since v7

Patch 5 contains all of Junios suggestions.

Thanks,
Stefan

diff to v8:
diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index aa9f220..e06520b 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -385,20 +385,23 @@ full pathname may have special meaning:
 Glob magic is incompatible with literal magic.
 
 attr;;
-	Additionally to matching the pathspec, the path must have the
-	attribute as specified. The syntax for specifying the required
-	attributes is "`attr: [mode] <attribute name> [=value]`"
-+
-Attributes can have 4 states (Set, Unset, Set to a value, unspecified) and
-you can query each attribute for certain states. The "`[mode]`" is a special
-character to indicate which attribute states are looked for. The following
-modes are available:
-
- - an empty "`[mode]`" matches if the attribute is set
- - "`-`" the attribute must be unset
- - "`!`" the attribute must be unspecified
- - an empty "`[mode]`" combined with "`[=value]`" matches if the attribute has
-   the given value.
+After `attr:` comes a space separated list of "attribute
+requirements", all of which must be met in order for the
+path to be considered a match; this is in addition to the
+usual non-magic pathspec pattern matching.
+
+Each of the attribute requirements for the path takes one of
+these forms:
+
+- "`ATTR`" requires that the attribute `ATTR` must be set.
+
+- "`-ATTR`" requires that the attribute `ATTR` must be unset.
+
+- "`ATTR=VALUE`" requires that the attribute `ATTR` must be
+  set to the string `VALUE`.
+
+- "`!ATTR`" requires that the attribute `ATTR` must be
+  unspecified.
 +
 
 exclude;;
diff --git a/dir.c b/dir.c
index f60a503..fc071af 100644
--- a/dir.c
+++ b/dir.c
@@ -231,11 +231,11 @@ static int match_attrs(const char *name, int namelen,
 		match_mode = item->attr_match[i].match_mode;
 
 		if (ATTR_TRUE(value))
-			matched = match_mode == MATCH_SET;
+			matched = (match_mode == MATCH_SET);
 		else if (ATTR_FALSE(value))
-			matched = match_mode == MATCH_UNSET;
+			matched = (match_mode == MATCH_UNSET);
 		else if (ATTR_UNSET(value))
-			matched = match_mode == MATCH_UNSPECIFIED;
+			matched = (match_mode == MATCH_UNSPECIFIED);
 		else
 			matched = (match_mode == MATCH_VALUE &&
 				   !strcmp(item->attr_match[i].value, value));
diff --git a/pathspec.c b/pathspec.c
index b795a9c..693a5e7 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -115,34 +115,38 @@ static void parse_pathspec_attr_match(struct pathspec_item *item, const char *va
 		const char *attr = si->string;
 		struct attr_match *am = &item->attr_match[j];
 
-		if (attr[0] == '!')
+		attr_len = strcspn(attr, "=");
+		switch (*attr) {
+		case '!':
 			am->match_mode = MATCH_UNSPECIFIED;
-		else if (attr[0] == '-')
+			attr++;
+			attr_len--;
+			break;
+		case '-':
 			am->match_mode = MATCH_UNSET;
-		else
-			am->match_mode = MATCH_SET;
-
-		if (am->match_mode != MATCH_SET)
-			/* skip first character */
 			attr++;
+			attr_len--;
+			break;
+		default:
+			if (attr[attr_len] != '=')
+				am->match_mode = MATCH_SET;
+			else {
+				am->match_mode = MATCH_VALUE;
+				am->value = xstrdup(&attr[attr_len + 1]);
+				if (strchr(am->value, '\\'))
+					die(_("attr spec values must not contain backslashes"));
+			}
+			break;
+		}
 
-		attr_len = strcspn(attr, "=");
-		if (attr[attr_len] == '=') {
-			am->match_mode = MATCH_VALUE;
-			am->value = xstrdup(&attr[attr_len + 1]);
-			if (strchr(am->value, '\\'))
-				die(_("attr spec values must not contain backslashes"));
-		} else
-			am->value = NULL;
-
-		if (!attr_name_valid(attr, attr_len)) {
+		am->attr = git_attr_counted(attr, attr_len);
+		if (!am->attr) {
 			struct strbuf sb = STRBUF_INIT;
 			am->match_mode = INVALID_ATTR;
 			invalid_attr_name_message(&sb, attr, attr_len);
 			die(_("invalid attribute in '%s': '%s'"), value, sb.buf);
 		}
 
-		am->attr = git_attr_counted(attr, attr_len);
 		git_attr_check_append(item->attr_check, am->attr);
 	}
 
diff --git a/string-list.h b/string-list.h
index 465a1f0..d3809a1 100644
--- a/string-list.h
+++ b/string-list.h
@@ -106,7 +106,7 @@ void unsorted_string_list_delete_item(struct string_list *list, int i, int free_
  * list->strdup_strings must be set, as new memory needs to be
  * allocated to hold the substrings.  If maxsplit is non-negative,
  * then split at most maxsplit times.  Return the number of substrings
- * appended to list. The list may be non-empty already.
+ * appended to list.
  *
  * Examples:
  *   string_list_split(l, "foo:bar:baz", ':', -1) -> ["foo", "bar", "baz"]
diff --git a/t/t6134-pathspec-with-labels.sh b/t/t6134-pathspec-with-labels.sh
index c0d8cda..5da1a63 100755
--- a/t/t6134-pathspec-with-labels.sh
+++ b/t/t6134-pathspec-with-labels.sh
@@ -4,39 +4,38 @@ test_description='test labels in pathspecs'
 . ./test-lib.sh
 
 test_expect_success 'setup a tree' '
+	cat <<-EOF >expect &&
+	fileA
+	fileAB
+	fileAC
+	fileB
+	fileBC
+	fileC
+	fileNoLabel
+	fileSetLabel
+	fileUnsetLabel
+	fileValue
+	fileWrongLabel
+	sub/fileA
+	sub/fileAB
+	sub/fileAC
+	sub/fileB
+	sub/fileBC
+	sub/fileC
+	sub/fileNoLabel
+	sub/fileSetLabel
+	sub/fileUnsetLabel
+	sub/fileValue
+	sub/fileWrongLabel
+	EOF
 	mkdir sub &&
-	for p in fileA fileB fileC fileAB fileAC fileBC fileNoLabel fileUnsetLabel fileSetLabel fileValue fileWrongLabel; do
-		: >$p &&
-		git add $p &&
-		: >sub/$p
-		git add sub/$p
-	done &&
-	git commit -m $p &&
+	while read path
+	do
+		: >$path &&
+		git add $path || return 1
+	done <expect &&
+	git commit -m "initial commit" &&
 	git ls-files >actual &&
-	cat <<EOF >expect &&
-fileA
-fileAB
-fileAC
-fileB
-fileBC
-fileC
-fileNoLabel
-fileSetLabel
-fileUnsetLabel
-fileValue
-fileWrongLabel
-sub/fileA
-sub/fileAB
-sub/fileAC
-sub/fileB
-sub/fileBC
-sub/fileC
-sub/fileNoLabel
-sub/fileSetLabel
-sub/fileUnsetLabel
-sub/fileValue
-sub/fileWrongLabel
-EOF
 	test_cmp expect actual
 '
 
@@ -51,47 +50,45 @@ test_expect_success 'pathspec with labels and non existent .gitattributes' '
 '
 
 test_expect_success 'setup .gitattributes' '
-	cat <<EOF >.gitattributes &&
-fileA labelA
-fileB labelB
-fileC labelC
-fileAB labelA labelB
-fileAC labelA labelC
-fileBC labelB labelC
-fileUnsetLabel -label
-fileSetLabel label
-fileValue label=foo
-fileWrongLabel label☺
-EOF
+	cat <<-EOF >.gitattributes &&
+	fileA labelA
+	fileB labelB
+	fileC labelC
+	fileAB labelA labelB
+	fileAC labelA labelC
+	fileBC labelB labelC
+	fileUnsetLabel -label
+	fileSetLabel label
+	fileValue label=foo
+	fileWrongLabel label☺
+	EOF
 	git add .gitattributes &&
 	git commit -m "add attributes"
 '
 
-sq="'"
-
 test_expect_success 'check specific set attr' '
-	cat <<EOF >expect &&
-fileSetLabel
-sub/fileSetLabel
-EOF
+	cat <<-EOF >expect &&
+	fileSetLabel
+	sub/fileSetLabel
+	EOF
 	git ls-files ":(attr:label)" >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'check specific unset attr' '
-	cat <<EOF >expect &&
-fileUnsetLabel
-sub/fileUnsetLabel
-EOF
+	cat <<-EOF >expect &&
+	fileUnsetLabel
+	sub/fileUnsetLabel
+	EOF
 	git ls-files ":(attr:-label)" >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'check specific value attr' '
-	cat <<EOF >expect &&
-fileValue
-sub/fileValue
-EOF
+	cat <<-EOF >expect &&
+	fileValue
+	sub/fileValue
+	EOF
 	git ls-files ":(attr:label=foo)" >actual &&
 	test_cmp expect actual &&
 	git ls-files ":(attr:label=bar)" >actual &&
@@ -99,61 +96,61 @@ EOF
 '
 
 test_expect_success 'check unspecified attr' '
-	cat <<EOF >expect &&
-.gitattributes
-fileA
-fileAB
-fileAC
-fileB
-fileBC
-fileC
-fileNoLabel
-fileWrongLabel
-sub/fileA
-sub/fileAB
-sub/fileAC
-sub/fileB
-sub/fileBC
-sub/fileC
-sub/fileNoLabel
-sub/fileWrongLabel
-EOF
-	git ls-files :\(attr:\!label\) >actual &&
+	cat <<-EOF >expect &&
+	.gitattributes
+	fileA
+	fileAB
+	fileAC
+	fileB
+	fileBC
+	fileC
+	fileNoLabel
+	fileWrongLabel
+	sub/fileA
+	sub/fileAB
+	sub/fileAC
+	sub/fileB
+	sub/fileBC
+	sub/fileC
+	sub/fileNoLabel
+	sub/fileWrongLabel
+	EOF
+	git ls-files ":(attr:!label)" >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'check multiple unspecified attr' '
-	cat <<EOF >expect &&
-.gitattributes
-fileC
-fileNoLabel
-fileWrongLabel
-sub/fileC
-sub/fileNoLabel
-sub/fileWrongLabel
-EOF
-	git ls-files :\(attr:\!labelB\ \!labelA\ \!label\) >actual &&
+	cat <<-EOF >expect &&
+	.gitattributes
+	fileC
+	fileNoLabel
+	fileWrongLabel
+	sub/fileC
+	sub/fileNoLabel
+	sub/fileWrongLabel
+	EOF
+	git ls-files ":(attr:!labelB !labelA !label)" >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'check label with more labels but excluded path' '
-	cat <<EOF >expect &&
-fileAB
-fileB
-fileBC
-EOF
+	cat <<-EOF >expect &&
+	fileAB
+	fileB
+	fileBC
+	EOF
 	git ls-files ":(attr:labelB)" ":(exclude)sub/" >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'check label excluding other labels' '
-	cat <<EOF >expect &&
-fileAB
-fileB
-fileBC
-sub/fileAB
-sub/fileB
-EOF
+	cat <<-EOF >expect &&
+	fileAB
+	fileB
+	fileBC
+	sub/fileAB
+	sub/fileB
+	EOF
 	git ls-files ":(attr:labelB)" ":(exclude,attr:labelC)sub/" >actual &&
 	test_cmp expect actual
 '

--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]